[ashanthamara]

Proposed changes in this pull request

Follow-up to #8335. A flow saved with branches or a decision step in a shape the engine never evaluates was stored silently. GraphBuilder now rejects these cases when the flow is saved, with the shared decision step error code (60014):

• Branches on the action of any step other than a decision step.
• Branches on a button's action, including a button inside a form.
• A decision step whose action type is not ROUTE.
• A decision step that carries components.

Also:

• A decision step with no branches now fails with "No branches defined." under the same error code.
• The decision step error descriptions name the step.

Tests for each case are added to GraphBuilderRuleEvaluationTest.

Related issue: wso2/product-is#28638

When should this PR be merged

No preconditions.

Follow up actions

• Bump the framework version in identity-api-server and product-is.

Developer Checklist (Mandatory)

• Complete the Developer Checklist in the related product-is issue to track any behavioral change or migration impact.

Checklist (for reviewing)

General

• Is this PR explained thoroughly? All code changes must be accounted for in the PR description.
• Is the PR labeled correctly?

Functionality

• Are all requirements met? Compare implemented functionality with the requirements specification.
• Does the UI work as expected? There should be no Javascript errors in the console; all resources should load. There should be no unexpected errors. Deliberately try to break the feature to find out if there are corner cases that are not handled.

Code

• Do you fully understand the introduced changes to the code? If not ask for clarification, it might uncover ways to solve a problem in a more elegant and efficient way.
• Does the PR introduce any inefficient database requests? Use the debug server to check for duplicate requests.
• Are all necessary strings marked for translation? All strings that are exposed to users via the UI must be marked for translation.

Tests

• Are there sufficient test cases? Ensure that all components are tested individually; models, forms, and serializers should be tested in isolation even if a test for a view covers these components.
• If this is a bug fix, are tests for the issue in place? There must be a test case for the bug to ensure the issue won’t regress. Make sure that the tests break without the new code to fix the issue.
• If this is a new feature or a significant change to an existing feature? has the manual testing spreadsheet been updated with instructions for manual testing?

Security

• Confirm this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.
• Are all UI and API inputs run through forms or serializers?
• Are all external inputs validated and sanitized appropriately?
• Does all branching logic have a default case?
• Does this solution handle outliers and edge cases gracefully?
• Are all external communications secured and restricted to SSL?

Documentation

• Are changes to the UI documented in the platform docs? If this PR introduces new platform site functionality or changes existing ones, the changes should be documented.
• Are changes to the API documented in the API docs? If this PR introduces new API functionality or changes existing ones, the changes must be documented.
• Are reusable components documented? If this PR introduces components that are relevant to other developers (for instance a mixin for a view or a generic form) they should be documented in the Wiki.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: wso2/carbon-identity-framework/.coderabbit.yml
• Review profile: CHILL
• Plan: Advanced
• Run ID: d28993d9-9c0a-4d68-ac3c-e0c712f1628b

📥 Commits

Reviewing files that changed from the base of the PR and between 0f92c5f and 5b2e01a.

📒 Files selected for processing: 2

• components/flow-orchestration-framework/org.wso2.carbon.identity.flow.mgt/src/main/java/org/wso2/carbon/identity/flow/mgt/utils/GraphBuilder.java
• components/flow-orchestration-framework/org.wso2.carbon.identity.flow.mgt/src/test/java/org/wso2/carbon/identity/flow/mgt/utils/GraphBuilderRuleEvaluationTest.java

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

GraphBuilder now validates rule-evaluation steps that use ROUTE actions and branches. It rejects invalid rule-step configuration and branches on non-rule steps or form buttons. Tests assert the validation errors and messages.

Changes: Rule-step graph validation

Layer / File(s)	Summary
Validate rule steps and branch placement components/flow-orchestration-framework/org.wso2.carbon.identity.flow.mgt/src/main/java/org/wso2/carbon/identity/flow/mgt/utils/GraphBuilder.java, components/flow-orchestration-framework/org.wso2.carbon.identity.flow.mgt/src/test/java/org/wso2/carbon/identity/flow/mgt/utils/GraphBuilderRuleEvaluationTest.java	Rule steps must use ROUTE, cannot include an executor or components, and must define valid branches. Branches on other step types and form buttons are rejected. Tests check the associated error codes and messages.

Possibly related issues

• wso2/product-is issue 28638: The pull request validates decision steps with ROUTE actions and branches when flows are saved.

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 5b2e0

The change tightens decision-step validation at save time and adds tests for each rejected case. No concrete merge-blocking issue was found. The PR description already notes that the downstream framework version bump is a separate follow-up.

Security architecture risk: 🔵 Low · up to 5b2e0

The change rejects unsupported decision-flow configurations before they are stored. No new execution capability or weakened control was identified. Previously accepted invalid definitions will need correction when saved again.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The confirmed boundary is flow management: updates use the caller-supplied tenant identifier and flow type, while reads can resolve an inherited flow through the organization hierarchy. This PR changes configuration acceptance, not that tenant-selection mechanism.

Trust Boundaries and Controls

• observed — The existing conditional-branching feature gate remains ahead of decision-node creation. The added validation narrows accepted actions and rejects components; it does not introduce an executor path or additional authority for decision steps.

Resilience and Maintainability Implications

• inferred — A failed save can still invalidate resolution caches, including asynchronous invalidation for child organizations, because that existing operation precedes validation. The unchanged miss path resolves flow ownership from persisted definitions; cache invalidation alone does not delete the previous flow or force selection of a system default.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Description check		The description explains the implemented validation changes, related issue, follow-up version bump, and added tests. However, it omits most template sections, including release note, documentation, tr…	Complete the required template sections. Mark non-applicable sections as “N/A” with a brief explanation, and provide the requested test, security, documentation, migration, and environment details.
Docstring Coverage		Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title is concise and clearly identifies the main change: stricter validation for decision steps.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the implemented validation changes, related issue, follow-up version bump, and added tests. However, it omits most template sections, including release note, documentation, training, certification, marketing, automation test details, security checks, test environment, migrations, and learning.

Resolution

Complete the required template sections. Mark non-applicable sections as “N/A” with a brief explanation, and provide the requested test, security, documentation, migration, and environment details.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[jenkins-is-staging]

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/39425765760

[codecov]

Codecov Report

❌ Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.90%. Comparing base (8806b0d) to head (5b2e01a).
⚠️ Report is 30 commits behind head on master.

Files with missing lines	Patch %	Lines
...2/carbon/identity/flow/mgt/utils/GraphBuilder.java	86.95%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #8368      +/-   ##
============================================
+ Coverage     54.76%   54.90%   +0.14%     
+ Complexity    22836    22746      -90     
============================================
  Files          2277     2277              
  Lines        135028   133919    -1109     
  Branches      22413    22204     -209     
============================================
- Hits          73945    73534     -411     
+ Misses        52180    51562     -618     
+ Partials       8903     8823      -80     

Flag	Coverage Δ
unit	40.43% <86.95%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jenkins-is-staging]

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/39425765760
Status: success

[jenkins-is-staging]

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/39425765760