fix(properties): stop a boundary port type edit from renaming the port

docs/proposals/structured-port-types.md

@@ -0,0 +1,198 @@
+# 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.
+
+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
+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.
+
+## 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
+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
+
+- 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. 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.

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
  */

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';

packages/diagram-client/src/stock-views.module.ts

@@ -45,6 +45,7 @@ import {
     WorkflowEdge,
     WorkflowLabel,
     BoundaryEditableLabel,
+    BoundaryLabel,
     HeaderCompartment,
     PortsCompartment
 } from './model';
@@ -117,9 +118,24 @@ 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.
+    //
+    // `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, BoundaryEditableLabel, WorkflowLabelView);
+    configureModelElement(context, WorkflowDiagramTypes.LABEL_BOUNDARY_TYPE, BoundaryLabel, WorkflowLabelView);
 
     // Port labels
     configureModelElement(context, WorkflowDiagramTypes.LABEL_PORT, WorkflowLabel, WorkflowLabelView);

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);
+    });
+});

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 `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