Skip to content

fix(bpmn): keep subprocess bodies through the document PUT - #8077

Merged
sfmskywalker merged 2 commits into
mainfrom
claude/bpmn-8072-nested-document
Sep 12, 2026
Merged

sfmskywalker merged 2 commits into
mainfrom
claude/bpmn-8072-nested-document

Conversation

@sfmskywalker

Copy link
Copy Markdown
Member

Closes #8072. Part of #7909.

Problem

PUT bpmn/definitions/{id}/document wrote the posted BpmnDefinitions through BpmnXmlWriter.Write(document) with no bindings. In Bpmn.Interchange 0.2.0, the bodies of subprocesses, transactions and event subprocesses live only in the reader's NestedProcess bindings, so every PUT of a document declaring one emptied it. On a second PUT, an emptied event subprocess disappeared entirely.

Fix (option 2 from the issue)

The PUT now re-reads the stored Bpmn:SourceXml. For each subprocess-type element the posted document still declares, matched by element id, it writes that element's stored body and every stored binding inside that scope, recursively, under the posted element's bindingRef. The call is BpmnXmlWriter.Write(definitions, bindings).

  • A subprocess the posted document removes, or retypes to something that is not a subprocess, stays gone. No stored body is brought back.
  • Matching by element id means a new element that reuses an old bindingRef inherits nothing.
  • Every binding in a kept scope is reused, not only NestedProcess. Otherwise a nested call activity's vw:waitForCompletion="false", which exists only on its CallProcess binding, would silently turn into a waiting call.
  • A subprocess with a stored body but a null bindingRef is refused with the existing uncoded 400 path rather than emptied. Studio never produces this, because it echoes bindingRefs.

No contract change. The GET body, the ETag formula and the 428/412/404 and BpmnErrorResponse codes are unchanged, so the Studio client (elsa-workflows/elsa-studio#1031) works as is.

Why not option 1

BpmnWorkBinding has no JSON contract in the library: no property names, no polymorphism, and it is not in bpmn-payload.schema.json. Carrying nested bodies to the client would need an Elsa-owned DTO that restates the library's addressing. That gap is filed as #8076, blocked by valence-works/bpmn#22.

Library workaround

Bpmn.Interchange 0.2.0 also keeps an interpreted multiInstanceLoopCharacteristics inside the retained nested content (valence-works/bpmn#21). Handing stored bodies back unfiltered would add a copy on every PUT and let the stale copy override a client's changed or removed marker. The PUT drops the retained copy wherever the model carries the marker. It keeps the copy only when it is the sole record of a marker the reader degraded, for example a Camunda-authored one. The workaround is marked for removal.

Tests

  • The D4 round-trip test from feat(bpmn): GET and PUT the BPMN document as library-format JSON #8060 is now a theory over five fixtures:
    • camunda-order-process;
    • subprocess-boundary-events and transaction-compensation, copied byte-identical from Studio's W9a fixtures;
    • two new files, nested-subprocesses and camunda-multi-instance-subprocess.
  • DocumentPut_OfUnchangedContent_ReturnsTheETagTheGetReturned is a four-fixture theory.
  • New cases:
    • a subprocess kept, removed or retyped;
    • a re-keyed bindingRef;
    • the null-bindingRef refusal;
    • a loop marker changed, removed or added.
  • 15 service mutations, each turned the targeted tests red.
  • Local runs:
    • Elsa.Bpmn.UnitTests: 30/30
    • Elsa.Bpmn.Interchange.UnitTests: 14/14, including the duplication guard
    • Elsa.Bpmn.IntegrationTests: 59/59
    • Elsa.Bpmn.Interchange.IntegrationTests: 99/99

Follow-ups filed

🤖 Generated with Claude Code

sfmskywalker and others added 2 commits September 12, 2026 18:28
BpmnDefinitions lists only top-level processes: Bpmn.Interchange carries the
body of an embedded subprocess, transaction or event subprocess as the
subprocess element's NestedProcess work binding. The document GET therefore
never returned the bodies, and the PUT wrote the posted document without any
bindings, so BpmnXmlWriter wrote every subprocess empty and the re-import
replaced the definition with one whose subprocesses were empty.

The PUT now re-reads the stored source and hands the writer the stored body of
every subprocess element the posted document still declares, matched by
element id, together with everything bound inside it (a call activity's
vw:waitForCompletion lives only on its CallProcess binding). A removed
subprocess's body is never written back, not even under a new element that
reuses its bindingRef, and a subprocess with a stored body but no bindingRef
is refused rather than emptied. The GET body, the ETag and the error codes are
unchanged; nested content stays uneditable through the document.

Bpmn.Interchange 0.2.0 also retains a copy of a subprocess's interpreted
multiInstanceLoopCharacteristics inside its body. Handed back verbatim, that
copy would override a posted marker on the next read and add another copy on
every PUT, so the PUT drops it wherever the element carries a marker in the
model.

The D4 round-trip test now covers subprocess-boundary-events.bpmn and
transaction-compensation.bpmn, copied from Studio's fixtures, plus two
nested-scope assets, going through the endpoints' JSON options.

Refs #8072

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

Safe to merge: the document update flow preserves retained nested scope content and safely discards it when the corresponding scope is retyped.

What we checked:

  • Exercised a real HTTP import, document read, conditional document update using the returned If-Match, and BPMN export requests, and all requests returned 200 OK. T-Rex
  • Validated endpoint status and reason phrase were captured in the command-output logs from the HTTP run. T-Rex
  • Confirmed the PR export retained every compared nested body element across event subprocess, transaction, and embedded subprocess cases, and retyping OnRecall to endEvent did not export a HandleRecall body. T-Rex
  • Security assessment concluded no issues were observed and noted the fixed behavior prevents silent workflow-structure data loss. T-Rex

Summary

  • BPMN document updates preserve embedded subprocess, transaction, and event-subprocess bodies when a document is read, written back, and exported.
  • Removed or retyped subprocess elements do not retain their former nested body.
  • The document update contract was exercised through HTTP import, document read, conditional update, and export requests. This change is safe to merge.

Reviews (1) · Last reviewed commit: "docs(bpmn): link the loop-marker workaro..."

@sfmskywalker
sfmskywalker merged commit 9f4d332 into main Sep 12, 2026
14 checks passed
@sfmskywalker
sfmskywalker deleted the claude/bpmn-8072-nested-document branch September 12, 2026 16:50
sfmskywalker added a commit to elsa-workflows/elsa-studio that referenced this pull request Sep 12, 2026
* feat(workflows): save BPMN documents with subprocesses

elsa-core now keeps a subprocess's stored body and bindings across the document PUT
(elsa-workflows/elsa-core#8077), so Studio no longer needs to refuse saving a BPMN
document that declares one. Removes BpmnDocumentFailureReason.SubProcessContentNotCarried,
the SubProcessIds check, and the "Performed by" panel's CanEdit/Save gating and info
alert. Editing an element inside a subprocess is still refused, since its body still
isn't part of the document, and now names that as a server limitation tracked in
elsa-workflows/elsa-core#8076 instead of implying a Studio one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* style(workflows): drop a stray blank line

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(workflows): cite the subprocess limitation only for elements in a subprocess

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BPMN W30: the document GET/PUT drops subprocess bodies (data loss)

1 participant