Skip to content

fix(designer): restore deploymentModelProperties for AzureOpenAI agent connections#9108

Open
takyyon wants to merge 1 commit into
mainfrom
fix/agent-azureopenai-deployment-model-properties
Open

fix(designer): restore deploymentModelProperties for AzureOpenAI agent connections#9108
takyyon wants to merge 1 commit into
mainfrom
fix/agent-azureopenai-deployment-model-properties

Conversation

@takyyon
Copy link
Copy Markdown
Contributor

@takyyon takyyon commented Apr 23, 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

Regression from PR #9012 (merged April 6) which removed AzureOpenAI from the visibility dependencies of deploymentModelProperties in the agent loop manifest. This causes saving agent workflows with AzureOpenAI model type to fail with:

InvalidAgentModelSettings: The 'name' parameter value can't be null or empty for 'AzureOpenAI' model type.

PR #9012 assumed the backend derives model info from the deployment for AzureOpenAI. The backend does not — it requires deploymentModelProperties.name, .format, .version explicitly.

Root Cause

The manifest at agentloop.ts had its deploymentModelProperties fields (name, format, version) restricted to values: ['MicrosoftFoundry'] only. When agentModelType is AzureOpenAI, these fields were hidden by the conditional visibility system and never serialized.

Fix

  1. Manifest (agentloop.ts): Restore AzureOpenAI in the visibility values array for all three deploymentModelProperties fields
  2. parametersTab (both designer + designer-v2): Restore the deployment-info lookup for AzureOpenAI so model properties are auto-populated from the ARM deployment response when a user selects a deployment
  3. Test: Added regression tests verifying deploymentModelProperties visibility includes both AzureOpenAI and MicrosoftFoundry

MicrosoftFoundry behavior is unchanged.

Impact of Change

  • Users: AzureOpenAI agent connections can be saved again. MicrosoftFoundry behavior unchanged.
  • Developers: No API changes.
  • System: No performance impact.

Test Plan

  • Unit tests added/updated
  • Manual testing completed
  • Tested in: agentloop.spec.ts (23/23 pass including 6 new regression tests)

Contributors

@takyyon

Screenshots/Videos

N/A

…t connections

Regression from #9012 which removed AzureOpenAI from the visibility
dependencies of deploymentModelProperties (name, format, version) in
the agent loop manifest. The assumption was that the backend derives
model info from the deployment — but the backend requires these values
explicitly, failing with InvalidAgentModelSettings when they're missing.

Changes:
- Restore AzureOpenAI in agentloop.ts manifest visibility for
  deploymentModelProperties.name, .format, .version
- Restore AzureOpenAI deployment-info lookup in parametersTab
  (both designer and designer-v2) so model properties are populated
  from the ARM deployment response when selecting a deployment
- Add regression tests verifying deploymentModelProperties visibility
  includes both AzureOpenAI and MicrosoftFoundry

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 23, 2026 22:35
@github-actions
Copy link
Copy Markdown
Contributor

🤖 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): restore deploymentModelProperties for AzureOpenAI agent connections
  • Issue: None — title is clear, scoped, and follows the conventional commit style.
  • Recommendation: Keep as-is. If you want to be extra explicit you can include the file/area changed (e.g., agentloop, parametersTab) but it's not required.

Commit Type

  • Properly selected (fix).
  • Only one option is selected which is correct.

Risk Level

  • Assessment: The PR body selects Medium risk, which is reasonable given UI + manifest changes and added tests. However, the repository PR labels do not include a risk label (expected risk:low, risk:medium, or risk:high). Every PR must have a matching risk label and the label must match the risk selected in the body.
  • Recommendation: Add the risk:medium label to this PR. If another reviewer or automation expects a different label, update the PR labels to match the body. If you believe the risk should be different, update the risk selection in the PR body to match the chosen label.

What & Why

  • Current: Regression from PR fix(designer): Only populate deploymentModelProperties for MicrosoftFoundry #9012 removed AzureOpenAI visibility for deploymentModelProperties; restored AzureOpenAI in manifest visibility and added UI fix to populate deploymentModelProperties; tests added.
  • Issue: None — the description is concise, explains root cause and fix steps.
  • Recommendation: Keep the current content. It is clear and actionable.

Impact of Change

  • No issues found: the impact section clearly indicates Users, Developers, and System impacts.
  • Recommendation: (optional) If you want to be more specific, mention which components or subsystems consume the agent loop manifest (e.g., designer, designer-v2) — but current content is adequate.

Test Plan

  • Assessment: Unit tests added (agentloop.spec.ts), manual testing completed, and the PR body references the test file and counts.
  • Recommendation: Ensure CI passes and the new regression tests are included in the test run. The diff shows the new tests present, so this section is consistent with changes.

Contributors

  • Assessment: Contributors section includes @takyyon which is fine.
  • Recommendation: Nothing required — credit is present.

Screenshots/Videos

  • Assessment: N/A — no UI screenshots necessary for this kind of fix and you marked it N/A.
  • Recommendation: No change required.

Summary Table

Section Status Recommendation
Title Keep the current descriptive title.
Commit Type commit type set to fix — correct.
Risk Level Add risk:medium label to the PR to match body.
What & Why Good description — no action required.
Impact of Change Clear — no action required.
Test Plan Tests were added; ensure CI passes.
Contributors Contributor listed — OK.
Screenshots/Videos N/A is acceptable for this change.

Final Notes

  • The single actionable failure here is the missing risk label on the PR. Your selected risk level in the PR body is Medium, and the code changes (manifest + UI + tests) align with that risk assessment. Please add the repository label risk:medium to this PR so labels and body match. After adding the label, re-run any required status checks if your repo requires it.

  • Everything else looks good: title, commit type, WHAT/WHY, Impact, and Test Plan (unit test added) are all consistent with the change.

Please update the PR by adding the risk:medium label, ensure CI passes, and re-request review if needed. Thank you for the clear description and the included regression tests!


Last updated: Thu, 23 Apr 2026 22:36:46 GMT

Copy link
Copy Markdown
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

Fixes a regression where AzureOpenAI agent connections could not be saved because deploymentModelProperties (name/format/version) were conditionally hidden and therefore not serialized/populated.

Changes:

  • Restores AzureOpenAI visibility dependencies for agentModelSettings.deploymentModelProperties.{name,format,version} in the agent loop manifest.
  • Updates Parameters Tab logic (designer + designer-v2) to populate deploymentModelProperties for AzureOpenAI as well as MicrosoftFoundry when a deployment is selected.
  • Adds unit coverage to prevent future regressions in manifest visibility conditions.

Reviewed changes

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

File Description
libs/logic-apps-shared/src/designer-client-services/lib/standard/manifest/agentloop.ts Re-adds AzureOpenAI to visibility dependencies for deployment model property fields so they can serialize.
libs/logic-apps-shared/src/designer-client-services/lib/standard/manifest/test/agentloop.spec.ts Adds regression tests asserting visibility values include AzureOpenAI + MicrosoftFoundry (and exclude V2).
libs/designer/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx Populates deploymentModelProperties for AzureOpenAI on deployment selection.
libs/designer-v2/src/lib/ui/panel/nodeDetailsPanel/tabs/parametersTab/index.tsx Same as above for designer-v2.

Comment on lines +1099 to +1103
// Populate deploymentModelProperties for both AzureOpenAI and MicrosoftFoundry.
// The backend requires name/format/version for AzureOpenAI model type.
if ((currentModelType === 'MicrosoftFoundry' || currentModelType === 'AzureOpenAI') && selectedModelId) {
const deploymentInfo = deploymentsForCognitiveServiceAccount?.find((deployment: any) => deployment.name === selectedModelId);
const modelName = deploymentInfo?.properties?.model?.name;
const modelName = deploymentInfo?.properties?.model?.name ?? selectedModelId;
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

Since deploymentModelProperties are now required/populated for AzureOpenAI too, this handler can still end up with empty deploymentModelProperties when agentModelType switches away from MicrosoftFoundry (the earlier logic clears these fields whenever newValue !== 'MicrosoftFoundry'). Because this populate logic only runs when the deploymentId field changes, the cleared values may never be repopulated (e.g., if deploymentId remains unchanged / is set programmatically). Update the clearing condition to keep these fields for AzureOpenAI as well, or trigger repopulation based on the currently-selected deploymentId after model type changes.

Copilot uses AI. Check for mistakes.
Comment on lines +1090 to +1094
// Populate deploymentModelProperties for both AzureOpenAI and MicrosoftFoundry.
// The backend requires name/format/version for AzureOpenAI model type.
if ((currentModelType === 'MicrosoftFoundry' || currentModelType === 'AzureOpenAI') && selectedModelId) {
const deploymentInfo = deploymentsForCognitiveServiceAccount?.find((deployment: any) => deployment.name === selectedModelId);
const modelName = deploymentInfo?.properties?.model?.name;
const modelName = deploymentInfo?.properties?.model?.name ?? selectedModelId;
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

Same issue as designer v1: deploymentModelProperties are now populated for AzureOpenAI here, but the earlier agentModelType-change handler still clears deploymentModelProperties whenever newValue !== 'MicrosoftFoundry'. Because this block only runs on a user change to deploymentId, those cleared values may not be repopulated (e.g., if deploymentId stays the same or is updated via dispatch). Adjust the clearing condition to exclude AzureOpenAI, and/or add a repopulation step when model type changes and a deployment is already selected.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

📊 Coverage Check

No source files changed in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants