>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3620613. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !424 >>> <h3>Problem</h3> <p>The modeler round-trip is meant to be lossless in both directions. These findings are the places it is not: an edit that is silently dropped on the way in, captured positions that are never read back, and a contrib update path that bypasses the preservation mechanism entirely.</p> <p>12 findings: 3 high, 3 med, 6 low.</p> <h3>High severity</h3> <h4>Clearing every assignee in a canvas modeler is silently discarded</h4> <p><strong>Where:</strong> <code>modules/orchestra_modeler/src/Plugin/ModelerApiModelOwner/Orchestra.php:564-567</code> and <code>:501-513</code></p> <p>The node-feature fold is gated on a superseded flat key being <em>present</em> in the submitted config, but <code>addComponent()</code> has already filtered out every empty value, so an emptied flat field can never reach the gate. <code>Assignments::fromConfig()</code> is therefore never called for the "the author cleared the field" case, and <code>finalizeAddingComponents()</code> restores the old assignments list instead. The round-trip carries additions and changes but not deletions, so unassigning a task in the diagram editor looks like it worked and did not: the step keeps notifying and admitting the person just removed.</p> <h4>Captured flow-label and node-label positions are never restored: the JS reads camelCase keys the PHP never ships</h4> <p><strong>Where:</strong> <code>modules/orchestra_bpmn_io/js/layout_restore.js:129,135,136</code> and <code>:150,156,157</code> against <code>modules/orchestra_bpmn_io/src/Hook/OrchestraBpmnIoHooks.php:75,109,159</code> and <code>modules/orchestra_bpmn_io/src/Plugin/ModelerApiModeler/LayoutAwareBpmnIo.php:121</code></p> <p>The capture writes <code>flow_labels</code> and <code>node_labels</code>, and those names are used consistently on the PHP side: the hook, its own documented return shape, the entity property and the interface. The attach then hands the stored array to <code>drupalSettings</code> verbatim, and Drupal performs no key transformation. The JS reads <code>layout.flowLabels</code> and <code>layout.nodeLabels</code>, which are therefore always undefined, so both restore loops iterate nothing. The sibling loops over nodes and flows work because those names match on both sides. Net effect: a dragged label's position is written into every workflow's stored layout on every save, growing the config, and is never read back; the label snaps to the default placement on reopen. That is a direct hole in the lossless-layout contract, and the data needed to satisfy it is already there.</p> <h4>The preservation mechanism is bypassed entirely by <code>modeler_api</code>'s own update path: node extras dropped and flows duplicated</h4> <p><strong>Where:</strong> <code>modules/orchestra_modeler/src/Plugin/ModelerApiModelOwner/Orchestra.php:429-515</code>, against contrib <code>modeler_api/src/Update.php:60-113</code></p> <p>Orchestra parks all non-carried node data in its preserved-extras and preserved-routing properties inside <code>resetComponents()</code> and only ever restores it in <code>finalizeAddingComponents()</code>. One of the API's two save paths never calls that method, and the other never calls the reset either, so the preservation is not robust to the very API it implements: every preserved node extra (timers, timeout, retry, assignments, status) and every join/split settings block is dropped, and the second branch re-adds nodes with no reset at all, leaving duplicate flows. The editor's own save path is careful by comparison, skipping the finalize on a dry run and when errors are present. Fixing this in orchestra means making the restoration self-contained rather than depending on a hook another project may not call, which is what shipped; orchestra no longer depends on that hook at all.</p> <h3>The rest</h3> <p>One line each: what it is, and where. The failing scenario and the fix for each are carried by its own commit.</p> <ul> <li><em>med</em> An integer-typed setting is re-typed to a string by a bpmn_io round-trip. <code>Orchestra.php:703-729</code> and <code>:548-552</code></li> <li><em>med</em> The running-instance warning never fires on the per-modeler canvas routes. <code>modules/orchestra_modeler/src/EventSubscriber/RunningInstanceWarningSubscriber.php:33-36</code></li> <li><em>med</em> The warning's own text contradicts the engine's shipped default. Same file, <code>:20-24</code> and <code>:90-94</code></li> <li><em>low</em> <code>Workflow::$layout</code>'s docblock names a waypoint key the capture never writes. <code>src/Entity/Workflow.php:135</code></li> <li><em>low</em> The "round-trips losslessly" contract test cannot fail on a re-type or a flow reorder. <code>modules/orchestra_modeler/tests/src/Kernel/ModelOwnerTest.php:123-124</code> and <code>:461-476</code></li> <li><em>low</em> The gateway healer's plugin-id lookup uses a descendant axis, not "only a property of this element". <code>modules/orchestra_modeler/src/Hook/OrchestraModelerWorkflowHooks.php:85-96</code></li> <li><em>low</em> <code>ModelOwnerTest::testPalette()</code>'s docblock states the opposite of its own assertion. Same test file, <code>:52-63</code></li> <li><em>low</em> The flat-config fold leaves an empty config map where the same node authored elsewhere has no config key at all. <code>Orchestra.php:510-511</code></li> <li><em>low</em> A dead branch in the modeler-info alter. <code>modules/orchestra_bpmn_io/src/Hook/OrchestraBpmnIoHooks.php:37-39</code></li> </ul> <h3>Corrections made while implementing</h3> <p>Two of the findings above were wrong as filed, and are corrected here rather than only in the commits.</p> <ul> <li>The dead branch in the modeler-info alter is the <strong>object</strong> branch, not the array one. The Modeler API discovers modelers from an attribute that does not override <code class="language-php">get()</code>, so every definition reaching an info alter is an array and the <code class="language-php">setClass()</code> call could never have run. The array write is what has been doing the work all along.</li> <li>The integer re-type is real but narrower than described. Config schema casts on save, so a setting whose schema declares a type was already repaired at the config write. What actually saw the wrong type was everything between the rebuild and that write, the presave hooks and the API's own validation, plus any node setting that declares no type and falls back to the generic string.</li> </ul> <p>This issue summary was drafted with the assistance of an AI agent (Claude). The analysis and the wording were reviewed by me before posting, and accountability for the content is mine.</p>