fix: Fix of bug #46192#615
Open
Shubhangi-Microsoft wants to merge 4 commits into
Open
Conversation
…azd env value Bug #46192: the postprovision hook trusted azd env get-value CONTENT_UNDERSTANDING_ACCOUNT_NAME blindly and passed it to az cognitiveservices account update. A stale value (e.g. from a prior deployment against a different template) made Azure CLI emit a noisy ResourceNotFound block before the script swallowed the exit code, surfacing visible errors even though azd up ultimately succeeded. infra/scripts/post_deployment.sh: - Silent existence probe (2>/dev/null) before using the env value. - Fallback discovery via az cognitiveservices account list --query "[?kind=='AIServices'].name | [0]". - azd env set self-heals the env on successful discovery so subsequent runs are clean. - Refresh failure is now a non-fatal warning instead of a hard error. infra/scripts/post_deployment.ps1: - Added the previously-missing CU refresh block with the same three-stage logic adapted to PowerShell. Windows users were silently skipping the refresh step entirely before. Verified by reproducing the stale-value condition in a live azd env and confirming azd hooks run postprovision completes cleanly with no ResourceNotFound, the env value is auto-corrected, and the Cognitive Services account receives the refresh=true tag. Refs ADO #46192, #46576, #46577 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nge) Re-compiling infra/main.bicep with bicep CLI 0.43.8.12551 (was 0.42.1.51946) produces: - 48 lines of metadata stamp churn (compiler version + templateHash) - 4 lines of cosmetic ordering change in a dependsOn array (storageQueue and aiServices entries swap positions). ARM dependsOn arrays are unordered and resolved topologically, so the deployed graph is identical. No resources, parameters, outputs, modules, or logic changed. This keeps infra/main.json in sync with infra/main.bicep for the GitHub Actions deploy workflow (which uses main.json as its ARM template). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates the post-deployment scripts to more robustly refresh the Content Understanding (Azure AI Services) Cognitive Services account by validating the azd environment value and attempting discovery when it’s missing/stale, and refreshes the generated ARM template (infra/main.json) to reflect an updated Bicep compiler version/hash and minor dependency ordering adjustments.
Changes:
- Add validation + fallback discovery of the
CONTENT_UNDERSTANDING_ACCOUNT_NAMEin bothpost_deployment.shandpost_deployment.ps1, and update the refresh step to be non-fatal. - Update Bicep generator metadata (version/templateHash) across
infra/main.json. - Minor
dependsOnordering adjustments ininfra/main.json.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| infra/scripts/post_deployment.sh | Validates env-provided account name, discovers AIServices account when missing/stale, and refreshes it post-deploy. |
| infra/scripts/post_deployment.ps1 | PowerShell equivalent of the same env validation/discovery + non-fatal refresh behavior. |
| infra/main.json | Regenerated ARM template metadata (Bicep version/hash) and small dependency ordering tweaks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…xist Addresses Copilot review feedback on PR #615: enumerate all AIServices accounts in the resource group instead of blindly picking the first via `| [0]`. When exactly one is found, auto-recover and persist it to azd env. When multiple are found, emit a warning listing the candidates and ask the user to set CONTENT_UNDERSTANDING_ACCOUNT_NAME explicitly -- this prevents persisting the wrong account name into azd env, which would cause subsequent runs to refresh the wrong resource. Restructured both bash and PowerShell discovery blocks into a single if/elif/else chain so the "no account found" warning no longer prints a second time after the multi-account branch fires. Verified with mocked az/azd stubs across 0, 1, and 3-account scenarios: each path now prints exactly one outcome message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shubhangi-Microsoft
added a commit
that referenced
this pull request
Jun 9, 2026
…erade as resource-not-found Addresses round-2 Copilot review feedback on PR #615. - `az cognitiveservices account show` previously discarded stderr and treated any non-zero exit as `account not found` - a transient auth or CLI error could wrongly clear CU_ACCOUNT_NAME and trigger discovery that persists a different account into azd env. Now we capture stderr, only mark stale when Azure actually reports the resource is missing (ResourceNotFound / was not found / could not be found), and otherwise log the error and keep the env value. - `az cognitiveservices account update` previously suppressed stderr too, hiding the actual CLI error on the non-fatal failure path. Now we capture and surface it in the warning message so deployment logs contain enough context to diagnose refresh failures. Validated with a mock harness covering 7 presets (none, one, multi, stale-real, stale-trans, ok, update-fail); `bash -n` and the PowerShell AST parser both pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…erade as resource-not-found Addresses Copilot review feedback on PR #615. stderr capture - `az cognitiveservices account show` previously discarded stderr and treated any non-zero exit as `account not found` - a transient auth or CLI error could wrongly clear CU_ACCOUNT_NAME and trigger discovery that persists a different account into azd env. Now we capture stderr, only mark stale when Azure actually reports the resource is missing (ResourceNotFound / was not found / could not be found), and otherwise log the error and keep the env value. - `az cognitiveservices account update` previously suppressed stderr too, hiding the actual CLI error on the non-fatal failure path. Now we capture and surface it in the warning message so deployment logs contain enough context to diagnose refresh failures. set -e safety - The script runs with `set -e`, so a failed `var=$(cmd)` would exit immediately, before we could inspect $?. Wrap the two new capture blocks with `set +e` / `set -e` so the error-handling paths actually run. bash 3.2 portability (macOS default) - Replace `mapfile` (bash 4+) with a portable `while IFS= read -r` loop so the discovery branch works on macOS too. Validated with a 7-preset harness (none, one, multi, stale-real, stale-trans, ok, update-fail) running the on-disk script under `set -e` against stubbed az/azd; `bash -n` passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5a44bda to
0b5d1a7
Compare
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.
This pull request introduces improvements to the deployment scripts and infrastructure template for better reliability and maintainability, particularly around the handling of the Content Understanding Cognitive Services account. The most significant changes are enhancements to the post-deployment scripts to robustly detect and refresh the correct Cognitive Services account, as well as minor updates to the Bicep-generated infrastructure template.
Deployment script improvements:
post_deployment.ps1andpost_deployment.shnow verify that theCONTENT_UNDERSTANDING_ACCOUNT_NAMEenvironment variable points to an existing Cognitive Services account. If not, they attempt to discover the correctAIServicesaccount in the resource group and update the environment variable accordingly. This prevents issues caused by stale or missing environment values and ensures the correct account is refreshed. [1] [2]Infrastructure template updates:
Updated the Bicep compiler version and template hashes throughout
infra/main.jsonto reflect the latest tooling and ensure template consistency. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Minor reordering of dependencies in resource arrays within
infra/main.jsonto maintain correct deployment sequencing and consistency. [1] [2]## PurposeDoes this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
Verify that the following are valid
Other Information