From a130c8efc382bc48f551f6f46102f2f521f20263 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 15:17:16 +0200 Subject: [PATCH 1/4] fix(properties): stop a boundary port type edit from renaming the port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating why a boundary port's type is read-only turned up something worse than a missing feature: the one place that DID offer the edit corrupted the source. A label edit travels as the protocol `ApplyLabelEditOperation`, and the only handler registered for it turns every label edit into a rename of the nearest ancestor carrying an entity name. For a boundary port's type label that ancestor is the boundary node — whose entity name is the PORT's name — so the edit went out as `renameNode { old: , new: }`. Changing a type renamed the port to its own type. An entity node's type subtitle had the same shape. Both the property panel's "Change Port Type" and a double-click on the canvas label reached it. The handler now allow-lists label types rather than accepting all of them: only a name may rename, and a label type added later is refused until someone decides a rename is what editing it means. Elements that are not typed labels pass through unchanged, so the MCP path that addresses a node by its label id still works. With the write path inert rather than destructive, the two affordances that promise a type edit are withdrawn — an editor that silently discards what you type is still worth not offering. Making it work needs a sidecar op that addresses a boundary port by `workflow`, the way `createPort` already does, rather than by an owning `entity`, which is what `updatePortType` requires. That is a change to both sidecars and is specced separately. Separately, boundary ports become navigable. The graph schema gives every port a `source: { file, line }` and an entity's port has always carried it into the model, which is what the go-to-source menu reads; `createBoundaryNode` took `name` and `type` off the same object and dropped `source`. It now emits the same metadata triple the entity ports do, so both travel one navigation path. Note this reaches the PORT's declaration, not its type's definition — that needs a schema field which does not exist yet. The parity baseline records the one intended registration change: `label:boundary:type` moves to the non-editable `WorkflowLabel`. --- packages/diagram-client/src/property-panel.ts | 21 ++-- .../diagram-client/src/stock-views.module.ts | 15 ++- .../test/container-parity.test.ts | 6 +- .../fixtures/container-parity.baseline.json | 12 +-- .../src/model/graph-gmodel-source.ts | 26 ++++- .../test/boundary-port-navigation.test.ts | 97 +++++++++++++++++++ .../apply-label-edit-rename-handler.ts | 33 ++++++- .../apply-label-edit-rename-handler.test.ts | 93 +++++++++++++++++- 8 files changed, 274 insertions(+), 29 deletions(-) create mode 100644 packages/diagram-server/test/boundary-port-navigation.test.ts diff --git a/packages/diagram-client/src/property-panel.ts b/packages/diagram-client/src/property-panel.ts index 5725592..c750a99 100644 --- a/packages/diagram-client/src/property-panel.ts +++ b/packages/diagram-client/src/property-panel.ts @@ -4454,20 +4454,17 @@ export class PropertyPanel implements ISelectionListener, IGModelRootListener { } as any); }); - const typeField = document.createElement('button'); - typeField.type = 'button'; + // Read-only, deliberately. This offered "Double-click to change + // type", and the edit went out as a label edit — whose only handler + // renames the nearest entity, i.e. the port. Changing a type renamed + // the port to that type. The handler now refuses non-name labels, so + // the edit would be inert instead; either way there is nothing to + // offer until a sidecar op can address a boundary port by `workflow` + // rather than by an owning `entity`. + const typeField = document.createElement('span'); typeField.className = 'port-field port-type'; - typeField.title = 'Double-click to change type'; + typeField.title = `Type: ${portType}`; typeField.textContent = portType; - typeField.addEventListener('dblclick', (e) => { - e.stopPropagation(); - void this.actionDispatcher.dispatch({ - kind: WorkflowPromptLabelEditAction.KIND, - labelId: `${node.id}_label_type`, - title: 'Change Port Type', - value: portType - } as any); - }); const removeBtn = document.createElement('button'); removeBtn.type = 'button'; diff --git a/packages/diagram-client/src/stock-views.module.ts b/packages/diagram-client/src/stock-views.module.ts index 2bc8809..f993892 100644 --- a/packages/diagram-client/src/stock-views.module.ts +++ b/packages/diagram-client/src/stock-views.module.ts @@ -117,9 +117,20 @@ export const workflowViewsModule = new ContainerModule((bind, unbind, isBound, r configureModelElement(context, WorkflowDiagramTypes.LABEL_TYPE, WorkflowLabel, GLabelView); configureModelElement(context, WorkflowDiagramTypes.LABEL_BADGE, WorkflowLabel, GLabelView); - // Boundary labels are directly editable (name + type) + // The boundary NAME is directly editable; the type is not. + // + // A label edit travels as the protocol `ApplyLabelEditOperation`, and the + // only handler for that renames the nearest entity — which for a boundary + // type label is the port itself. Editing the type therefore renamed the port + // to whatever type was typed. The handler now refuses anything that is not a + // name label, so the edit is merely inert rather than destructive; an editor + // that silently discards what you type is still worth not offering. + // + // Making the type editable for real needs a sidecar op that addresses a + // boundary port by `workflow` (the way `createPort` already does) rather + // than by owning `entity`, which is what `updatePortType` requires today. configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_NAME, BoundaryEditableLabel, WorkflowLabelView); - configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE, BoundaryEditableLabel, WorkflowLabelView); + configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE, WorkflowLabel, WorkflowLabelView); // Port labels configureModelElement(context, WorkflowDiagramTypes.LABEL_PORT, WorkflowLabel, WorkflowLabelView); diff --git a/packages/diagram-client/test/container-parity.test.ts b/packages/diagram-client/test/container-parity.test.ts index f0f5139..3163b01 100644 --- a/packages/diagram-client/test/container-parity.test.ts +++ b/packages/diagram-client/test/container-parity.test.ts @@ -15,8 +15,10 @@ * * The baseline is no longer a pure capture of that commit: features added since * the split are appended to it deliberately, and the fixture currently carries - * two — `IEdgeRouter -> LibavoidEdgeRouter` (the client-side live routing tier) - * and `ChangeBoundsTool -> WorkflowChangeBoundsTool` (the mouse-drag threshold). + * two additions — `IEdgeRouter -> LibavoidEdgeRouter` (the client-side live + * routing tier) and `ChangeBoundsTool -> WorkflowChangeBoundsTool` (the + * mouse-drag threshold) — plus one change: `label:boundary:type` is bound to + * the non-editable `WorkflowLabel`, because editing it renamed the port. * The oracle still does its job: it fails on any binding this composition gains * or loses, and updating the fixture is the deliberate act of accepting one. * Regenerate it only after confirming the diff contains exactly the bindings the diff --git a/packages/diagram-client/test/fixtures/container-parity.baseline.json b/packages/diagram-client/test/fixtures/container-parity.baseline.json index 871af7c..da68062 100644 --- a/packages/diagram-client/test/fixtures/container-parity.baseline.json +++ b/packages/diagram-client/test/fixtures/container-parity.baseline.json @@ -191,12 +191,6 @@ "typeId": "label:boundary:name", "view": "WorkflowLabelView" }, - { - "model": "BoundaryEditableLabel", - "op": "modelElement", - "typeId": "label:boundary:type", - "view": "WorkflowLabelView" - }, { "model": "BoundaryInputNode", "op": "modelElement", @@ -347,6 +341,12 @@ "typeId": "label:badge", "view": "GLabelView" }, + { + "model": "WorkflowLabel", + "op": "modelElement", + "typeId": "label:boundary:type", + "view": "WorkflowLabelView" + }, { "model": "WorkflowLabel", "op": "modelElement", diff --git a/packages/diagram-server/src/model/graph-gmodel-source.ts b/packages/diagram-server/src/model/graph-gmodel-source.ts index 428da14..cefebb7 100644 --- a/packages/diagram-server/src/model/graph-gmodel-source.ts +++ b/packages/diagram-server/src/model/graph-gmodel-source.ts @@ -1046,6 +1046,29 @@ export class GraphGModelSource { }; const typeText = port?.type ?? ''; + // Where the port is declared, so "Go to source" works on a boundary node + // the same way it already does on an entity's port (see `mkPort`). This + // was simply never wired: the schema carries `port.source`, and the + // boundary node read only `name` and `type` off the same object. + // + // NOTE this is the PORT's declaration site, not its type's definition — + // navigating to where a type is defined needs a field the graph schema + // does not yet have. + const boundarySourceFile = normalizeNavigationFileUri(port?.source?.file); + const boundarySourceLine = port?.source?.line; + const boundaryNavigation = boundarySourceFile && boundarySourceLine + ? (() => { + const at = { + start: { line: Math.max(0, boundarySourceLine - 1), character: 0 }, + end: { line: Math.max(0, boundarySourceLine - 1), character: 0 } + }; + return { + [WorkflowDiagramMetadata.SOURCE_RANGE]: at, + [WorkflowDiagramMetadata.REFERENCED_SOURCE_RANGE]: at, + [WorkflowDiagramMetadata.REFERENCED_URI]: boundarySourceFile + }; + })() + : {}; return { id: stableNodeId, type: isInput ? WorkflowDiagramTypes.NODE_BOUNDARY_INPUT : WorkflowDiagramTypes.NODE_BOUNDARY_OUTPUT, @@ -1080,7 +1103,8 @@ export class GraphGModelSource { [WorkflowDiagramMetadata.PORT_NAME]: port?.name ?? '', [WorkflowDiagramMetadata.PORT_TYPE]: typeText, 'wf:boundaryKind': isInput ? 'input' : 'output', - 'wf:portName': port?.name ?? '' + 'wf:portName': port?.name ?? '', + ...boundaryNavigation } }; } diff --git a/packages/diagram-server/test/boundary-port-navigation.test.ts b/packages/diagram-server/test/boundary-port-navigation.test.ts new file mode 100644 index 0000000..57bf689 --- /dev/null +++ b/packages/diagram-server/test/boundary-port-navigation.test.ts @@ -0,0 +1,97 @@ +/** + * "Go to source" on a boundary port. + * + * The graph schema gives every port a `source: { file, line }`, and an entity's + * port has always carried it into the model (see `mkPort`) — that is what the + * go-to-source context menu reads. `createBoundaryNode` took the `name` and the + * `type` off the very same port object and dropped `source` on the floor, so a + * network's own inputs and outputs were the one kind of port you could not + * navigate from. Nothing reported it as broken because nothing offered it. + * + * The metadata is deliberately the same triple the entity ports emit, so both + * kinds of port travel the one navigation path rather than growing a second. + */ +import { describe, expect, it } from 'vitest'; +import { URI } from 'vscode-uri'; +import * as path from 'node:path'; +import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { GraphGModelSource, type PyGraphDocument } from '../src/model/graph-gmodel-source'; + +/** A one-node graph whose only content is a boundary input port. */ +function docWithBoundaryInput(source?: { file?: string; line?: number }): PyGraphDocument { + return { + version: '1', + graph: { + id: 'root', + nodes: [ + { + id: 'wf:input:Alloc', + kind: 'wf-input', + label: 'Alloc', + scope: 'root', + ports: [ + { + id: 'port:wf:input:Alloc:out', + name: 'Alloc', + direction: 'out', + type: 'RobAlloc', + source + } + ] + } + ], + edges: [] + } + }; +} + +/** The single boundary node in a transformed graph. */ +function boundaryNodeOf(doc: PyGraphDocument): any { + const result = new GraphGModelSource().transform(doc); + const node = result.graph.children?.find( + (child: any) => child.args?.['wf:boundaryKind'] !== undefined + ); + expect(node, 'no boundary node in the transformed graph').toBeDefined(); + return node; +} + +describe('boundary port navigation metadata', () => { + it('carries the port declaration site through to the model', () => { + const node = boundaryNodeOf(docWithBoundaryInput({ file: '/w/pipeline.py', line: 12 })); + + // Line numbers arrive 1-based from the sidecar and are stored 0-based, + // matching what the entity ports emit. + const at = { start: { line: 11, character: 0 }, end: { line: 11, character: 0 } }; + expect(node.args[WorkflowDiagramMetadata.SOURCE_RANGE]).toEqual(at); + expect(node.args[WorkflowDiagramMetadata.REFERENCED_SOURCE_RANGE]).toEqual(at); + expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBe( + URI.file(path.resolve('/w/pipeline.py')).toString() + ); + }); + + it('leaves the port identity alone', () => { + const node = boundaryNodeOf(docWithBoundaryInput({ file: '/w/pipeline.py', line: 12 })); + + expect(node.args[WorkflowDiagramMetadata.PORT_NAME]).toBe('Alloc'); + expect(node.args[WorkflowDiagramMetadata.PORT_TYPE]).toBe('RobAlloc'); + expect(node.args['wf:boundaryKind']).toBe('input'); + }); + + /** + * A port whose source the sidecar could not resolve must not produce a + * navigation target at all — an entry pointing nowhere would offer the menu + * item and then fail on click. + */ + it('emits nothing navigable when the sidecar gave no source', () => { + const node = boundaryNodeOf(docWithBoundaryInput(undefined)); + + expect(node.args[WorkflowDiagramMetadata.SOURCE_RANGE]).toBeUndefined(); + expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBeUndefined(); + }); + + it('emits nothing navigable when the source has a file but no line', () => { + const node = boundaryNodeOf(docWithBoundaryInput({ file: '/w/pipeline.py' })); + + expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBeUndefined(); + }); +}); diff --git a/packages/sidecar-toolkit/src/server/operations/apply-label-edit-rename-handler.ts b/packages/sidecar-toolkit/src/server/operations/apply-label-edit-rename-handler.ts index b42827d..0fdd90d 100644 --- a/packages/sidecar-toolkit/src/server/operations/apply-label-edit-rename-handler.ts +++ b/packages/sidecar-toolkit/src/server/operations/apply-label-edit-rename-handler.ts @@ -1,6 +1,6 @@ import { Action, ApplyLabelEditOperation, Command, GModelElement, ModelState, OperationHandler } from '@eclipse-glsp/server'; import { inject, injectable } from 'inversify'; -import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import { RenameEntityOperation, RenameEntityOperationHandler } from './rename-entity-handler'; @@ -58,10 +58,14 @@ export class ApplyLabelEditRenameHandler extends OperationHandler { /** * Walk up from the edited label to the nearest ancestor whose `args` carry the entity name — * that is the node {@link RenameEntityOperationHandler} renames. Returns its id, or `undefined` - * when the label belongs to no entity node (e.g. a port/decorator label). + * when the label belongs to no entity node (e.g. a port/decorator label), or when the label is + * not a NAME (see {@link isRenameableLabel}). */ private resolveEntityElementId(labelId: string): string | undefined { let element: GModelElement | undefined = this.modelState.index.find(labelId); + if (!this.isRenameableLabel(element)) { + return undefined; + } while (element) { const name = element.args?.[WorkflowDiagramMetadata.ENTITY_NAME]; if (typeof name === 'string' && name.trim() !== '') { @@ -71,4 +75,29 @@ export class ApplyLabelEditRenameHandler extends OperationHandler { } return undefined; } + + /** + * Whether the edited element is something a rename may act on. + * + * This handler turns EVERY label edit into a rename of the nearest ancestor carrying an entity + * name, which is right for a name label and silently destructive for any other. A boundary + * port's TYPE label is the case that bit: it carries only a port name of its own, so the walk + * lands on the boundary node — whose entity name is the PORT's name — and the edit went out as + * `renameNode { old: , new: }`. Editing a type renamed the + * port to that type. An entity node's type subtitle had the same shape. + * + * So labels are allow-listed by type, not deny-listed: a label type added later is refused + * until someone deliberately decides a rename is what editing it means. + * + * Anything that is not a typed label passes through unchanged — the MCP caller addresses a node + * by its label id, but a synthetic or untyped element must keep the original walk-up rather than + * be silently dropped. + */ + private isRenameableLabel(element: GModelElement | undefined): boolean { + const type = (element as { type?: unknown } | undefined)?.type; + if (typeof type !== 'string' || !type.startsWith('label:')) { + return true; + } + return type === WorkflowDiagramTypes.LABEL_NAME || type === WorkflowDiagramTypes.LABEL_BOUNDARY_NAME; + } } diff --git a/packages/sidecar-toolkit/test/apply-label-edit-rename-handler.test.ts b/packages/sidecar-toolkit/test/apply-label-edit-rename-handler.test.ts index 8b63d14..d28f888 100644 --- a/packages/sidecar-toolkit/test/apply-label-edit-rename-handler.test.ts +++ b/packages/sidecar-toolkit/test/apply-label-edit-rename-handler.test.ts @@ -13,15 +13,15 @@ import 'reflect-metadata'; import { describe, expect, it } from 'vitest'; import { ApplyLabelEditOperation, type Command } from '@eclipse-glsp/server'; -import { WorkflowDiagramMetadata } from '@dialogram/shared'; +import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import { ApplyLabelEditRenameHandler } from '../src/server/operations/apply-label-edit-rename-handler'; import { RenameEntityOperation } from '../src/server/operations/rename-entity-handler'; -/** A node element carrying the entity name in `args`, plus its header label child. */ -function modelWith(nodeId: string, entityName: string, labelId: string) { +/** A node element carrying the entity name in `args`, plus one typed label child. */ +function modelWith(nodeId: string, entityName: string, labelId: string, labelType?: string) { const node: any = { id: nodeId, args: { [WorkflowDiagramMetadata.ENTITY_NAME]: entityName } }; - const label: any = { id: labelId, args: {}, parent: node }; + const label: any = { id: labelId, type: labelType, args: {}, parent: node }; node.parent = undefined; const index = { find: (id: string) => (id === labelId ? label : id === nodeId ? node : undefined) @@ -29,6 +29,20 @@ function modelWith(nodeId: string, entityName: string, labelId: string) { return { index }; } +/** The delegating handler, wired to record what it would rename. */ +function handlerFor(model: ReturnType) { + const handler = new ApplyLabelEditRenameHandler(); + (handler as any).modelState = model; + const seen: RenameEntityOperation.Operation[] = []; + (handler as any).renameHandler = { + createCommand: (op: RenameEntityOperation.Operation) => { + seen.push(op); + return { label: 'rename-command' } as unknown as Command; + } + }; + return { handler, seen }; +} + describe('ApplyLabelEditRenameHandler (MCP label-edit → reversible rename)', () => { it('exposes the protocol operation kind as a static literal (DI-less read)', () => { // The GLSP DefaultGlobalActionProvider reads `.operationType` off a `new ctor()` built @@ -84,3 +98,74 @@ describe('ApplyLabelEditRenameHandler (MCP label-edit → reversible rename)', ( ).toBeUndefined(); }); }); + +/** + * Editing a TYPE must never rename. + * + * This handler turns every label edit into a rename of the nearest ancestor carrying an entity + * name. For a boundary port's type label that ancestor is the boundary node, whose entity name is + * the PORT's name — so changing a port's type sent `renameNode { old: , new: }` and renamed the port to its own type. The property panel offers exactly that edit + * ("Change Port Type"), and the canvas label is editable too, so both routes corrupted the source. + * + * The guard is an allow-list, which is the polarity that matters: the next label type added is + * refused by default rather than silently inheriting "editing this renames the node". + */ +describe('ApplyLabelEditRenameHandler — only a NAME may rename', () => { + it('refuses a boundary port type label', () => { + const { handler, seen } = handlerFor( + modelWith('bnd:Com', 'Com', 'bnd:Com_label_type', WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE) + ); + + const command = handler.createCommand( + ApplyLabelEditOperation.create({ labelId: 'bnd:Com_label_type', text: 'RobAlloc' }) + ); + + expect(command).toBeUndefined(); + // The bug was not "wrong command" but "renamed the port to the type". + expect(seen).toEqual([]); + }); + + it('refuses an entity node type subtitle', () => { + const { handler, seen } = handlerFor( + modelWith('node:rob', 'rob', 'node:rob_subtitle', WorkflowDiagramTypes.LABEL_TYPE) + ); + + expect( + handler.createCommand(ApplyLabelEditOperation.create({ labelId: 'node:rob_subtitle', text: 'Rob' })) + ).toBeUndefined(); + expect(seen).toEqual([]); + }); + + it('still renames from a name label, on a node and on a boundary port', () => { + const entity = handlerFor( + modelWith('node:rob', 'rob', 'node:rob_label_name', WorkflowDiagramTypes.LABEL_NAME) + ); + entity.handler.createCommand( + ApplyLabelEditOperation.create({ labelId: 'node:rob_label_name', text: 'reorder' }) + ); + expect(entity.seen).toEqual([ + { kind: RenameEntityOperation.KIND, elementId: 'node:rob', newName: 'reorder' } + ]); + + const boundary = handlerFor( + modelWith('bnd:Com', 'Com', 'bnd:Com_label_name', WorkflowDiagramTypes.LABEL_BOUNDARY_NAME) + ); + boundary.handler.createCommand( + ApplyLabelEditOperation.create({ labelId: 'bnd:Com_label_name', text: 'Commit' }) + ); + expect(boundary.seen).toEqual([ + { kind: RenameEntityOperation.KIND, elementId: 'bnd:Com', newName: 'Commit' } + ]); + }); + + /** A label type nobody has classified yet is refused, not assumed to be a name. */ + it('refuses a label type it has never heard of', () => { + const { handler, seen } = handlerFor(modelWith('node:x', 'x', 'node:x_label_new', 'label:something:new')); + + expect( + handler.createCommand(ApplyLabelEditOperation.create({ labelId: 'node:x_label_new', text: 'Y' })) + ).toBeUndefined(); + expect(seen).toEqual([]); + }); +}); From 8a9af69dd07887a57ce88d45cd94d8948f5d6b5e Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 15:19:04 +0200 Subject: [PATCH 2/4] docs: spec the schema and op changes structured port types need MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writes down what the sidecars have to grow before a boundary port's type can be edited, before a product or variant can be shown, and before a type's definition can be navigated to. All three are blocked outside the platform, and both sidecars have to implement it identically or the platform behaves differently per product — so it is a spec, not a ticket. Two parts, each independently shippable and separately gated: Part A is one operation, `updateBoundaryPortType`, addressed by `workflow` and `direction` the way `createPort` and `deletePort` already address boundary ports. Proposed as a new op rather than a `workflow` variant of the existing `updatePortType`, whose `entity` is required: overloading it would make a sidecar that has not been updated read a boundary edit as an entity edit, silently, against the wrong declaration. Part B makes a port's type structured — kind, members, and a definition site per member — as an OPTIONAL field beside the existing string, which stays and remains what gets displayed. The platform must never re-render a type from its structure; that would drift from the language's own formatting and differ between products. Expansion depth is the sidecar's call, marked with `truncated`, so a recursive type cannot become an unbounded payload. Both ride the existing `getCapabilities` negotiation rather than a version bump, so neither sidecar blocks the other and neither blocks the platform. Records four open questions the sidecar authors have to answer — network addressability, default expansion depth, whether one `kind` vocabulary spans both type systems, and whether entity ports get the same treatment. --- docs/proposals/structured-port-types.md | 173 ++++++++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 docs/proposals/structured-port-types.md diff --git a/docs/proposals/structured-port-types.md b/docs/proposals/structured-port-types.md new file mode 100644 index 0000000..e3eaf04 --- /dev/null +++ b/docs/proposals/structured-port-types.md @@ -0,0 +1,173 @@ +# Proposal: structured port types + +**Status:** proposal — nothing here is implemented in any sidecar. +**Affects:** the graph export schema and the operation vocabulary, so **every** +sidecar has to implement it identically or the platform behaves differently per +product. + +## Why + +Three things users ask for are blocked in the same place, and none of them can +be fixed in the platform: + +1. **Editing a boundary port's type.** The platform has no operation that can + address a boundary port for an update. +2. **A viewer for composite types** (products and variants) in the property + panel. +3. **Go to the definition of a type**, from a port. + +(2) and (3) are blocked by one line of the export schema: + +```ts +export type PyGraphPort = { + id: string; + name: string; + direction: 'in' | 'out'; + type?: string; // ← the whole type, as text + role?: string; + source?: { file?: string; line?: number } | null; +}; +``` + +A type is opaque text. There is no structure to render and no location to +navigate to, and the platform cannot derive either: it never parses source — +that is the sidecar's entire job. + +`source` is the **port's** declaration site. It is not where the type is +defined, and it is already used for "go to source" on the port itself. + +## Part A — editing a boundary port's type + +### The gap + +Port operations already split by ownership, and address the two cases +differently: + +| | addressed by | direction key | +| --- | --- | --- | +| `createPort` (boundary) | `workflow` | `direction` | +| `deletePort` (boundary) | `workflow` | `direction` | +| `createEntityPort` | `entityType` | `portDirection` | +| `deleteEntityPort` | `entityType` | `portDirection` | +| `renamePort` | `entity` | `portDirection` | +| `updatePortType` | `entity` | `portDirection` | + +A boundary port belongs to the network, not to any entity, so `updatePortType` +cannot name it. `createPort` already writes a boundary port's type — the +capability exists in the sidecar; only the update path is missing. + +### Proposed operation + +``` +updateBoundaryPortType + args: { workflow: string, direction: 'input' | 'output', + portName: string, newValue: string } +``` + +Addressed the way the other boundary port ops are. A separate op rather than a +`workflow` variant of `updatePortType`, because that op's `entity` is required +today and overloading it would make an un-updated sidecar interpret a boundary +edit as an entity edit — silently, against the wrong declaration. + +Renaming a boundary port needs nothing new: it already rides `renameNode`. + +## Part B — structured types + +### Proposed schema + +Additive and optional. A sidecar that has not implemented it emits nothing new, +and the platform behaves exactly as it does today. + +```ts +export type PyTypeRef = { + /** Rendered form, in the language's own formatting. ALWAYS present. */ + text: string; + kind?: 'primitive' | 'alias' | 'product' | 'variant' | 'list' | 'unknown'; + /** The declared name, when it has one. */ + name?: string; + /** Where the type is DEFINED. Not the port's declaration site. */ + source?: { file?: string; line?: number } | null; + /** Fields of a product; cases of a variant. Omitted for other kinds. */ + members?: Array<{ + name: string; + type?: PyTypeRef; + source?: { file?: string; line?: number } | null; + }>; + /** Set when expansion stopped here rather than bottoming out. */ + truncated?: boolean; +}; + +export type PyGraphPort = { + // ...unchanged... + type?: string; // unchanged; stays the display fallback + typeRef?: PyTypeRef; // NEW, optional +}; +``` + +### Why it is shaped this way + +**`text` is mandatory and `type` stays.** The platform must never re-render a +type from its structure — it would drift from how the language actually writes +it, and differ between products. The sidecar formats; the platform displays. +Keeping `type` means the change cannot regress an existing diagram. + +**Expansion is bounded by the sidecar, not the platform.** A recursive type +would otherwise be an unbounded payload on every graph export. The sidecar +expands to whatever depth it judges reasonable and sets `truncated: true` where +it stopped; the platform shows an affordance that *navigates* rather than +expands, so depth is never the platform's problem. + +**`kind` is open and `'unknown'` is legal.** A type the sidecar cannot resolve +must still round-trip its `text`. Partial information is expected — a graph +export is already allowed to be `partial`. + +**`source` on each member.** Navigating to one field of a product is the useful +case; a viewer that can only reach the whole type is much less so. + +### What the platform would do with it + +| field | used for | +| --- | --- | +| `text` / `type` | the chip shown today — unchanged | +| `kind` + `members` | the property-panel viewer for products and variants | +| `source` | "Go to definition" on the type, and on each member | +| `truncated` | render "…" as a navigation affordance, not an expander | + +Absent `typeRef`, the panel renders exactly what it renders now. + +## Capability gating + +Both parts should be gated through the existing negotiation rather than +version-bumped: `getCapabilities` returns `ops: string[]`, and the platform +already has `supportsOp(uri, kind)`. + +- **Part A:** the panel offers the type edit only when + `updateBoundaryPortType` is advertised. Otherwise the field stays read-only, + which is what it is today. +- **Part B:** needs no gate. `typeRef` is optional and its absence is the + current behaviour. + +This means neither sidecar blocks the other, and neither blocks the platform. + +## Open questions for the sidecar authors + +1. Is a network's interface addressable by `workflow` name alone, or is a file + with several networks ambiguous in a way `createPort` gets away with today + only because it appends? +2. What is a sensible default expansion depth — is one level enough to be + useful for the common product type? +3. Do the two products' type systems agree closely enough for one `kind` + vocabulary, or does it need a product-specific escape hatch? +4. Should `typeRef` also be emitted for entity ports? The schema change is on + `PyGraphPort`, so it comes for free — but only if the resolver is available + on that path too. + +## What is already done in the platform + +Neither of these waits on a sidecar: + +- A boundary port type edit no longer **renames the port**. Every label edit was + routed to a rename of the nearest entity; for a type label that resolved to + the port itself. The handler now allow-lists name labels. +- Boundary ports carry navigation metadata, so "go to source" reaches the port's + declaration. This is the port, not its type — Part B is what adds the type. From 49a7556d3e5292406abb60f66f01e0d3947124aa Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 15:46:20 +0200 Subject: [PATCH 3/4] fix(views): keep the boundary type label out of the layout engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making the type label non-editable by pointing it at the generic `WorkflowLabel` moved it: it rendered small, in the node's top-left corner, outside the rounded box. `WorkflowLabelView` positions boundary labels itself, relative to the parent node and centred on the parent's width. That only works while the labels are invisible to the layout engine, which is why `BoundaryEditableLabel` REPLACES sprotty's default label features rather than adding to them — those defaults include `boundsFeature`, `alignFeature` and `layoutableChildFeature`. `WorkflowLabel` declares no features of its own, so pointing at it handed the label back to the layout engine, which duly laid it out. So the non-editable variant is its own class whose empty feature list is the entire point, rather than a reach for the nearest label class that happened to lack edit support. The trap here is inheritance, so that is what the tests assert: each boundary label declares its OWN feature list, and its EFFECTIVE list — resolved the way sprotty resolves it, falling back to sprotty's defaults when a class declares none — carries no layout feature. Modelling the fallback is what makes the second assertion real: reading only the own list, it passed while the class inherited every layout feature there is. The real sprotty defaults are read past the vitest stub, whose empty ones would have made the whole file vacuous. --- packages/diagram-client/src/model.ts | 21 +++++ .../diagram-client/src/stock-views.module.ts | 7 +- .../test/boundary-label-features.test.ts | 80 +++++++++++++++++++ .../test/container-parity.test.ts | 2 +- .../fixtures/container-parity.baseline.json | 12 +-- 5 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 packages/diagram-client/test/boundary-label-features.test.ts diff --git a/packages/diagram-client/src/model.ts b/packages/diagram-client/src/model.ts index dc8609a..4d1d75b 100644 --- a/packages/diagram-client/src/model.ts +++ b/packages/diagram-client/src/model.ts @@ -446,6 +446,27 @@ export class BoundaryEditableLabel extends WorkflowLabel { editControlDimension = { width: 100, height: 20 }; } +/** + * A boundary label that cannot be edited — the type, which has no write path yet. + * + * The empty feature list is the whole point of this class, and the reason it is + * not just `WorkflowLabel`. Sprotty's default label features include + * `boundsFeature`, `alignFeature` and `layoutableChildFeature`, and a boundary + * label must have NONE of them: `WorkflowLabelView` positions these labels + * itself, relative to the parent node, centring the text on the node's own + * width. Give the label layout features back and the node's layout engine + * claims it instead — it lands in the top-left corner, small, outside the + * rounded box. That is exactly what happened when this was first made + * non-editable by pointing it at `WorkflowLabel`. + * + * So: same (absent) features as {@link BoundaryEditableLabel}, minus the two + * edit ones. Not selectable either, for the reason described above it — a + * selectable label swallows the drag that should move the port. + */ +export class BoundaryLabel extends WorkflowLabel { + static override readonly DEFAULT_FEATURES = []; +} + /** * CAL Compartment for grouping elements */ diff --git a/packages/diagram-client/src/stock-views.module.ts b/packages/diagram-client/src/stock-views.module.ts index f993892..bcfc6b3 100644 --- a/packages/diagram-client/src/stock-views.module.ts +++ b/packages/diagram-client/src/stock-views.module.ts @@ -45,6 +45,7 @@ import { WorkflowEdge, WorkflowLabel, BoundaryEditableLabel, + BoundaryLabel, HeaderCompartment, PortsCompartment } from './model'; @@ -129,8 +130,12 @@ export const workflowViewsModule = new ContainerModule((bind, unbind, isBound, r // Making the type editable for real needs a sidecar op that addresses a // boundary port by `workflow` (the way `createPort` already does) rather // than by owning `entity`, which is what `updatePortType` requires today. + // + // `BoundaryLabel`, NOT the generic `WorkflowLabel`: boundary labels must + // carry no layout features, or the node's layout engine positions them + // instead of the view and the type lands in the top-left corner. configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_NAME, BoundaryEditableLabel, WorkflowLabelView); - configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE, WorkflowLabel, WorkflowLabelView); + configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE, BoundaryLabel, WorkflowLabelView); // Port labels configureModelElement(context, WorkflowDiagramTypes.LABEL_PORT, WorkflowLabel, WorkflowLabelView); diff --git a/packages/diagram-client/test/boundary-label-features.test.ts b/packages/diagram-client/test/boundary-label-features.test.ts new file mode 100644 index 0000000..4b8ba6e --- /dev/null +++ b/packages/diagram-client/test/boundary-label-features.test.ts @@ -0,0 +1,80 @@ +/** + * Boundary labels must not participate in layout. + * + * `WorkflowLabelView` positions a boundary node's name and type itself, relative + * to the parent node and centred on the parent's width. That only works while + * the labels are invisible to the layout engine — so both boundary label classes + * REPLACE sprotty's default label features, which include `boundsFeature`, + * `alignFeature` and `layoutableChildFeature`. + * + * This has already been broken once. Making the type label non-editable by + * pointing it at the generic `WorkflowLabel` looked like a pure capability + * change, but `WorkflowLabel` declares no features of its own and therefore + * inherits sprotty's — so the layout engine claimed the label and parked it in + * the node's top-left corner, small and outside the rounded box. Nothing failed; + * it only looked wrong. + * + * The trap is inheritance, so that is what these assert: each boundary label + * declares its OWN feature list, and that list contains no layout feature. The + * real sprotty defaults are read through `createRequire` — the vitest alias + * points `@eclipse-glsp/sprotty` at a stub, and asserting against the stub's + * (empty) defaults would pass no matter what. + */ +import { describe, expect, it } from 'vitest'; +import { createRequire } from 'node:module'; +import { BoundaryEditableLabel, BoundaryLabel, WorkflowLabel } from '../src/model'; + +const { SLabelImpl } = createRequire(import.meta.url)('sprotty/lib/graph/sgraph'); + +/** Feature symbols compared by description: the stub's and sprotty's differ by identity. */ +const describeAll = (features: readonly symbol[] | undefined): string[] => + (features ?? []).map(f => String(f)); + +const LAYOUT_FEATURES = ['Symbol(boundsFeature)', 'Symbol(alignFeature)', 'Symbol(layoutableChildFeature)']; + +describe('boundary label features', () => { + /** If sprotty ever stopped defaulting labels into layout, this whole concern would be moot. */ + it("sprotty's own label defaults are the hazard", () => { + const defaults = describeAll(SLabelImpl.DEFAULT_FEATURES); + + for (const feature of LAYOUT_FEATURES) { + expect(defaults, `sprotty labels no longer default to ${feature}`).toContain(feature); + } + }); + + it.each([ + ['BoundaryLabel', BoundaryLabel], + ['BoundaryEditableLabel', BoundaryEditableLabel] + ])('%s declares its own features rather than inheriting', (name, cls) => { + // The inheritance trap: a class that does not declare DEFAULT_FEATURES + // silently takes sprotty's, layout features and all. + expect( + Object.prototype.hasOwnProperty.call(cls, 'DEFAULT_FEATURES'), + `${name} must declare DEFAULT_FEATURES, not inherit them` + ).toBe(true); + }); + + it.each([ + ['BoundaryLabel', BoundaryLabel], + ['BoundaryEditableLabel', BoundaryEditableLabel] + ])('%s carries no layout feature', (name, cls) => { + // EFFECTIVE features, the way sprotty resolves them: a class without its + // own list gets sprotty's. Reading only the own list would make this + // pass for a class that inherits every layout feature there is — which + // is precisely the regression, so it has to be modelled here. + const own = (cls as { DEFAULT_FEATURES?: symbol[] }).DEFAULT_FEATURES; + const effective = describeAll(own ?? SLabelImpl.DEFAULT_FEATURES); + + for (const feature of LAYOUT_FEATURES) { + expect(effective, `${name} would be positioned by the layout engine`).not.toContain(feature); + } + }); + + /** + * The class the regression reached for. Pinning that it is NOT safe here is + * what makes the distinction visible to the next person. + */ + it('the generic WorkflowLabel is not a valid boundary label', () => { + expect(Object.prototype.hasOwnProperty.call(WorkflowLabel, 'DEFAULT_FEATURES')).toBe(false); + }); +}); diff --git a/packages/diagram-client/test/container-parity.test.ts b/packages/diagram-client/test/container-parity.test.ts index 3163b01..98576a7 100644 --- a/packages/diagram-client/test/container-parity.test.ts +++ b/packages/diagram-client/test/container-parity.test.ts @@ -18,7 +18,7 @@ * two additions — `IEdgeRouter -> LibavoidEdgeRouter` (the client-side live * routing tier) and `ChangeBoundsTool -> WorkflowChangeBoundsTool` (the * mouse-drag threshold) — plus one change: `label:boundary:type` is bound to - * the non-editable `WorkflowLabel`, because editing it renamed the port. + * the non-editable `BoundaryLabel`, because editing it renamed the port. * The oracle still does its job: it fails on any binding this composition gains * or loses, and updating the fixture is the deliberate act of accepting one. * Regenerate it only after confirming the diff contains exactly the bindings the diff --git a/packages/diagram-client/test/fixtures/container-parity.baseline.json b/packages/diagram-client/test/fixtures/container-parity.baseline.json index da68062..fd1b55b 100644 --- a/packages/diagram-client/test/fixtures/container-parity.baseline.json +++ b/packages/diagram-client/test/fixtures/container-parity.baseline.json @@ -197,6 +197,12 @@ "typeId": "node:boundary:input", "view": "BoundaryInputNodeView" }, + { + "model": "BoundaryLabel", + "op": "modelElement", + "typeId": "label:boundary:type", + "view": "WorkflowLabelView" + }, { "model": "BoundaryOutputNode", "op": "modelElement", @@ -341,12 +347,6 @@ "typeId": "label:badge", "view": "GLabelView" }, - { - "model": "WorkflowLabel", - "op": "modelElement", - "typeId": "label:boundary:type", - "view": "WorkflowLabelView" - }, { "model": "WorkflowLabel", "op": "modelElement", From b9cb8f01c77341f8a3fb7a476e488ff411ada833 Mon Sep 17 00:00:00 2001 From: Endri Bezati Date: Mon, 31 Aug 2026 16:02:16 +0200 Subject: [PATCH 4/4] fix(model): resolve boundary port source the way every other node does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The boundary navigation added earlier is inert: it read `port.source`, the typed field the schema declares on a port, and neither product populates it. That was the wrong field to reach for. Entity nodes do not use it either — every one of them resolves navigation from `node.meta.source`, and `node.meta.referencedSource` for the definition. A boundary node is a PyGraphNode like any other and has the same `meta`, so the convention already in use across the graph was available and went unread. Both are now checked, preferring meta so a boundary node behaves like the nodes beside it, falling back to the typed port field so a product that fills in what the schema declares is also served. Requiring neither specifically matters because the platform cannot make either product change. Whether this lights up still depends on a product emitting one of them for a boundary node, which as far as can be told neither does yet. So the proposal gains a Part C: populate `meta.source` on a boundary node with the port's declaration site, exactly as entity nodes already do. It needs no new op and no schema change — `meta` is an open bag and the field name is the one in use — which makes it much the cheapest of the three parts. The proposal's "already done" section is corrected accordingly. It claimed boundary ports were navigable; they are navigable only once something supplies a location, and saying otherwise would have sent someone looking for a feature that cannot appear. --- docs/proposals/structured-port-types.md | 35 +++++++++++-- .../src/model/graph-gmodel-source.ts | 31 +++++++---- .../test/boundary-port-navigation.test.ts | 51 ++++++++++++++++--- 3 files changed, 96 insertions(+), 21 deletions(-) diff --git a/docs/proposals/structured-port-types.md b/docs/proposals/structured-port-types.md index e3eaf04..1b63c3f 100644 --- a/docs/proposals/structured-port-types.md +++ b/docs/proposals/structured-port-types.md @@ -16,6 +16,10 @@ be fixed in the platform: panel. 3. **Go to the definition of a type**, from a port. +A fourth turned up while investigating them: a boundary port has no source +location at all, so "Go to Source" does not even reach the port, let alone its +type. That one is much smaller — see Part C. + (2) and (3) are blocked by one line of the export schema: ```ts @@ -135,6 +139,28 @@ case; a viewer that can only reach the whole type is much less so. Absent `typeRef`, the panel renders exactly what it renders now. +## Part C — a source location for boundary ports + +Smaller than the other two, and possibly already satisfied. + +"Go to Source" resolves from navigation metadata on the element. Every entity +node gets it from `node.meta.source`; an entity's ports get it from the typed +`port.source`. A boundary node read neither — it took `name` and `type` off its +port and nothing else — so a network's own inputs and outputs were the one kind +of port with no navigation at all. + +The platform now reads **both**, preferring `node.meta.source` (so a boundary +node behaves like the nodes beside it) and falling back to `port.source`. It +requires neither specifically, because the platform cannot make a product +change. But it does require **one of them to be populated**, and as of writing +neither product appears to emit either for a boundary node — the feature is +wired and inert. + +So: for a boundary node (`kind: 'wf-input'` / `'wf-output'`), populate +`meta.source = { file, line }` with the port's declaration site, exactly as +entity nodes already do. Nothing else is needed; no new op, no schema change — +`meta` is already an open bag and the field name is the one in use. + ## Capability gating Both parts should be gated through the existing negotiation rather than @@ -164,10 +190,9 @@ This means neither sidecar blocks the other, and neither blocks the platform. ## What is already done in the platform -Neither of these waits on a sidecar: - - A boundary port type edit no longer **renames the port**. Every label edit was routed to a rename of the nearest entity; for a type label that resolved to - the port itself. The handler now allow-lists name labels. -- Boundary ports carry navigation metadata, so "go to source" reaches the port's - declaration. This is the port, not its type — Part B is what adds the type. + the port itself. The handler now allow-lists name labels. This needs no + sidecar work and is the one change here with immediate effect. +- Boundary nodes emit navigation metadata when a source location is available — + see Part C for why that is currently never. diff --git a/packages/diagram-server/src/model/graph-gmodel-source.ts b/packages/diagram-server/src/model/graph-gmodel-source.ts index cefebb7..c1e4ad8 100644 --- a/packages/diagram-server/src/model/graph-gmodel-source.ts +++ b/packages/diagram-server/src/model/graph-gmodel-source.ts @@ -1046,16 +1046,29 @@ export class GraphGModelSource { }; const typeText = port?.type ?? ''; - // Where the port is declared, so "Go to source" works on a boundary node - // the same way it already does on an entity's port (see `mkPort`). This - // was simply never wired: the schema carries `port.source`, and the - // boundary node read only `name` and `type` off the same object. + // Where this port is declared, so "Go to Source" works on a boundary node + // the way it already does on every other node. It was never wired at + // all: the boundary node took `name` and `type` off its port and read + // nothing else. // - // NOTE this is the PORT's declaration site, not its type's definition — - // navigating to where a type is defined needs a field the graph schema - // does not yet have. - const boundarySourceFile = normalizeNavigationFileUri(port?.source?.file); - const boundarySourceLine = port?.source?.line; + // Two places carry it, and BOTH are checked because the products differ + // on which they fill in. `node.meta.source` is the established one — + // every entity node above resolves its navigation from exactly that, so + // a sidecar already emitting node source needs no change at all. + // `port.source` is the typed field the schema declares on a port, used + // by an entity's ports in `mkPort`. Preferring meta means a boundary node + // behaves like its neighbours; falling back to the port keeps faith with + // the declared schema. + // + // NOTE this reaches the PORT's declaration, not its type's definition — + // navigating to where a type is defined needs a field the schema does + // not have. See docs/proposals/structured-port-types.md. + const boundaryMetaSource = node.meta?.['source'] as { file?: string; line?: number } | undefined; + const boundarySource = boundaryMetaSource?.file && boundaryMetaSource?.line + ? boundaryMetaSource + : port?.source ?? undefined; + const boundarySourceFile = normalizeNavigationFileUri(boundarySource?.file); + const boundarySourceLine = boundarySource?.line; const boundaryNavigation = boundarySourceFile && boundarySourceLine ? (() => { const at = { diff --git a/packages/diagram-server/test/boundary-port-navigation.test.ts b/packages/diagram-server/test/boundary-port-navigation.test.ts index 57bf689..cbd330e 100644 --- a/packages/diagram-server/test/boundary-port-navigation.test.ts +++ b/packages/diagram-server/test/boundary-port-navigation.test.ts @@ -17,8 +17,16 @@ import * as path from 'node:path'; import { WorkflowDiagramMetadata } from '@dialogram/shared'; import { GraphGModelSource, type PyGraphDocument } from '../src/model/graph-gmodel-source'; -/** A one-node graph whose only content is a boundary input port. */ -function docWithBoundaryInput(source?: { file?: string; line?: number }): PyGraphDocument { +type At = { file?: string; line?: number }; + +/** + * A one-node graph whose only content is a boundary input port. + * + * `meta` and `port` are separate because the two places a product can put the + * location are the whole point: `node.meta.source` is what every other node + * resolves navigation from, `port.source` is what the schema declares on a port. + */ +function docWithBoundaryInput(at?: { meta?: At; port?: At }): PyGraphDocument { return { version: '1', graph: { @@ -29,13 +37,14 @@ function docWithBoundaryInput(source?: { file?: string; line?: number }): PyGrap kind: 'wf-input', label: 'Alloc', scope: 'root', + meta: at?.meta ? { source: at.meta } : undefined, ports: [ { id: 'port:wf:input:Alloc:out', name: 'Alloc', direction: 'out', type: 'RobAlloc', - source + source: at?.port } ] } @@ -56,8 +65,8 @@ function boundaryNodeOf(doc: PyGraphDocument): any { } describe('boundary port navigation metadata', () => { - it('carries the port declaration site through to the model', () => { - const node = boundaryNodeOf(docWithBoundaryInput({ file: '/w/pipeline.py', line: 12 })); + it('resolves the declaration site from node meta, as every other node does', () => { + const node = boundaryNodeOf(docWithBoundaryInput({ meta: { file: '/w/pipeline.py', line: 12 } })); // Line numbers arrive 1-based from the sidecar and are stored 0-based, // matching what the entity ports emit. @@ -70,7 +79,7 @@ describe('boundary port navigation metadata', () => { }); it('leaves the port identity alone', () => { - const node = boundaryNodeOf(docWithBoundaryInput({ file: '/w/pipeline.py', line: 12 })); + const node = boundaryNodeOf(docWithBoundaryInput({ meta: { file: '/w/pipeline.py', line: 12 } })); expect(node.args[WorkflowDiagramMetadata.PORT_NAME]).toBe('Alloc'); expect(node.args[WorkflowDiagramMetadata.PORT_TYPE]).toBe('RobAlloc'); @@ -90,8 +99,36 @@ describe('boundary port navigation metadata', () => { }); it('emits nothing navigable when the source has a file but no line', () => { - const node = boundaryNodeOf(docWithBoundaryInput({ file: '/w/pipeline.py' })); + const node = boundaryNodeOf(docWithBoundaryInput({ meta: { file: '/w/pipeline.py' } })); expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBeUndefined(); }); + + /** + * The schema declares `source` on the port, so a product that fills that in + * instead of node meta must work too — the point is to require neither + * specifically, since the platform cannot make either product change. + */ + it('falls back to the port when node meta carries no source', () => { + const node = boundaryNodeOf(docWithBoundaryInput({ port: { file: '/w/pipeline.py', line: 5 } })); + + expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBe( + URI.file(path.resolve('/w/pipeline.py')).toString() + ); + expect(node.args[WorkflowDiagramMetadata.SOURCE_RANGE]).toEqual({ + start: { line: 4, character: 0 }, + end: { line: 4, character: 0 } + }); + }); + + /** Node meta wins, so a boundary node navigates like the nodes beside it. */ + it('prefers node meta over the port when both are present', () => { + const node = boundaryNodeOf( + docWithBoundaryInput({ meta: { file: '/w/net.py', line: 12 }, port: { file: '/w/other.py', line: 5 } }) + ); + + expect(node.args[WorkflowDiagramMetadata.REFERENCED_URI]).toBe( + URI.file(path.resolve('/w/net.py')).toString() + ); + }); });