Skip to content

refactor(workflows): drop the BpmnProcess port provider now that core declares its outcomes - #1027

Merged
sfmskywalker merged 2 commits into
mainfrom
claude/bpmn-w25-drop-port-provider
Sep 12, 2026
Merged

sfmskywalker merged 2 commits into
mainfrom
claude/bpmn-w25-drop-port-provider

Conversation

@sfmskywalker

Copy link
Copy Markdown
Member

Closes #1019. Part of elsa-workflows/elsa-core#7909.

What changed

BpmnProcessPortProvider existed only because elsa-core's BpmnProcess declared no outcomes (#1002). Since elsa-workflows/elsa-core#8066, BpmnProcess carries [FlowNode(Done, Cancelled)], so the server's descriptor lists both flow ports and the provider is a no-op superset.

  • Deleted BpmnProcessPortProvider and its registration.
  • BpmnProcessConstants keeps ActivityTypeName and DoneOutcomeName, which are still used by the designer provider, the wrapper, display settings and tests. The unused CancelledOutcomeName and OutcomeNames are removed.
  • The provider tests are replaced by BpmnProcessPortsTests. It feeds a descriptor shaped like the server's response through the default port path and asserts Done and Cancelled appear as flow ports, with nothing synthesized.

Verification

dotnet build Elsa.Studio.sln --configuration Release
dotnet test src/modules/Elsa.Studio.Workflows.Tests --configuration Release --no-build --framework net10.0   # 359/359

The nested-scope drill-down tests from #1002 still pass.

Review: the standards and spec axes, both clean. One judgement was taken: the test class was renamed from the deleted provider's name.

🤖 Generated with Claude Code

sfmskywalker and others added 2 commits September 12, 2026 12:58
…s declares its own flow ports

elsa-core's BpmnProcess now declares Done and Cancelled via [FlowNode]
(elsa-workflows/elsa-core#8066), so the descriptor already carries both
flow ports and the provider that used to synthesize them is a no-op
superset. Removed the provider and its registration, trimmed the now-
unused CancelledOutcomeName/OutcomeNames off BpmnProcessConstants
(ActivityTypeName and DoneOutcomeName are still used elsewhere), and
replaced the provider tests with one that feeds a descriptor shaped
like the new server response through the default port path.

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

The deleted BpmnProcessPortProvider no longer exists; the test now exercises
DefaultActivityPortProvider against the server's Elsa.BpmnProcess descriptor.

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: 4/5

Not safe to merge until Studio either preserves BPMN outcome-port compatibility for older backends or explicitly prevents connections to unsupported backend versions.

Findings

  1. P1 Preserve Older Backend Ports
  2. P2 Test Descriptor Metadata
Fix with agent prompt
### Issue 1
src/modules/Elsa.Studio.ActivityPortProviders/Extensions/ServiceCollectionExtensions.cs:23
If Studio connects to an independently deployed backend that does not declare BPMN `Done` and `Cancelled` ports, removing this registration leaves those ports absent. The remaining default provider only returns ports supplied by the descriptor, so users cannot create or retain completion and cancellation branches. Keep a compatibility fallback or enforce a backend capability or version boundary before removing it.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 2
src/modules/Elsa.Studio.Workflows.Tests/BpmnProcessPortsTests.cs:40-51
This test manually adds `Done` and `Cancelled` to `ActivityDescriptor.Ports` before asserting the default provider result. Because the provider returns that list unchanged, the test passes even if the server stops declaring either outcome. Add coverage at the descriptor acquisition or generation boundary, or use a server-produced contract fixture, so missing metadata is detected. This is non-blocking, but the current test can provide false confidence in the compatibility change.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

  • Summary

  • Studio now relies exclusively on backend-supplied BPMN ports. This breaks compatibility with independently deployed older backends that omit Done and Cancelled, because Studio no longer adds those outcomes locally.
  • The replacement test also supplies those ports itself, so it does not verify the metadata contract that the change depends on.
  • T-Rex validation blocked

  • The focused .NET test command could not run because the environment does not have dotnet installed. Source-backed scenario checks completed and reproduced the affected port behavior.

Reviews (1) · Last reviewed commit: "test(workflows): rename BpmnProcessPortP..."

Comment thread src/modules/Elsa.Studio.Workflows.Tests/BpmnProcessPortsTests.cs
@sfmskywalker
sfmskywalker merged commit 406c7ef into main Sep 12, 2026
10 checks passed
@sfmskywalker
sfmskywalker deleted the claude/bpmn-w25-drop-port-provider branch September 12, 2026 11:22
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 W25: drop the BpmnProcess port provider once core declares its outcomes

1 participant