Add call node components to the flow builder and gate UI - #3738
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBackend call-depth handling now returns a dedicated overflow error. The frontend adds CALL step types, call-node UI and properties, rich-text action wiring, transformer updates, runtime dispatch, validation, translations, and workspace version updates. ChangesCross-flow Call step and rich-text action support
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CallNode
participant Dialog
participant Router
User->>CallNode: open referenced flow
CallNode->>Dialog: show confirmation
User->>Dialog: continue
Dialog->>Router: navigate to referenced flow
sequenceDiagram
participant User
participant RichTextAdapter
participant FlowComponentRenderer
participant ActionHandler
User->>RichTextAdapter: click wired anchor
RichTextAdapter->>ActionHandler: prevent default and build action
ActionHandler->>FlowComponentRenderer: onSubmit(action, values)
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
backend/internal/flow/flowexec/engine.go (1)
821-828: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDebug log drops target-flow context present in the prior error log.
The new debug log only records
frameDepth/maxCallDepth; the previous Error-level log also included the call node and target flow IDs, which are useful for tracing which nested call chain hit the limit. Consider addingnodeResp.CallTargetFlowID(and/or the current node ID) to the debug log for troubleshooting, since it's already available in this method.♻️ Suggested log enrichment
if ctx.frameDepth() >= maxCallDepth { logger.Debug(ctx.Context, "Maximum call depth exceeded", log.Int("frameDepth", ctx.frameDepth()), - log.Int("maxCallDepth", maxCallDepth)) + log.Int("maxCallDepth", maxCallDepth), log.String("targetFlowID", nodeResp.CallTargetFlowID)) return nil, &ErrorMaxCallDepthExceeded }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/flow/flowexec/engine.go` around lines 821 - 828, The max-call-depth debug log in handleCallResponse is missing the target-flow context that was previously included, so enrich the logger call with the available call-chain identifiers from nodeResp and the current node. Update the existing Maximum call depth exceeded branch in flowEngine.handleCallResponse to include nodeResp.CallTargetFlowID and the current node ID (or equivalent call node identifier) alongside frameDepth and maxCallDepth so nested call failures are easier to trace.frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid reusing
ExecutionMinimal.scssclasses directly.Call imports and relies on
execution-minimal-step*/execution-handle-*classes owned by the Execution step. This creates an undeclared coupling — a future refactor ofExecutionMinimal.scsscould silently break Call's layout/handles with no compile-time warning.♻️ Suggested approach
Extract the shared card/handle styles both components need into a common file (e.g.
shared/MinimalStepCard.scss) and have bothExecutionMinimalandCallimport from it, rather than one feature importing another's stylesheet.Also applies to: 156-159, 184-184, 230-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx` at line 47, The Call component is directly depending on Execution’s stylesheet classes, which creates a hidden coupling between unrelated features. Move the shared card and handle styles used by Call and ExecutionMinimal into a common stylesheet (for example, a shared MinimalStepCard SCSS file), then update both Call and ExecutionMinimal to import that shared file instead of importing ExecutionMinimal.scss from Call. Keep the Call-specific selectors aligned with the shared class names so the layout and handles still work without depending on Execution-owned styles.frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsx (1)
53-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the "unknown/stale flow ref" message.
Tests cover load/error/list/select paths but not the case where
currentRefis set yet absent from the resolvedflowslist, which should surfaceflows:core.call.properties.flow.error.unknown.✅ Suggested additional test
+ it('shows a stale-reference message when the referenced flow no longer exists', () => { + mockUseGetFlows.mockReturnValue({ + data: {flows: [{id: 'flow-a', name: 'Flow A', flowType: 'AUTHENTICATION'}]}, + isLoading: false, + error: null, + }); + render( + <CallProperties + resource={makeResource({data: {flow: {ref: 'missing-flow'}}})} + onChange={onChange} + />, + ); + expect(screen.getByTestId('call-flow-ref-error')).toHaveTextContent(/no longer exists/i); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsx` around lines 53 - 112, The CallProperties test suite is missing coverage for the stale/unknown flow reference state, where currentRef exists but is not present in the fetched flows list. Add a test in CallProperties.test.tsx that mocks useGetFlows with a flows array excluding the current ref, renders CallProperties with a resource whose data.flow.ref points to that missing id, and asserts the unknown flow error message key flows:core.call.properties.flow.error.unknown is shown. Use CallProperties, makeResource, and mockUseGetFlows to locate and exercise the existing flow dropdown behavior.frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsx (1)
91-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for edge cleanup on disable.
This test only asserts
onChange('action', null, ...); it doesn't verifysetEdgesis called to prune edges sourced from this handle, which is the other important side effect of disabling.✅ Suggested additional assertion
it('turning the toggle off clears the action', () => { const resource = makeResource({action: {ref: 'action_signup'}} as unknown as Partial<Resource>); render(<RichTextActionFields resource={resource} onChange={onChange} />); const toggle = screen.getByTestId('rich-text-action-enabled').querySelector('input') as HTMLInputElement; fireEvent.click(toggle); expect(onChange).toHaveBeenCalledWith('action', null, resource); + expect(mockSetEdges).toHaveBeenCalled(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsx` around lines 91 - 97, The disable-toggle test in RichTextActionFields only checks that onChange clears the action, but it also needs to cover the edge-pruning side effect. Update the test around RichTextActionFields to assert that setEdges is called when the toggle is turned off, and that it removes edges originating from the action handle in addition to calling onChange('action', null, resource). Use the existing onChange and setEdges mocks in the RichTextActionFields test setup to verify both behaviors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.ts`:
- Around line 97-137: The change-detection check in useVisualFlowHandlers is
always true because updateRichTextRef recreates the array on every call, so
every next connection triggers updateNodeData. Track whether a matching
rich-text element was actually found and only call updateNodeData when the
recursive walk in updateRichTextRef changes something, using the
connection.sourceHandle, componentId, and ElementTypes.RichText match to gate
the update.
---
Nitpick comments:
In `@backend/internal/flow/flowexec/engine.go`:
- Around line 821-828: The max-call-depth debug log in handleCallResponse is
missing the target-flow context that was previously included, so enrich the
logger call with the available call-chain identifiers from nodeResp and the
current node. Update the existing Maximum call depth exceeded branch in
flowEngine.handleCallResponse to include nodeResp.CallTargetFlowID and the
current node ID (or equivalent call node identifier) alongside frameDepth and
maxCallDepth so nested call failures are easier to trace.
In
`@frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsx`:
- Around line 91-97: The disable-toggle test in RichTextActionFields only checks
that onChange clears the action, but it also needs to cover the edge-pruning
side effect. Update the test around RichTextActionFields to assert that setEdges
is called when the toggle is turned off, and that it removes edges originating
from the action handle in addition to calling onChange('action', null,
resource). Use the existing onChange and setEdges mocks in the
RichTextActionFields test setup to verify both behaviors.
In
`@frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx`:
- Line 47: The Call component is directly depending on Execution’s stylesheet
classes, which creates a hidden coupling between unrelated features. Move the
shared card and handle styles used by Call and ExecutionMinimal into a common
stylesheet (for example, a shared MinimalStepCard SCSS file), then update both
Call and ExecutionMinimal to import that shared file instead of importing
ExecutionMinimal.scss from Call. Keep the Call-specific selectors aligned with
the shared class names so the layout and handles still work without depending on
Execution-owned styles.
In
`@frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsx`:
- Around line 53-112: The CallProperties test suite is missing coverage for the
stale/unknown flow reference state, where currentRef exists but is not present
in the fetched flows list. Add a test in CallProperties.test.tsx that mocks
useGetFlows with a flows array excluding the current ref, renders CallProperties
with a resource whose data.flow.ref points to that missing id, and asserts the
unknown flow error message key flows:core.call.properties.flow.error.unknown is
shown. Use CallProperties, makeResource, and mockUseGetFlows to locate and
exercise the existing flow dropdown behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a7137b9-ee8d-498a-9840-e2f36e8a6564
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
backend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/system/i18n/core/defaults.gofrontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/RichTextAdapter.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/__tests__/Call.test.tsxfrontend/apps/console/src/features/flows/constants/VisualFlowConstants.tsfrontend/apps/console/src/features/flows/data/steps.jsonfrontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.tsfrontend/apps/console/src/features/flows/models/__tests__/steps.test.tsfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/flows/models/responses.tsfrontend/apps/console/src/features/flows/models/steps.tsfrontend/apps/console/src/features/flows/models/widget.tsfrontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.tsfrontend/apps/console/src/features/flows/utils/generateFlowGraph.tsfrontend/apps/console/src/features/flows/utils/reactFlowTransformer.tsfrontend/apps/console/src/features/flows/validation/validation-rules.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsxfrontend/apps/console/src/features/login-flow/data/widgets.jsonfrontend/packages/design/src/components/flow/FlowComponentRenderer.tsxfrontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsxfrontend/packages/i18n/src/locales/en-US.tspnpm-workspace.yaml
936b223 to
35c1b32
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx (1)
126-167: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize sanitize/parse work; it currently reruns on every render regardless of
valueschanges.
sanitized/finalHtml(DOMPurify + template DOM parsing/querySelectorAll) andhandleClickare recomputed on every render of this branch. Sincevaluesis now a prop that likely changes on every keystroke elsewhere in the same step, and this component isn't wrapped inReact.memo, the sanitize/neutralize work re-runs even though it doesn't depend onvaluesat all — onlyhandleClick's closure needsvalues.♻️ Suggested memoization
- const resolvedLabel = resolve(rawLabel) ?? rawLabel; - const actionRef = richTextAction.ref; - - const handleClick = (event: ReactMouseEvent<HTMLDivElement>): void => { + const resolvedLabel = resolve(rawLabel) ?? rawLabel; + const actionRef = richTextAction.ref; + + const finalHtml = useMemo(() => { + const sanitized = DOMPurify.sanitize(resolvedLabel, {ADD_ATTR: ['target', 'data-action-ref']}); + return neutralizeActionAnchors(sanitized, actionRef); + }, [resolvedLabel, actionRef]); + + const handleClick = useCallback((event: ReactMouseEvent<HTMLDivElement>): void => { const anchor = (event.target as HTMLElement).closest('a'); ... - }; - - const sanitized = DOMPurify.sanitize(resolvedLabel, {ADD_ATTR: ['target', 'data-action-ref']}); - const finalHtml = neutralizeActionAnchors(sanitized, actionRef); + }, [actionRef, onSubmit, values]);Note: hooks can't be called conditionally after the early-return pattern here — this would need restructuring (e.g., compute unconditionally and branch on the JSX, or extract this branch into its own component) to satisfy the Rules of Hooks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx` around lines 126 - 167, The RichTextAdapter render path is recomputing DOMPurify sanitization and anchor neutralization on every re-render even though that work does not depend on values. Refactor this branch so the expensive sanitized/finalHtml work is memoized or moved into a separate component that can use hooks safely, while keeping handleClick able to read the latest values. Use the RichTextAdapter, handleClick, sanitized, and finalHtml symbols to locate the branch and avoid re-running parse/querySelectorAll work unless richTextAction.ref or rawLabel changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx`:
- Around line 61-71: The anchor sanitization in RichTextAdapter should not leave
non-matching wired-action links untouched when any sentinel is present, because
their original target/rel can persist and enable reverse tabnabbing. Update the
anchor-walking logic in the RichTextAdapter processing flow so that the early
return for anchors whose data-action-ref does not match actionRef still
normalizes their security attributes, or otherwise strips/rewrites target and
rel consistently. Make the fix in the same anchor loop that currently sets href,
target, and rel on matching links.
- Around line 54-58: Make the action-link HTML deterministic in RichTextAdapter
so SSR and hydration produce the same markup. The current
neutralizeActionAnchors function returns the original HTML when document is
undefined, which causes dangerouslySetInnerHTML to differ between server and
client; update neutralizeActionAnchors and the RichTextAdapter render path to
emit the neutralized anchors on both sides, or gate this subtree to client-only
rendering so the HTML never changes during hydration.
---
Nitpick comments:
In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx`:
- Around line 126-167: The RichTextAdapter render path is recomputing DOMPurify
sanitization and anchor neutralization on every re-render even though that work
does not depend on values. Refactor this branch so the expensive
sanitized/finalHtml work is memoized or moved into a separate component that can
use hooks safely, while keeping handleClick able to read the latest values. Use
the RichTextAdapter, handleClick, sanitized, and finalHtml symbols to locate the
branch and avoid re-running parse/querySelectorAll work unless
richTextAction.ref or rawLabel changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 97a37e46-d932-4363-b1b6-d412f0992ed9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (33)
backend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/system/i18n/core/defaults.gofrontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/RichTextAdapter.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/__tests__/Call.test.tsxfrontend/apps/console/src/features/flows/constants/VisualFlowConstants.tsfrontend/apps/console/src/features/flows/data/steps.jsonfrontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.tsfrontend/apps/console/src/features/flows/models/__tests__/steps.test.tsfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/flows/models/responses.tsfrontend/apps/console/src/features/flows/models/steps.tsfrontend/apps/console/src/features/flows/models/widget.tsfrontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.tsfrontend/apps/console/src/features/flows/utils/generateFlowGraph.tsfrontend/apps/console/src/features/flows/utils/reactFlowTransformer.tsfrontend/apps/console/src/features/flows/validation/validation-rules.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsxfrontend/apps/console/src/features/login-flow/data/widgets.jsonfrontend/packages/design/src/components/flow/FlowComponentRenderer.tsxfrontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsxfrontend/packages/i18n/src/locales/en-US.tspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (4)
- frontend/apps/console/src/features/flows/models/steps.ts
- backend/internal/system/i18n/core/defaults.go
- frontend/packages/i18n/src/locales/en-US.ts
- pnpm-workspace.yaml
🚧 Files skipped from review as they are similar to previous changes (27)
- frontend/apps/console/src/features/flows/models/tests/steps.test.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/tests/CallProperties.test.tsx
- frontend/apps/console/src/features/flows/models/responses.ts
- frontend/apps/console/src/features/flows/models/flows.ts
- frontend/apps/console/src/features/flows/data/steps.json
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx
- frontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsx
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/tests/RichTextAdapter.test.tsx
- frontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/tests/RichTextActionFields.test.tsx
- backend/internal/flow/flowexec/engine.go
- frontend/apps/console/src/features/flows/components/resources/steps/call/tests/Call.test.tsx
- frontend/apps/console/src/features/flows/models/widget.ts
- frontend/packages/design/src/components/flow/FlowComponentRenderer.tsx
- backend/internal/flow/flowexec/error_constants.go
- frontend/apps/console/src/features/flows/constants/VisualFlowConstants.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsx
- frontend/apps/console/src/features/flows/utils/generateFlowGraph.ts
- frontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.ts
- frontend/apps/console/src/features/flows/utils/reactFlowTransformer.ts
- frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx
- frontend/apps/console/src/features/flows/validation/validation-rules.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsx
- frontend/apps/console/src/features/login-flow/data/widgets.json
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsx
- frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts
0c1c3b6 to
61aa616
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx (1)
60-61: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winNormalize
target/relbefore skipping non-action anchors.This still returns the original anchor for non-matching
data-action-ref, preservingtarget="_blank"without enforcingrel="noopener noreferrer".🔒️ Suggested hardening
const anchorRef = sentinelMatch ? (sentinelMatch[1] ?? sentinelMatch[2]) : undefined; if (anchorRef !== actionRef) { + const safeAttrs = attrs.replace(/\srel\s*=\s*(?:"[^"]*"|'[^']*')/gi, ''); + if (/\starget\s*=\s*(?:"_blank"|'_blank')/i.test(safeAttrs)) { + return `<a${safeAttrs} rel="noopener noreferrer">`; + } return match; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx` around lines 60 - 61, The anchor handling in RichTextAdapter still returns early for non-action links before applying safe link normalization. Update the logic around the anchor matching check in RichTextAdapter so `target` and `rel` are normalized for all anchors first, then only skip the action-specific rewrite when `data-action-ref` does not match `actionRef`; this ensures `target="_blank"` links still get the hardened `rel` values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx`:
- Around line 126-128: The click handler in RichTextAdapter’s handleClick should
guard the event target before using .closest because event.target may be a Text
node and not an HTMLElement. Update the logic so it safely checks or narrows
event.target before calling .closest on it, while preserving the existing action
dispatch behavior when an anchor is found.
---
Duplicate comments:
In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx`:
- Around line 60-61: The anchor handling in RichTextAdapter still returns early
for non-action links before applying safe link normalization. Update the logic
around the anchor matching check in RichTextAdapter so `target` and `rel` are
normalized for all anchors first, then only skip the action-specific rewrite
when `data-action-ref` does not match `actionRef`; this ensures
`target="_blank"` links still get the hardened `rel` values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 15f6c6b7-b5a9-4bd9-ae49-def75c52b1b3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
backend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/system/i18n/core/defaults.gofrontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/CommonElementPropertyFactory.test.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/RichTextAdapter.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/__tests__/Call.test.tsxfrontend/apps/console/src/features/flows/constants/VisualFlowConstants.tsfrontend/apps/console/src/features/flows/data/steps.jsonfrontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.tsfrontend/apps/console/src/features/flows/models/__tests__/steps.test.tsfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/flows/models/responses.tsfrontend/apps/console/src/features/flows/models/steps.tsfrontend/apps/console/src/features/flows/models/widget.tsfrontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.tsfrontend/apps/console/src/features/flows/utils/generateFlowGraph.tsfrontend/apps/console/src/features/flows/utils/reactFlowTransformer.tsfrontend/apps/console/src/features/flows/validation/validation-rules.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsxfrontend/apps/console/src/features/login-flow/data/widgets.jsonfrontend/packages/design/src/components/flow/FlowComponentRenderer.tsxfrontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsxfrontend/packages/i18n/src/locales/en-US.tspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (2)
- pnpm-workspace.yaml
- frontend/packages/i18n/src/locales/en-US.ts
🚧 Files skipped from review as they are similar to previous changes (30)
- frontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsx
- frontend/apps/console/src/features/flows/models/tests/steps.test.ts
- frontend/apps/console/src/features/flows/data/steps.json
- frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsx
- frontend/apps/console/src/features/flows/utils/generateFlowGraph.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx
- frontend/apps/console/src/features/flows/models/flows.ts
- frontend/apps/console/src/features/flows/models/widget.ts
- backend/internal/flow/flowexec/error_constants.go
- frontend/packages/design/src/components/flow/FlowComponentRenderer.tsx
- frontend/apps/console/src/features/flows/models/steps.ts
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/tests/RichTextAdapter.test.tsx
- frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/tests/RichTextActionFields.test.tsx
- frontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsx
- frontend/apps/console/src/features/flows/constants/VisualFlowConstants.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/tests/CallProperties.test.tsx
- backend/internal/system/i18n/core/defaults.go
- frontend/apps/console/src/features/flows/components/resources/steps/call/tests/Call.test.tsx
- frontend/apps/console/src/features/flows/models/responses.ts
- backend/internal/flow/flowexec/engine.go
- frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsx
- frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx
- backend/internal/flow/flowexec/engine_test.go
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsx
- frontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.ts
- frontend/apps/console/src/features/flows/validation/validation-rules.ts
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsx
- frontend/apps/console/src/features/flows/utils/reactFlowTransformer.ts
- frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts
- frontend/apps/console/src/features/login-flow/data/widgets.json
19fc5de to
6db13aa
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
790a9cc to
4cb91a7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx (1)
55-68: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRe-open the non-matching anchor hardening.
When
anyHasSentinelis true, Line 61 returns non-matching anchors unchanged, sotarget="_blank"can still survive without enforcedrel="noopener noreferrer". Normalize those attributes before returning.🔒 Suggested hardening
return html.replace(/<a\b([^>]*)>/gi, (match: string, attrs: string) => { + const withoutWindowAttrs = attrs + .replace(/\starget\s*=\s*(?:"[^"]*"|'[^']*')/gi, '') + .replace(/\srel\s*=\s*(?:"[^"]*"|'[^']*')/gi, ''); + if (anyHasSentinel) { const sentinelMatch = /\sdata-action-ref\s*=\s*(?:"([^"]*)"|'([^']*)')/i.exec(attrs); const anchorRef = sentinelMatch ? (sentinelMatch[1] ?? sentinelMatch[2]) : undefined; if (anchorRef !== actionRef) { - return match; + return `<a${withoutWindowAttrs}>`; } } - const stripped = attrs + const stripped = withoutWindowAttrs .replace(/\shref\s*=\s*(?:"[^"]*"|'[^']*')/gi, '') - .replace(/\starget\s*=\s*(?:"[^"]*"|'[^']*')/gi, '') - .replace(/\srel\s*=\s*(?:"[^"]*"|'[^']*')/gi, ''); return `<a${stripped} href="#">`; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx` around lines 55 - 68, In RichTextAdapter’s anchor normalization logic, the non-matching branch inside the html replacement still returns anchors unchanged when any sentinel is present, which can leave unsafe target/rel attributes intact. Update the anchor handling in the replace callback to normalize every anchor’s target and rel attributes before returning, including the branch that skips href rewriting for non-matching data-action-ref values, so unsafe target="_blank" links are consistently hardened.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@frontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsx`:
- Around line 55-68: In RichTextAdapter’s anchor normalization logic, the
non-matching branch inside the html replacement still returns anchors unchanged
when any sentinel is present, which can leave unsafe target/rel attributes
intact. Update the anchor handling in the replace callback to normalize every
anchor’s target and rel attributes before returning, including the branch that
skips href rewriting for non-matching data-action-ref values, so unsafe
target="_blank" links are consistently hardened.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fea1a16-ad22-4f29-9071-fcf638fd07be
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
backend/internal/flow/flowexec/engine.gobackend/internal/flow/flowexec/engine_test.gobackend/internal/flow/flowexec/error_constants.gobackend/internal/system/i18n/core/defaults.gofrontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/__tests__/CommonElementPropertyFactory.test.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsxfrontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/__tests__/RichTextActionFields.test.tsxfrontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsxfrontend/apps/console/src/features/flows/components/resources/elements/adapters/__tests__/RichTextAdapter.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsxfrontend/apps/console/src/features/flows/components/resources/steps/__tests__/CommonStepFactory.test.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsxfrontend/apps/console/src/features/flows/components/resources/steps/call/__tests__/Call.test.tsxfrontend/apps/console/src/features/flows/constants/VisualFlowConstants.tsfrontend/apps/console/src/features/flows/data/steps.jsonfrontend/apps/console/src/features/flows/hooks/__tests__/useVisualFlowHandlers.test.tsxfrontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.tsfrontend/apps/console/src/features/flows/models/__tests__/steps.test.tsfrontend/apps/console/src/features/flows/models/flows.tsfrontend/apps/console/src/features/flows/models/responses.tsfrontend/apps/console/src/features/flows/models/steps.tsfrontend/apps/console/src/features/flows/models/widget.tsfrontend/apps/console/src/features/flows/utils/__tests__/flowToCanvasTransformer.test.tsfrontend/apps/console/src/features/flows/utils/__tests__/reactFlowTransformer.test.tsfrontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.tsfrontend/apps/console/src/features/flows/utils/generateFlowGraph.tsfrontend/apps/console/src/features/flows/utils/reactFlowTransformer.tsfrontend/apps/console/src/features/flows/validation/validation-rules.tsfrontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/__tests__/ResourceProperties.test.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsxfrontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/__tests__/CallProperties.test.tsxfrontend/apps/console/src/features/login-flow/data/widgets.jsonfrontend/packages/design/src/components/flow/FlowComponentRenderer.tsxfrontend/packages/design/src/components/flow/adapters/RichTextAdapter.tsxfrontend/packages/i18n/src/locales/en-US.tspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (4)
- backend/internal/system/i18n/core/defaults.go
- frontend/apps/console/src/features/flows/utils/generateFlowGraph.ts
- pnpm-workspace.yaml
- frontend/packages/i18n/src/locales/en-US.ts
🚧 Files skipped from review as they are similar to previous changes (28)
- frontend/apps/console/src/features/flows/components/resources/elements/CommonElementFactory.tsx
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/tests/CallProperties.test.tsx
- frontend/apps/console/src/features/flows/models/tests/steps.test.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/tests/CommonElementPropertyFactory.test.tsx
- frontend/apps/console/src/features/flows/data/steps.json
- backend/internal/flow/flowexec/error_constants.go
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/ResourceProperties.tsx
- frontend/apps/console/src/features/flows/models/steps.ts
- frontend/packages/design/src/components/flow/FlowComponentRenderer.tsx
- frontend/apps/console/src/features/flows/constants/VisualFlowConstants.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/CommonElementPropertyFactory.tsx
- backend/internal/flow/flowexec/engine.go
- frontend/apps/console/src/features/flows/models/widget.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/tests/RichTextActionFields.test.tsx
- frontend/apps/console/src/features/flows/components/resources/steps/call/tests/Call.test.tsx
- frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts
- backend/internal/flow/flowexec/engine_test.go
- frontend/apps/console/src/features/flows/models/flows.ts
- frontend/apps/console/src/features/flows/models/responses.ts
- frontend/apps/console/src/features/flows/utils/reactFlowTransformer.ts
- frontend/apps/console/src/features/flows/components/resource-property-panel/rich-text/RichTextActionFields.tsx
- frontend/apps/console/src/features/flows/components/resources/steps/call/Call.tsx
- frontend/apps/console/src/features/flows/validation/validation-rules.ts
- frontend/apps/console/src/features/login-flow/components/resource-property-panel/extended-properties/CallProperties.tsx
- frontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.ts
- frontend/apps/console/src/features/flows/components/resources/steps/CommonStepFactory.tsx
- frontend/apps/console/src/features/flows/components/resources/elements/adapters/RichTextAdapter.tsx
- frontend/apps/console/src/features/login-flow/data/widgets.json
4cb91a7 to
3f9875e
Compare
Purpose
This pull request adds call node UI implementation to the flow builder UI and bumps
@thunderid/reactversion to onboard call node changes to the gate. This involves introducing call node widgets, steps and improvements to the rich text component to define action ref.Additionally this PR increases the max call depth limit to
10and modifies call node depth exceeding error to be a client error.Approach
Backend: Flow Execution Limits & Error Handling
Call depth limit and error reporting:
maxCallDepthfrom 5 to 10 to allow deeper flow nesting (engine.go).ErrorMaxCallDepthExceeded) with internationalized messages and changed the log level from error to debug when the call depth is exceeded (engine.go,error_constants.go,defaults.go) [1] [2] [3].Frontend: Rich Text Interactive Linking
Rich Text action configuration UI:
RichTextActionFieldscomponent, which provides a toggle and read-only field for connecting rich text elements to steps. Includes logic for enabling/disabling actions and handling edge connections (RichTextActionFields.tsx).RichTextActionFieldsinto the property panel, so it appears when editing a rich text label (CommonElementPropertyFactory.tsx) [1] [2].RichTextActionFields.test.tsx).Rich Text element rendering:
RichTextAdapterto render a source handle when an action is enabled, allowing authors to visually connect rich text elements to other steps. Ensured re-measurement of node internals when the action state changes and passedelementIndexfor handle positioning (RichTextAdapter.tsx,CommonElementFactory.tsx) [1] [2] [3] F33c821aL134R134, [4].Frontend: Step Factory
Support for Call steps:
Callstep type (CommonStepFactory.tsx) [1] [2].Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit