Skip to content

Conversation

@hartra344
Copy link
Collaborator

@hartra344 hartra344 commented Dec 8, 2025

Commit Type

  • feature - New functionality

Risk Level

  • Low - Minor changes, limited scope

What & Why

Previously, Agent Loop nodes could only be added at the root level of agentic/A2A workflows. This change removes that restriction with a version gate:

Changes:

  1. Search panel filtering (searchView.tsx): Agent operations now check enableNestedAgentLoops flag when not at root
  2. Context menu (EdgeContextualMenu.tsx): "Add an agent" button visibility now checks the flag when not at root
  3. Version gating: Added enableNestedAgentLoops hostOption that requires bundle version >= 1.115.0

Files modified:

  • libs/designer/src/lib/core/state/designerOptions/designerOptionsInterfaces.ts - Added new hostOption
  • libs/designer/src/lib/core/state/designerOptions/designerOptionsSelectors.ts - Added selector hook
  • libs/designer/src/lib/ui/panel/recommendation/searchView.tsx - Check flag in filter
  • libs/designer/src/lib/ui/common/EdgeContextualMenu/EdgeContextualMenu.tsx - Check flag for button visibility
  • Same changes in libs/designer-v2/
  • apps/vs-code-react/src/app/designer/app.tsx & appV2.tsx - Version check using isVersionSupported

Impact of Change

  • Users: Users with bundle version >= 1.115.0 can add Agent Loop nodes inside FOREACH/UNTIL loops in agentic workflows
  • Developers: New enableNestedAgentLoops hostOption available
  • System: No performance or architecture impact

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Standalone designer - verified Agent Loop appears in both operation search and context menu when inside a FOREACH loop (with flag enabled)

Contributors

Screenshots/Videos

Remove the root-level restriction that prevented Agent Loop nodes from
being added inside FOREACH/UNTIL loops in agentic workflows.
Copilot AI review requested due to automatic review settings December 8, 2025 20:00
@github-actions
Copy link

github-actions bot commented Dec 8, 2025

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(designer): Enable nesting Agent Loop nodes in FOREACH/UNTIL control flows in designer
  • Issue: None — title is clear, follows conventional style, and explains what and where is changed.
  • Recommendation: Keep as-is. It's descriptive and good.

Commit Type

  • Properly selected (feature) in the PR body.
  • Note: The PR contains commits that also include refactor/chore items (e.g., centralizing version constants). That's OK — the PR-level commit type selection of feature is appropriate since the main intent is a new capability.

Risk Level

  • The PR body marks Low and the repo label is risk:low — this matches and is reasonable because the capability is behind a host option and a bundle-version gate.
  • Recommendation: Keep risk:low label. However, see the note below about a potential version-value mismatch that could affect rollout (operational risk, not code-safety risk).

⚠️ What & Why

  • Current: The PR explains the removal of the root-only restriction for Agent Loop nodes and lists changed files and feature gating.
  • Issue: There is an inconsistency between the PR description and the code regarding the minimum bundle version:
    • PR description (What & Why) and commit text mention requiring bundle version >= 1.115.0.
    • The new BundleVersionRequirements constant sets NESTED_AGENT_LOOPS to 2.0.0 (placeholder) in libs/logic-apps-shared/src/utils/src/lib/helpers/version.ts.
  • Recommendation: Make the version requirement consistent and explicit. Choose one of the following and update code + PR body to match:
    • If the real minimum is 1.115.0: set BundleVersionRequirements.NESTED_AGENT_LOOPS = '1.115.0' and update the PR body to reflect that value.
    • If 2.0.0 is intentionally a placeholder: call that out in the PR body (clear TODO) and file a follow-up to set the actual version before release. Example line for What & Why: "Version gating uses BundleVersionRequirements.NESTED_AGENT_LOOPS = '2.0.0' (placeholder) — will be updated to actual bundle min version before release — see issue #XXXX."

Impact of Change

  • Impact described in PR body. Good separation of Users/Developers/System.
  • Recommendation: Add one short clarification under Users to explicitly state default behavior for existing users (i.e., nothing changes unless hostOption is set or bundle meets requirement). Example bullet you can add:
    • Users: No change by default; nested Agent Loops are available only when hostOptions.enableNestedAgentLoops is true and the extension bundle version meets the minimum requirement.

⚠️ Test Plan

  • Current: Manual testing completed and marked. Unit and E2E tests are not included.
  • Issue: No automated tests added. This is a UI/behavior gating change that benefits from test coverage.
  • Recommendation: Add at least the following tests (or open follow-up work items if tests will be added in a follow-up PR):
    • Unit tests for the new selector useEnableNestedAgentLoops.
    • Unit tests for SearchView filtering logic to assert that Agent nodes are excluded/included depending on isRoot, workflow type, and enableNestedAgentLoops flag.
    • Unit tests for EdgeContextualMenu to ensure the Add Agent menu item is shown only when graphId === 'root' OR enableNestedAgentLoops is true (and only for agentic/A2A workflows).
    • If possible, a small E2E or integration test that simulates a designer session with a mocked hostOptions object and checks menu + search behavior.
      If adding tests now is not feasible, mark the PR with a TODO and link to an issue tracking adding automated coverage.

⚠️ Contributors

  • Current: Empty section in PR body.
  • Recommendation: Optional but recommended: add any PMs, designers, reviewers who contributed decisions or verification. If none to add, consider a short comment in the PR body: Contributors: (none to add) to avoid blankness.

⚠️ Screenshots/Videos

  • Current: Marked N/A in PR body.
  • Assessment: Acceptable. This change mostly affects filtering/visibility logic rather than new visuals. No screenshots required.

Summary Table

Section Status Recommendation
Title Keep as-is.
Commit Type Keep as-is.
Risk Level Keep risk:low, but review version gating note.
What & Why ⚠️ Fix the bundle-version inconsistency (1.115.0 vs 2.0.0) and document placeholder if intended.
Impact of Change Add one-line clarifying default behavior for users.
Test Plan ⚠️ Add unit tests for selector + search/menu behavior; consider E2E.
Contributors ⚠️ Add contributors or a short note if none.
Screenshots/Videos N/A is fine.

Final notes & actionable recommendations

  1. Version gating mismatch (HIGH priority to fix before release):
    • Update BundleVersionRequirements.NESTED_AGENT_LOOPS to the intended min bundle version (e.g., 1.115.0) or clearly mark the 2.0.0 value as a placeholder in the PR body and create a follow-up to set the final value.
    • Ensure PR body What & Why and the code agree on the version requirement — this avoids rollout surprises where the feature remains blocked (if code requires 2.0.0) or is enabled earlier than intended.
  2. Tests (recommended before larger rollout):
    • Add unit tests for the new selector and the conditional logic in SearchView and EdgeContextualMenu.
    • If not possible in this PR, create an issue linking to automated test coverage to add soon.
  3. Localization change:
    • You updated the localization id for the "Add an MCP server (preview)" string (moved from JTy5al to kIei9R). Confirm this is intentional and inform localization owners so translations are preserved/merged correctly.
  4. Small UX/PR body improvements:
    • Add the short sentence clarifying default behavior (nested loops off unless hostOption true and version matches).
    • Add contributors list or a short note that none need to be credited.

Please update the PR body to either correct the version constant or call out the placeholder explicitly, and add or track tests for the new gating logic. Once the version mismatch is clarified and tests are planned/added, this PR is in good shape to land.

Overall: This PR passes the PR-body/title checks with one important action item: reconcile the bundle version number between the PR description and the code (or document the placeholder) and add/track tests for the gating behavior. Thanks for the clear description and good structure!


Last updated: Mon, 15 Dec 2025 18:24:35 GMT

@hartra344 hartra344 added the risk:low Low risk change with minimal impact label Dec 8, 2025
@hartra344 hartra344 changed the title feat(designer): Allow Agent Loop nodes inside regular loops Enable nesting Agent Loop nodes in FOREACH/UNTIL control flows in designer Dec 8, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the restriction that prevented Agent Loop nodes from being nested inside regular control flow loops (FOREACH, UNTIL) in agentic and A2A workflows. Previously, Agent Loop nodes could only be added at the root level; now they can be placed anywhere within these workflow types.

Key Changes:

  • Simplified the filtering logic to only check workflow type (agentic/A2A) instead of both workflow type and node position
  • Removed the isRoot condition that was preventing nested Agent Loop nodes

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
libs/designer/src/lib/ui/panel/recommendation/searchView.tsx Updated agent operation filter to remove root position check
libs/designer-v2/src/lib/ui/panel/recommendation/searchView.tsx Updated agent operation filter to remove root position check (v2 implementation)

Remove the root-level restriction from the EdgeContextualMenu that
prevented the "Add an agent" button from appearing inside loops.
Add enableNestedAgentLoops hostOption that requires bundle version
>= 1.115.0 to allow Agent Loop nodes inside regular loops.

Changes:
- Add enableNestedAgentLoops to hostOptions interface (designer/designer-v2)
- Add useEnableNestedAgentLoops selector hook
- Update searchView.tsx to check the flag before showing Agent in search
- Update EdgeContextualMenu.tsx to check the flag before showing Agent button
- VS Code apps calculate the flag based on bundle version
Add BundleVersionRequirements constant object in version.ts to centralize
minimum version requirements for features:
- MULTI_VARIABLE: '1.114.22'
- NESTED_AGENT_LOOPS: '1.115.0'

Update VS Code apps to use constants instead of hardcoded strings.
@hartra344 hartra344 enabled auto-merge (squash) December 9, 2025 17:28
@hartra344 hartra344 merged commit a35c86e into main Dec 9, 2025
12 checks passed
@hartra344 hartra344 deleted the feat/allow-agent-loops-in-loops branch December 9, 2025 17:35
@Eric-B-Wu Eric-B-Wu changed the title Enable nesting Agent Loop nodes in FOREACH/UNTIL control flows in designer fix(designer): Enable nesting Agent Loop nodes in FOREACH/UNTIL control flows in designer Dec 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants