Orchestrator 1.9 backport

[lokanandaprabhu]

Cherrypick of below PR's

#2834
#2808
#2818

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Deep import may break tests 🐞 Bug ☼ Reliability

Description

safeSet.test.ts imports JsonObject from '@backstage/types/index', which can fail under Node/package
"exports" restrictions even though the production code imports from '@backstage/types'. This can
break CI/test runs depending on module resolution settings.

Code

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts[R19-21]

+import { safeSet } from './safeSet';
+import { JsonObject } from '@backstage/types/index';
+

Relevance

⭐⭐⭐ High

Repo previously replaced '@backstage/types/index' with root import to fix TS/exports issues.

PR-#1990

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test uses a deep import path that may not be exported by the package, while the implementation
uses the supported package root import. Keeping test imports consistent with production reduces the
chance of resolution failures.

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts[17-21]
workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.ts[17-18]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A Jest test uses `@backstage/types/index` deep import, which may not be allowed by the package's `exports` map and can break builds/tests in stricter resolvers.

### Issue Context
Production code imports `JsonObject` from `@backstage/types`.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts[17-21]

### Suggested fix
Change:
```ts
import { JsonObject } from '@backstage/types/index';
```
To:
```ts
import { JsonObject } from '@backstage/types';
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Async rejections may go unhandled 🐞 Bug ☼ Reliability

Description

useGetExtraErrors now awaits an array of already-started promises one-by-one; if any promise rejects
before it is awaited (or if an earlier await throws), other rejections may temporarily or
permanently be unobserved, leading to unhandledRejection warnings/failing tests. This change also
does not actually serialize execution because walkThrough eagerly starts all async callbacks before
the loop begins.

Code

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[R146-151]

+    // Run sequentially so concurrent async callbacks cannot interleave
+    // `safeSet` mutations on the shared `errors` object (lost updates under
+    // the same path prefix, e.g. `step.xParams.fieldA` vs `fieldB`).
+    for (const p of promises) {
+      await p;
+    }

Relevance

⭐ Low

Same sequential-await change merged recently to fix lost updates; team accepted this approach.

PR-#2818

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

walkThrough immediately invokes the async callback for each matching uiSchema node and returns the
resulting Promise objects, meaning all validations start running concurrently before any awaiting
happens. The new for..of loop awaits promises sequentially, but it does not prevent concurrent
execution and it attaches awaiting/handlers to promises later (one at a time), which can allow
promise rejections to occur before being observed; additionally, if one await throws, the loop
aborts and later promises are never awaited.

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[32-66]
workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[77-137]
workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[139-153]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`useGetExtraErrors` builds an array of *already running* promises and then awaits them in a `for..of` loop. This does not truly serialize validations and can increase the risk of unhandled promise rejections (promises can reject before they are awaited/observed; and if an early `await` throws, later promises are never awaited).

### Issue Context
The goal seems to be preventing lost updates to the shared `errors` object. The safeSet deep-path fix already addresses the primary overwrite issue; if true serialization is still desired, tasks must be started sequentially (not by awaiting already-started promises).

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[32-66]
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[139-153]

### Suggested fixes
Choose one:
1) **Restore safe observation**: switch back to `await Promise.all(promises)` (or `Promise.allSettled`) so all promises have handlers attached immediately.
2) **True sequential execution**: change `walkThrough` to return an array of thunks `() => Promise<void>` (or an async generator), then `for..of` call/await each thunk so later validations don’t start until earlier ones complete.
3) If keeping the current structure, at least attach handlers eagerly: `const observed = promises.map(p => p.catch(e => { throw e; }));` and await `Promise.all(observed)` / `allSettled` so no rejection goes unobserved.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Mixed MUI major versions 🐞 Bug ⚙ Maintainability

Description

WorkflowResult now imports Alert components from @material-ui/lab (MUI v4) while the rest of the
file uses @mui/material (v5). This increases styling/theming complexity and maintenance burden
within the same component.

Code

workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowResult.tsx[R28-31]

+import { Alert, AlertTitle } from '@material-ui/lab';
import Box from '@mui/material/Box';
import Button from '@mui/material/Button';
import CircularProgress from '@mui/material/CircularProgress';

Relevance

⭐ Low

Team intentionally switched Alerts to @material-ui/lab to avoid MUI v5 styling conflicts.

PR-#2808

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

WorkflowResult imports Alert/AlertTitle from the v4 lab package alongside v5 @mui/material
components, and package.json adds @material-ui/lab while also depending on @mui/material. This can
lead to inconsistent theming and larger dependency surface.

workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowResult.tsx[28-36]
workspaces/orchestrator/plugins/orchestrator/package.json[55-87]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The component mixes MUI v4 (`@material-ui/lab`) and MUI v5 (`@mui/material`) imports.

### Issue Context
This may be intentional for styling parity, but it increases maintenance cost and can create theming inconsistencies.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowResult.tsx[28-36]
- workspaces/orchestrator/plugins/orchestrator/package.json[55-87]

### Suggested fix
If possible, revert to `@mui/material/Alert` + `@mui/material/AlertTitle` and apply the needed styling overrides using v5 APIs (`sx`, theme overrides, etc.), avoiding the extra v4 lab dependency.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[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

[rhdh-qodo-merge]

Review Summary by Qodo

Fix orchestrator form validation errors and review step crashes

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Fix multi-step form validation errors being dropped or misplaced in nested fields
• Handle empty form data in review step to prevent crashes
• Correct deep nested field path handling in error aggregation
• Fix UI styling issues in workflow results page components

Diagram

flowchart LR
  A["Multi-step Forms"] -->|"Sequential async validation"| B["useGetExtraErrors"]
  B -->|"Deep path handling"| C["safeSet utility"]
  C -->|"Wrapped with active step"| D["toRootExtraErrors"]
  D -->|"Root-shaped errors"| E["RJSF Form"]
  F["Empty Form Data"] -->|"Fallback to empty object"| G["generateReviewTableData"]
  G -->|"Safe data access"| H["NestedReviewTable"]
  I["UI Components"] -->|"Styling fixes"| J["WorkflowResult & TextCodeBlock"]

Loading

File Changes

1. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.ts 🐞 Bug fix +2/-1

Return empty object instead of undefined

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.ts

---

2. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.test.ts 🧪 Tests +17/-0

Add test for empty form data handling

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/generateReviewTableData.test.ts

---

3. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.ts ✨ Enhancement +41/-0

New utility to wrap errors with active step

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.ts

---

View more (14)

4. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.test.ts 🧪 Tests +53/-0

Test error wrapping for multi-step forms

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.test.ts

---

5. workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx 🐞 Bug fix +2/-1

Guard against undefined data parameter

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/NestedReviewTable.tsx

---

6. workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorFormWrapper.tsx ✨ Enhancement +2/-9

Use toRootExtraErrors for error handling

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorFormWrapper.tsx

---

7. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.ts 🐞 Bug fix +8/-6

Fix deep nested path handling in errors

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.ts

---

8. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts 🧪 Tests +68/-0

Test deep path and sequential error merging

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts

---

9. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts 🐞 Bug fix +6/-1

Run async validation sequentially to prevent race conditions

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts

---

10. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.test.tsx 🧪 Tests +120/-0

Test async validation error aggregation

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.test.tsx

---

11. workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowDescriptionModal.tsx 🐞 Bug fix +8/-1

Add flexbox styling to dialog title

workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowDescriptionModal.tsx

---

12. workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowResult.tsx 🐞 Bug fix +3/-4

Fix Alert imports and Divider styling

workspaces/orchestrator/plugins/orchestrator/src/components/WorkflowInstancePage/WorkflowResult.tsx

---

13. workspaces/orchestrator/plugins/orchestrator/src/components/ui/TextCodeBlock.tsx 🐞 Bug fix +4/-1

Fix outline and background styling specificity

workspaces/orchestrator/plugins/orchestrator/src/components/ui/TextCodeBlock.tsx

---

14. workspaces/orchestrator/plugins/orchestrator/package.json Dependencies +1/-0

Add material-ui lab dependency

workspaces/orchestrator/plugins/orchestrator/package.json

---

15. workspaces/orchestrator/.changeset/fix-review-empty-formdata.md 📝 Documentation +5/-0

Changeset for empty form data fix

workspaces/orchestrator/.changeset/fix-review-empty-formdata.md

---

16. workspaces/orchestrator/.changeset/new-books-matter.md 📝 Documentation +5/-0

Changeset for UI styling fixes

workspaces/orchestrator/.changeset/new-books-matter.md

---

17. workspaces/orchestrator/.changeset/sharp-dots-sing.md 📝 Documentation +10/-0

Changeset for async validation error fixes

workspaces/orchestrator/.changeset/sharp-dots-sing.md

---

[karthikjeeyar]

/lgtm