Skip to content

Conversation

@anthony0t
Copy link
Contributor

@anthony0t anthony0t commented Jan 8, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Adding support for dynamic list in mcp tools so user can choose the tools they want, rather than having to manually type in the tools which is difficult for them. This PR adds the capability. The capability is purely additive and doesn't impact existing features.

Impact of Change

  • Users:
    User will be able to choose the tools now instead of manually type them in.
  • Developers:
    if dynamic list on MCP client tool feature is needed, workflowName needs to be passed to connector service, and getConfiguration callback needs to support new optional parameters. If this feature is not needed, no change is needed.
  • System:

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: local host

Contributors

Screenshots/Videos

Copilot AI review requested due to automatic review settings January 8, 2026 19:43
@github-actions
Copy link

github-actions bot commented Jan 8, 2026

🤖 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: (feat:designer) Support dynamic list for MCP client tool actions
  • Issue: Title is descriptive but deviates slightly from conventional commit formatting and can be shortened/clarified. The leading parentheses style is unusual; prefer the conventional commit scope format with no parentheses-wrapped prefix (e.g., feat(designer): ...).
  • Recommendation: Use a conventional, concise title. Example: feat(designer): add dynamic list support for MCP client tools

Commit Type

  • Properly selected (feature).
  • Note: Only one commit type is selected which is correct.

Risk Level

  • The PR body and labels claim Low risk, and the repo has the risk:low label applied.
  • Based on the code changes (adds new dynamic-list behavior for MCP connections, changes to StandardConnectorService signatures/options, new runtime requirement for workflowName, and changes across designer and shared libs), the realistic advised risk level is Medium.
  • Recommendation: Update the PR Risk Level selection and label to Medium (e.g., check Medium in the PR body and add risk:medium label). Add justification in the body noting that the change introduces new runtime flows and a required workflowName for MCP flows.

What & Why

  • Current: "Adding support for dynamic list in mcp tools so user can choose the tools they want... The capability is purely additive and doesn't impact existing features."
  • Issue: Good concise summary. However, because code introduces a runtime requirement (workflowName must be provided for MCP connections) and modifies connector behavior, the PR's "purely additive" claim should explicitly mention that the new MCP flow will require configuration (workflowName) and may change dynamic-list fetching behavior for MCP-type dynamicState.
  • Recommendation: Expand the What & Why to note:
    • The new MCP dynamic list calls a workflow endpoint (requires workflowName).
    • The connectorService now optionally uses managed/agent MCP connections and strips connectionRuntimeUrl when calling managed endpoints.
    • Note any backward-compatibility expectations (e.g., non-MCP flows are unchanged).

⚠️ Impact of Change

  • The Impact section contains core points for Users and Developers but leaves System blank.
  • Recommendation: Fill the System section and make the impacts explicit:
    • Users: Users can select MCP tools from a dynamic list instead of typing them.
    • Developers: New optional parameter operationPath added on OperationInfo; getConfiguration now accepts a useManagedConnections boolean; StandardConnectorService options now accept an optional workflowName and behavior depends on it.
    • System: Explain that a new workflow endpoint is invoked for MCP dynamic lists (e.g., /workflows/{workflowName}/listMcpTools) which could affect runtime requests and metrics. Note the additional HTTP call and that connection properties may be sanitized (connectionRuntimeUrl removed).

Test Plan

  • The PR claims unit tests added/updated and manual testing completed. I checked the diff and a unit test file was added: libs/logic-apps-shared/src/designer-client-services/lib/standard/__tests__/connector.spec.ts which covers MCP and non-MCP dynamic value flows (including missing workflowName error). Good coverage for the new logic.
  • Recommendation: If applicable, add or mention e2e coverage or provide rationale why E2E tests are not required (e.g., requires environment or infra). If the MCP flow depends on runtime APIs, an E2E or integration test in a pipeline would help catch runtime regressions.

⚠️ Contributors

  • The Contributors section is empty.
  • Recommendation: Add any PMs/designers/peers who contributed, or at least add a short note reminding reviewers of design input. This is optional but recommended to give credit.

⚠️ Screenshots/Videos

  • No screenshots/videos provided. If this change affects UX (it does — dynamic list in an editor), consider adding a quick GIF or screenshot showing the dynamic list in action. If purely API-level, this is optional.

Summary Table

Section Status Recommendation
Title ⚠️ Use conventional commit style and tighten wording (e.g., feat(designer): add dynamic list support for MCP client tools)
Commit Type None
Risk Level Change to Medium and update label & PR body to reflect runtime impact and workflow requirement
What & Why Expand to call out workflowName requirement and backward-compat expectations
Impact of Change ⚠️ Fill System section and call out runtime HTTP call and sanitization of connection properties
Test Plan Unit tests present. Consider E2E/integration justification or add an integration test if possible
Contributors ⚠️ Add contributors or a short credit line (optional but recommended)
Screenshots/Videos ⚠️ Add visual proof if this affects the editor UX (recommended)

Final notes

  • Advised risk level: risk:medium — this is higher than the risk:low currently selected. Please update the PR risk selection and label to reflect this. The reasoning: the PR touches runtime connector flows (StandardConnectorService), adds a required runtime context (workflowName) for MCP flows, and changes operation metadata and dynamic-list plumbing across multiple core libraries. These are additive but introduce new runtime behavior and a failing condition if workflowName is missing.
  • Title: please adopt conventional commit style and make it concise as suggested.
  • Body: explicitly document the new runtime requirement (workflowName) and any deployment/upgrade guidance (e.g., ensure services that instantiate StandardConnectorService pass workflowName). Also include a short note about backward compatibility.

Please update the PR title, the Risk Level selection in the PR body (and add the risk:medium label), and expand the Impact/System details. Once updated, this PR looks good to merge from a PR-body/template perspective.


Last updated: Fri, 09 Jan 2026 04:55:34 GMT

@anthony0t anthony0t added the risk:low Low risk change with minimal impact label Jan 8, 2026
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 pull request adds support for dynamic list functionality in MCP (Model Context Protocol) tools, enabling users to select tools from a dropdown rather than manually typing tool names. The feature is implemented but currently disabled (commented out in the manifest) pending backend deployment.

Key Changes

  • Extended operation metadata to include operationPath for MCP operations
  • Implemented dynamic tool listing via the listMcpTools API endpoint
  • Updated connection configuration handling to support both agent-based and managed MCP connections

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
libs/logic-apps-shared/src/utils/src/lib/models/operationmanifest.ts Added optional operationPath field to OperationInfo interface
libs/logic-apps-shared/src/designer-client-services/lib/standard/manifest/builtinmcpclient.ts Updated manifest with commented-out dynamic list configuration for MCP tools
libs/logic-apps-shared/src/designer-client-services/lib/standard/connector.ts Added MCP-specific logic to _listDynamicValues method and updated service options to include workflowName
libs/logic-apps-shared/src/designer-client-services/lib/connector.ts Extended IConnectorService interface to include operationPath parameter
libs/designer/src/lib/core/utils/parameters/dynamicdata.ts Updated to pass operationPath to dynamic values queries
libs/designer/src/lib/core/state/operation/operationMetadataSlice.ts Modified to store operationPath in operation info state
libs/designer/src/lib/core/queries/connector.ts Updated query to include operationPath parameter
libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts Updated to initialize operation info with operationPath
libs/designer/src/lib/core/actions/bjsworkflow/add.ts Added logic to extract and store operationPath from swagger for managed MCP clients
libs/designer-v2/src/lib/core/utils/parameters/dynamicdata.ts Updated to pass operationPath to dynamic values queries (v2)
libs/designer-v2/src/lib/core/state/operation/operationMetadataSlice.ts Modified to store operationPath in operation info state (v2)
libs/designer-v2/src/lib/core/queries/connector.ts Updated query to include operationPath parameter (v2)
libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts Updated to initialize operation info with operationPath (v2)
libs/designer-v2/src/lib/core/actions/bjsworkflow/test/operationdeserializer.spec.ts Added test assertion for operationPath in operation info
apps/Standalone/src/templates/app/TemplatesStandard.tsx Added required workflowName parameter (empty string) to connector service
apps/Standalone/src/mcp/app/McpStandard.tsx Added required workflowName parameter (empty string) to connector service
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesignerV2.tsx Updated connection configuration function and modified readonly logic
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesigner.tsx Updated connection configuration to support MCP connections

@github-actions
Copy link

github-actions bot commented Jan 8, 2026

📊 Coverage check completed. See workflow run for details.

@anthony0t anthony0t changed the title Support dynamic list for mcp tool Support dynamic list for MCP client tool actions Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@Azure Azure deleted a comment from Copilot AI Jan 8, 2026
@github-actions
Copy link

github-actions bot commented Jan 8, 2026

📊 Coverage check completed. See workflow run for details.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

📊 Coverage check completed. See workflow run for details.

1 similar comment
@github-actions
Copy link

github-actions bot commented Jan 9, 2026

📊 Coverage check completed. See workflow run for details.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

📊 Coverage check completed. See workflow run for details.

@github-actions
Copy link

github-actions bot commented Jan 9, 2026

📊 Coverage check completed. See workflow run for details.

@anthony0t anthony0t changed the title Support dynamic list for MCP client tool actions (feat:designer) Support dynamic list for MCP client tool actions Jan 9, 2026
@anthony0t anthony0t requested a review from takyyon January 9, 2026 05:53
@anthony0t anthony0t merged commit e812708 into main Jan 9, 2026
15 checks passed
@anthony0t anthony0t deleted the tonytang/mcptool branch January 9, 2026 17:02
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.

3 participants