Skip to content

fix: Fix of bug #46192#615

Open
Shubhangi-Microsoft wants to merge 4 commits into
devfrom
psl-bug46192
Open

fix: Fix of bug #46192#615
Shubhangi-Microsoft wants to merge 4 commits into
devfrom
psl-bug46192

Conversation

@Shubhangi-Microsoft

Copy link
Copy Markdown
Contributor

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:

  • Both post_deployment.ps1 and post_deployment.sh now verify that the CONTENT_UNDERSTANDING_ACCOUNT_NAME environment variable points to an existing Cognitive Services account. If not, they attempt to discover the correct AIServices account 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.json to 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.json to maintain correct deployment sequencing and consistency. [1] [2]## Purpose

  • ...

Does this introduce a breaking change?

  • Yes
  • No

Golden Path Validation

  • I have tested the primary workflows (the "golden path") to ensure they function correctly without errors.

Deployment Validation

  • I have validated the deployment process successfully and all services are running as expected with this change.

What to Check

Verify that the following are valid

  • ...

Other Information

Shubhangi-Microsoft and others added 2 commits June 9, 2026 14:38
…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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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_NAME in both post_deployment.sh and post_deployment.ps1, and update the refresh step to be non-fatal.
  • Update Bicep generator metadata (version/templateHash) across infra/main.json.
  • Minor dependsOn ordering adjustments in infra/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.

Comment thread infra/scripts/post_deployment.sh
Comment thread infra/scripts/post_deployment.ps1
@Shubhangi-Microsoft Shubhangi-Microsoft changed the title Fix of bug #46192 fix: Fix of bug #46192 Jun 9, 2026
…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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread infra/scripts/post_deployment.sh
Comment thread infra/scripts/post_deployment.sh Outdated
Comment thread infra/scripts/post_deployment.ps1
Comment thread infra/scripts/post_deployment.ps1
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread infra/scripts/post_deployment.sh
Comment thread infra/scripts/post_deployment.sh Outdated
Comment thread infra/scripts/post_deployment.sh
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants