fix: correct end-of-init message logic for existing project + existing deployment#8318
Open
v1212 wants to merge 2 commits into
Open
fix: correct end-of-init message logic for existing project + existing deployment#8318v1212 wants to merge 2 commits into
v1212 wants to merge 2 commits into
Conversation
…g deployment The end message incorrectly recommended 'azd up' when the user selected an existing project with an existing deployment (no provisioning needed). Root cause: len(a.deploymentDetails) == 0 was used to detect whether new provisioning is needed, but existing deployments also populate this slice. Fix: Replace with an explicit 'needsProvision' boolean that is only set when a new model deployment is created (deploy-from-catalog path). This aligns init.go with the pattern already used in init_from_code.go. Follow-up to Azure#8292.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the “next steps” messaging at the end of azd ai agent init so it no longer recommends azd up when the user chose an existing Foundry project with an existing model deployment (i.e., when no new provisioning is required).
Changes:
- Introduces an explicit
needsProvisionflag onInitActionto represent whether the init flow configured a new model deployment that requires provisioning. - Updates the end-of-init message logic in
init.goto useneedsProvisioninstead oflen(deploymentDetails) == 0. - Sets
needsProvision = truewhen the flow falls through to “deploy new model deployment” logic ingetModelDeploymentDetails.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/cmd/init.go | Switches end-of-init guidance to rely on needsProvision (avoids misclassifying existing deployments as needing azd up). |
| cli/azd/extensions/azure.ai.agents/internal/cmd/init_models.go | Sets needsProvision when configuring a new model deployment (deploy-from-catalog / deploy-new path). |
Extract initCompletionNeedsDeploy() helper and add TestInitCompletionNeedsDeploy covering the truth table: - existing project + no new deployments -> azd deploy - existing project + new model deployed -> azd up - no project set -> azd up
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the end-of-init message incorrectly recommending
azd upwhen the user selected an existing project with an existing deployment (no provisioning needed).Root cause:
len(a.deploymentDetails) == 0was used to detect whether new provisioning is needed, but existing deployments also populate this slice.Fix: Replace with an explicit
needsProvisionboolean set only when a new model deployment is created (deploy-from-catalog path). This alignsinit.gowith the pattern already used ininit_from_code.go.Truth table
azd deployazd upazd deployazd upazd upFollow-up to #8292 — identified by @wbreza in re-review.