You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refine import/export of workflow configurations - #5523
Failed merges can leave an existing workflow configuration partially modified
F2
🟠 high
🟢 high
🔴 new
Missing workflow states are recreated with fabricated semantics
F3
🔵 low
🟢 high
🔴 new
The new transfer workflow is absent from help and What’s New content
F4
🔵 low
🟢 high
🔴 new
Production method documentation is missing or stale
Recommendations
Fix in this PR
F1: Make merge-to-existing imports atomic, or explicitly reverse every added phase, derived-state mapping, and transition-group link when any later step fails; add failure-injection tests for each mutation boundary.
F2: Preserve and validate complete state definitions in the transfer format (including name and automatic_only), or retain the previous behavior of rejecting packages whose referenced states do not exist.
F3: Update HelpSettingsStateMatrix.cshtml and both language versions of whats_new_facts for selective export, custom filenames, and merge imports.
F4: Add the required XML summary for OnAfterRenderAsync and update Import so its summary covers both new-configuration and merge modes.
Create follow-up issue
None. Each finding was introduced by this feature and belongs within its stated scope.
Details
F1 — Failed merges can leave an existing workflow configuration partially modified
WorkflowConfigurationTransferService.cs:108-109 enters MergeIntoConfiguration, where each phase and each transition-group link is written through a separate GraphQL request (:197-208, :220-233, :248-275). If, for example, the derived-state upsert succeeds and a later link or phase insert fails, the catch block calls RollBack (:132-140), but that method only deletes newly created transition groups, visibility groups, and states (:625-665). It never removes phases, derived-state mappings, or links already added to the selected target configuration. The import therefore reports failure while leaving the existing configuration partly changed; newly created states referenced by those leftover rows can also make rollback fail with an aggregate exception. Use one transactional mutation or track and reverse every merge mutation, and test failures after an added phase, derived state, and earlier link.
F2 — Missing workflow states are recreated with fabricated semantics
CreateMissingStates inserts every referenced-but-absent ID with name = "Imported state {id}" and automaticOnly = false (WorkflowConfigurationTransferService.cs:279-290), but version 1 of the package does not carry either source attribute. This changes behavior, not just presentation: StateMatrix.cs:66 builds AutomaticOnlyStates from that flag, and StateMatrix.cs:98-110 uses it to suppress manual transitions. Importing a package whose source state was automatic-only can therefore expose that state as a manual transition on the target, while also losing its real name. Negative/reserved or otherwise invalid IDs are not rejected either. Transfer complete state definitions with validation (and a format-version update), or reject missing target states as before.
F3 — The new transfer workflow is absent from help and What’s New content
The UI now adds selectable export phases, a custom filename, and import into an existing configuration (TransferWorkflowConfiguration.razor:23-33, :60-85, :247-260), but the PR changes neither roles/ui/files/FWO.UI/Pages/Help/HelpSettingsStateMatrix.cshtml nor the German/English whats_new_facts entries in fworch-texts.sql. Repository policy requires both for a UI feature change.
F4 — Production method documentation is missing or stale
The new OnAfterRenderAsync override at TransferWorkflowConfiguration.razor:175 has no required /// summary. The existing summary at :245 now says Import always creates a new inactive configuration even though :257 can merge into an existing target. Bring both comments in line with the repository’s C# documentation rule and the implemented behavior.
Review summary
Round: 1 of max. 5.
Depth/capability: deep; fwo-review-pr two-pass review (correctness/quality plus security), followed by primary-model reconciliation and verification.
Scope: all 9 changed files at head 692e9a8dc63b5c6b9cc8bf5e87cd41f5b2638483 were inspected, together with the directly relevant state-matrix runtime code and changed tests. Security checks covered authorization reachability, GraphQL/input handling, stored XSS/output encoding, CSP, tenant isolation, secrets/logging, and rollback/data integrity.
Delegation: reduced-tier gpt-5.6-sol workers collected GitHub diff/history evidence and performed mechanical coding-guideline, localization/help, HTML/CSP, and test-presence scans. Security decisions, defect verification, ratings, numbering, and this comment were handled on the primary model. One earlier reduced-tier dispatch failed at capacity and returned no review data.
Validation:FWO.Services, FWO.Ui, and FWO.Test built successfully with .NET 10 and BuildProjectReferences=false; 72 targeted tests passed. git diff --check passed. A full solution/reference build could not be validated because the local MSBuild worker launcher failed without a diagnostic and then spawned runaway workers under serialized retry; those review-launched workers were stopped. This remains residual validation risk.
Token budget: usage could not be measured because this environment exposes no session usage/cost figure or attributable transcript counter (measurement method 4). The deep-review ceiling is 1M review tokens; proxy limits were enforced (4 dispatches, one failed without data; no repeated diff/history fetch; review limited to changed files and direct dependencies).
Model: OpenAI Codex, GPT-5 family. The exact serving version and primary reasoning setting are not exposed to the agent; reduced-tier scans used gpt-5.6-sol with low reasoning. No more specific model metadata is claimed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.