fix: missing subgraph contents lost when moved

[camielvs]

Description

Two bug fixes, both cases where a pipeline quietly lost part of itself.

1. A subgraph inside a subgraph was emptied when you moved the task holding it

If a subgraph contained another subgraph, two everyday actions destroyed the inner one:

• Unpacking the outer subgraph
• Selecting the outer subgraph together with another task and grouping them into a new subgraph

Afterwards the inner subgraph task was still sitting on the canvas with its name and position, but it was hollow — double-clicking no longer opened it, and its ports were gone. Nothing warned you; you only found out when you went looking inside.

Both actions now bring the nested contents along with the task. Copy/paste already handled this correctly — these two paths simply weren't doing the same thing.

2. Connections to ports that don't exist were accepted

It was possible to end up with a connection that looked fine on the canvas but disappeared the moment the pipeline was saved, because it pointed at a port the component doesn't actually have, or ran the wrong way (out of a pipeline output, or into a pipeline input). Nothing flagged it, so the pipeline would happily submit and then behave as if the value had never been connected.

These are now caught in two places:

• The pipeline validator reports them as errors, with a "Delete Connection" fix in the issues panel — so imported or hand-edited YAML gets checked too.
• The AI assistant refuses to create one, and tells you which ports the task really has so it can correct itself.

Dragging on the canvas could never produce these, so in practice they came from the AI assistant or from imported YAML.

Related Issue and Pull requests

Follows up on the two review comments left on #2684 that were deferred out of that stack.

Type of Change

• Bug fix
• New feature
• Improvement
• Cleanup/Refactor
• Breaking change
• Documentation update

Checklist

• I have tested this does not break current pipelines / runs functionality
• I have tested the changes on staging

Test Instructions

Nested subgraphs (fix 1)

1. Build a pipeline with a subgraph nested two deep: add two tasks, group them into a subgraph, then select that subgraph plus one more task and group again.
2. Double-click down through both levels to confirm the inner subgraph opens and has its contents.
3. Back at the top level, unpack the outer subgraph.
4. The inner subgraph task should still open on double-click, with its tasks and ports intact. Before this change it opened empty.
5. Undo, then try the other route: shift-select the outer subgraph plus another task and choose Create Subgraph. Look inside the new subgraph — the nested one should still have its contents.

Connections (fix 2)

1. With the AI assistant on, ask it to connect two tasks using a port name that doesn't exist ("connect Train's foo output to Evaluate"). It should refuse and list the real port names rather than creating a connection that goes nowhere.
2. Import a YAML file with a connection pointing at a port its component doesn't declare. The editor should now show an error for it with a Delete Connection action, instead of letting the pipeline submit.

Additional Comments

Regression tests were added for both fixes, and each new test was confirmed to fail without its fix. The full unit suite passes (2362 tests).

The new connection check is error-severity, which blocks submission, so all 16 shipped example and tour pipelines were checked against it — 95 connections, no new errors. Nothing that ships starts showing problems.

The check deliberately only looks at connections between tasks. Connections to a pipeline input or output are named after the input or output itself when saved, so renaming one leaves a harmless leftover name behind that would otherwise get flagged as broken.

[github-actions]

🎩 Preview

A preview build has been created at: 09-04-fix_subgraph_contents_lost_when_moved/8dda8ef

[morgan-wowk]

🤖 Automated review

Reviewed both fixes against the head; the design is right and I traced the parts that could bite.

Fix 1 — nested contents preserved on move (verified correct). The switch from t.componentRef to t.resolvedComponentRef in snapshotTask/addInnerTasks, then routing through promoteInlineSubgraph, is exactly the right seam:

• resolvedComponentRef returns this.componentRef verbatim for a non-subgraph and only serializes the live subgraphSpec for a subgraph — so ordinary library/container tasks are untouched (no accidental spec-inlining), and a subgraph task now carries the contents that previously lived only in the runtime subgraphSpec and never made it into componentRef.spec. That missing serialization is precisely why the inner graph came out hollow.
• promoteInlineSubgraph gates on isGraphImplementation(ref.spec.implementation), so it promotes a real nested subgraph into a live subgraphSpec and leaves everything else as a plain ref. Both new tests assert the inner task, its position, and that the promoted task is a subgraph with componentRef.spec stripped — good.

Fix 2 — binding-endpoint validation (verified correct, incl. the risky edges). One shared findBindingEndpointProblems feeding both the validator and the CSOM bridge is the right call — the agent is refused on exactly the grounds the validator would report. The edges that could produce false errors all hold:

• Silent while ports are unknown: non-subgraph resolvedComponentSpec is componentRef.spec, which is undefined until the component resolves, so the sourcePorts &&/targetPorts && guards short-circuit and an unresolved library task is never flagged. (Tested.)
• Aggregator: createAggregatorInput yields a real InputSpec (agg_N) that lands in the spec's inputs, so legitimate aggregator connections resolve and pass; only the synthetic __add_aggregator_input__ handle — never a real port — is caught. (Tested.)
• Direction checks fire only on genuinely reversed connections: source-is-output matches only when the source id is a pipeline output, target-is-input only when the target id is a pipeline input; the valid task→output and input→task shapes don't match either .find. And IS_ENABLED_PORT_NAME is whitelisted on targets.
• No double-report with the orphaned-binding check: those require the entity id to be absent, while these require it to be present, so they're mutually exclusive per endpoint.

INVALID_BINDING_SOURCE/TARGET wire to a "Delete Connection" resolution, and the error-severity choice is justified (a bad endpoint serializes to nothing, so warning wouldn't prevent the silent data loss). Nice touch validating all 16 shipped pipelines against the new error before shipping it.

NIT (test gap, not a defect)

Fix 1's promote path round-trips through serializeComponentSpec(subgraphSpec) → YamlDeserializer.deserialize, and both regression tests assert the inner tasks and a position survive — but not the inner bindings (a connection between two nested tasks). Since the bug was "contents silently lost," a nested task→task binding surviving the group/unpack is the other half of "contents," and it's the part that leans on the serializer round-trip rather than a direct field copy. Low risk (that serializer is heavily exercised elsewhere), but adding one binding to the nested fixture and asserting it's still there would fully close the regression.

Approving — both fixes are correct and the coverage is strong; the note above is a nice-to-have.