fix(ai-agent): resolve bot name in activity agent deployment - #9497
fix(ai-agent): resolve bot name in activity agent deployment#9497Huajie Zhang (jayzhang) wants to merge 10 commits into
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 19 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Resolves Azure Bot names during Activity agent deployment and avoids progress UI interference with extension handlers.
Changes:
- Discovers, creates, and persists Activity agent bot resources.
- Moves bot setup from postdeploy into the service target.
- Delays deploy progress rendering until predeploy handlers finish.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
cli/azd/internal/cmd/deploy.go |
Delays deploy title and progress ticker. |
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go |
Resolves and provisions Activity bots. |
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go |
Tests bot-name resolution and persistence. |
cli/azd/extensions/azure.ai.agents/internal/pkg/envkey/envkey.go |
Adds the bot-name environment key. |
cli/azd/extensions/azure.ai.agents/internal/pkg/botservice/botservice.go |
Adds bot discovery and default naming. |
cli/azd/extensions/azure.ai.agents/internal/pkg/botservice/botservice_test.go |
Tests bot discovery. |
cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go |
Moves bot setup out of postdeploy. |
cli/azd/extensions/azure.ai.agents/internal/cmd/activity_bot_provision.go |
Persists default bot names during provisioning. |
cli/azd/extensions/azure.ai.agents/extension.yaml |
Updates extension metadata version. |
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:2956
- [azd-code-reviewer] The selected bot can now have a custom name or live in another resource group, but
teardownActivityBotsstill recomputes the old scopedBotNameinAZURE_RESOURCE_GROUP. Consequentlyazd downleaves newly created bots orphaned. Persist enough lifecycle metadata (name, resource group, and whether azd created it) and make teardown consume it without deleting an adopted bot.
suffix := strings.ToUpper(ep.Protocol)
key := fmt.Sprintf("AGENT_%s_%s_ENDPOINT", serviceKey, suffix)
envVars = append(envVars, azdext.SetEnvRequest{EnvName: p.env.Name, Key: key, Value: ep.URL})
}
envVars = append(envVars,
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (8)
cli/azd/extensions/azure.ai.agents/internal/cmd/activity_bot_provision.go:59
DefaultBotNamedrops the deployment-scope hash even though Bot Service names are globally unique (botservice.go:113-118). Common names such asagent-awill therefore all try to createagent-a-bot, and deployments in other subscriptions can fail on a name collision. Keep the scopedBotNamedefault and calculate it once the subscription and resource group are known rather than persisting an unscoped default.
botName := strings.TrimSpace(botservice.DefaultBotName(agent.Name))
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1139
- This makes every Activity deployment depend on the subscription-wide BotService list operation before trying the persisted bot name. A principal with Bot Service permissions scoped only to the target resource group can create/update that bot but cannot list all bots in the subscription, so deployment now fails here with 403. Try the persisted name first and reserve the subscription lookup for conflict recovery, or treat insufficient list permission as a fallback condition.
boundBot, err := botFinder.FindByMsaAppID(ctx, agentIdentityClientID)
if err != nil {
return "", "", err
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1160
- The suggestion is misleading:
azd provisioncallsprovisionActivityBotNames, which saves this same default and never asks for a custom value. Tell users to set the environment key explicitly so the documented action actually changes the bot name.
"Azure Bot name was not set in %s; using default %q. Run `azd provision` to save a custom value.\n",
key,
name,
cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go:403
- Removing the
ensureActivityBotcall also removes the only calls towriteTeamsSetupGuideandprintTeamsNextSteps(listen_activity.go:135-136). Bot provisioning still leaves Teams app packaging and sideloading to the user, but successful deploys now provide neither the generatedTEAMS_APP_SETUP.mdnor the next-step pointer. Keep a non-provisioning postdeploy step that reads the persisted bot name/identity and generates these instructions.
// Read the inputs used by best-effort optimization reporting.
// Activity bot provisioning is performed in the service target deploy path
// (single source of truth), so postdeploy no longer performs a second bot
// configuration pass that can conflict with the deploy-time bot name.
cli/azd/extensions/azure.ai.agents/extension.yaml:8
- The manifest now advertises
1.0.0-beta.17, whileversion.txtremains1.0.0-beta.9;ci-build.ps1readsversion.txt, so package and manifest versions diverge. Perazure.ai.agents/AGENTS.md:164-170, version bumps belong in a dedicated release PR that updatesversion.txt,extension.yaml, andCHANGELOG.mdtogether. Revert this unrelated bump here.
version: 1.0.0-beta.17
cli/azd/extensions/azure.ai.agents/internal/pkg/botservice/botservice_test.go:190
- Use
t.Context()so the test follows the repository's Go 1.26 test pattern and propagates test cancellation instead of creating an unrelated background context.
got, err := c.FindByMsaAppID(context.Background(), "CLIENT-ID-123")
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1360
- Creation can now use a custom/default bot name and can switch to a bot in another resource group, but
teardownActivityBotsstill deletes onlyBotName(serviceName, scopeSalt)fromAZURE_RESOURCE_GROUP(listen_activity.go:285-287). A newly created<agent>-botwill therefore be left behind byazd down. Persist the selected resource group and update teardown to use the persisted name/location; if an existing bot is reused, also track ownership so teardown does not delete a resource azd did not create.
ensureCfg := botservice.BotConfig{
ResourceGroup: botResourceGroup,
BotName: activityBotName,
cli/azd/extensions/azure.ai.agents/internal/cmd/activity_bot_provision.go:22
- This new lifecycle handler has no tests even though neighboring command handlers are covered and it persists deployment state. Add tests for Activity vs. non-Activity services, preserving an existing custom value, missing environment responses, and default-name generation; the cross-scope naming regression would otherwise remain easy to reintroduce.
This issue also appears on line 59 of the same file.
func provisionActivityBotNames(
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (4)
cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go:404
- azd-code-reviewer: Removing this call leaves
ensureActivityBotas an unreferenced second implementation that still claims to run during postdeploy, and leavesgatherPostdeployInputsdocumentation describing the removed bot step. Remove the obsolete helper and update the stale comments so the deploy path is actually the single source of truth.
// Read the inputs used by best-effort optimization reporting.
// Activity bot provisioning is performed in the service target deploy path
// (single source of truth), so postdeploy no longer performs a second bot
// configuration pass that can conflict with the deploy-time bot name.
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1385
- azd-code-reviewer: Adopting a bound bot can select a different name and resource group, but
teardownActivityBotsstill recomputes the default from the service name andAZURE_RESOURCE_GROUP.azd downtherefore reports deleting a nonexistent default while silently leaving the selected bot behind. Track the selected bot's name/resource group and ownership, then either delete managed bots or explicitly preserve externally owned bots during teardown.
activityBotName = strings.TrimSpace(boundBot.Name)
if strings.TrimSpace(boundBot.ResourceGroup) != "" {
botResourceGroup = strings.TrimSpace(boundBot.ResourceGroup)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1347
- azd-code-reviewer: This client is scoped to
AZURE_SUBSCRIPTION_ID, but the bot resource group comes fromp.foundryProject, whose authoritative subscription is parsed fromAZURE_AI_PROJECT_ID. For an adopted project where those subscriptions differ, this searches and creates the bot in the wrong subscription (and may target a nonexistent same-named resource group). Scope the Bot Service client top.foundryProject.SubscriptionID.
This issue also appears on line 1383 of the same file.
client, err := botservice.NewClient(azdEnv["AZURE_SUBSCRIPTION_ID"], p.credential, nil)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1382
- azd-code-reviewer: The conflict-recovery branch is not exercised by the added tests; they cover name resolution and bot lookup independently. Add a test that makes the first
EnsureBotreturn the MsaAppId conflict, returns a bound bot from the follow-up lookup, verifies the retry uses that bot's name/resource group, and verifies the final name is persisted.
if isMsaAppIDAlreadyInUseError(err) {
if boundBot, findErr := client.FindByMsaAppID(ctx, identity.ClientID); findErr == nil &&
boundBot != nil && strings.TrimSpace(boundBot.Name) != "" {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1189
- [azd-code-reviewer] The environment fallback drops the persisted bot resource group. After an identity-bound bot in another resource group is persisted, a later list failure selects its stored name here but returns an empty group, so
Deploysubstitutes the Foundry resource group and targets the wrong resource. Return the matchingAgentBotResourceGroupvalue with the environment-selected name.
return name, "", nil
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1189
- [azd-code-reviewer] When identity lookup fails, this fallback returns only the persisted bot name and drops the persisted
AGENT_<SERVICE>_BOT_RESOURCE_GROUP. If the prior identity-bound bot lives outside the Foundry resource group, the next deploy probes and updates the wrong resource group, then can hit the sameMsaAppIdconflict it is meant to recover from. Return the persisted bot resource group together with the environment bot name.
return name, "", nil
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1342
- [azd-code-reviewer]
result.agentVersionis still the create/update response when its status is alreadyactive, because polling is skipped above. Those responses can omitinstance_identity(the existing ZIP response fixtures atoperations_test.go:744and:799do exactly that), so an otherwise successful Activity deployment now fails here instead of fetching the active version as the former postdeploy path did. Refresh the version when the identity is missing before rejecting the deployment.
identity := result.agentVersion.InstanceIdentity
if identity == nil || identity.ClientID == "" {
return nil, exterrors.Dependency(
cli/azd/extensions/azure.ai.agents/internal/cmd/listen_activity.go:314
- [azd-code-reviewer] This has no upgrade path for Bots created by the previous implementation: those Bots have neither the new environment tracking keys nor the new
azd-createdtag, so cleanup always skips them here. A redeploy does not migrate them either—activityBotOwnershipclassifies any existing untagged Bot as adopted and persistsowned=false. Add a one-time legacy migration for the former deterministic Bot name/resource group soazd downdoes not leave existing users' globally named Bot resources orphaned.
if !tracked {
continue
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (4)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1189
- [azd-code-reviewer] The environment fallback drops the persisted bot resource group. If the identity lookup is unavailable and the previously selected bot lives outside the Foundry resource group, this returns only its name;
Deploythen looks in the Foundry resource group and attempts to create the globally unique name there, so a later deploy fails instead of reusing the persisted bot. ReturnAgentBotResourceGroup(serviceName)together with the persisted name and update the fallback tests.
return name, "", nil
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1376
- [azd-code-reviewer] Only the existing bot's tags are preserved before the create-or-update PUT. This path now adopts identity-bound or explicitly named bots that may be externally managed, but
EnsureBotstill submitsSKUNameF0; an existing S1 bot is therefore downgraded or rejected during deploy. Preserve the existing SKU and other unaffected resource fields, defaulting to F0 only for a new bot.
if existingBot != nil {
existingTags = existingBot.Tags
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1393
- [azd-code-reviewer]
EnsureBotcan successfully create the tagged Bot and then fail while enabling the Teams channel. This returns beforefinalizeDeploypersistsBOT_RESOURCE_GROUPandBOT_OWNED, while teardown requires both values, so a first failed deploy leaves an Azure Bot thatazd downcannot discover or delete. Persist cleanup metadata immediately after the Bot PUT succeeds, or roll back newly created bots when the channel step fails.
if err := client.EnsureBot(ctx, ensureCfg); err != nil {
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1398
- [azd-code-reviewer] The new MsaAppID-conflict recovery branch is not exercised by the added tests; they test name resolution and lookup separately but never verify conflict → re-query → retry → persisted final bot. Add a bot-client seam and a deployment test covering both successful recovery and lookup/retry failure, since this is the central regression path.
if isMsaAppIDAlreadyInUseError(err) {
if boundBot, findErr := client.FindByMsaAppID(ctx, identity.ClientID); findErr == nil &&
boundBot != nil && strings.TrimSpace(boundBot.Name) != "" {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
cli/azd/extensions/azure.ai.agents/internal/cmd/listen_activity.go:198
- azd-code-reviewer: This drops cleanup compatibility for bots created by earlier extension versions. Those deployments have neither the new
BOT_*environment metadata nor theazd-createdtag, so after upgrading,azd downalways takes this branch and leaves the globally named Bot resource behind; the previous code derived and deleted that bot. Add a legacy fallback/migration for missing tracking metadata before requiring the ownership marker.
if !tracked {
continue
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1393
- azd-code-reviewer:
EnsureBotcan successfully create the tagged Bot and then fail while enabling the Teams channel. This returns beforefinalizeDeploypersists the new ownership metadata, while the new teardown path deletes only persisted owned bots, so an immediateazd downcannot clean up the partially created globally named resource. Roll back a newly created Bot on channel failure, or persist its target/ownership before the channel operation can fail.
}
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1398
- azd-code-reviewer: The conflict-recovery branch is the central behavior of this fix, but no test executes it; the added tests stop at name resolution and
FindByMsaAppID. A regression in detecting the conflict, switching resource groups, retryingEnsureBot, or persisting the recovered name would pass. Inject the Bot client/factory and add a deploy-level test that makes the first ensure return the duplicate-MsaAppID error and verifies the retry plus persisted values.
// bot name is already bound to this identity, switch to that bot and retry.
if isMsaAppIDAlreadyInUseError(err) {
if boundBot, findErr := client.FindByMsaAppID(ctx, identity.ClientID); findErr == nil &&
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1189
- azd-code-reviewer: This fallback discards the persisted bot resource group. If an identity lookup temporarily fails after a prior deploy reused a bot in another resource group,
Deployredirects the saved bot name to the Foundry resource group and can hit the same global-name/MsaAppId conflict again. ReturnAGENT_<SERVICE>_BOT_RESOURCE_GROUPwhenever the environment bot name is selected, and extend the fallback test to assert it.
return name, "", nil
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1144
- azd-code-reviewer: This diagnostic is emitted even though deployment continues with the environment/default fallback. Extension diagnostics must use
log.Print*so they stay hidden unless--debugis enabled (cli/azd/extensions/azure.ai.agents/AGENTS.md:178-184); directfmtoutput is reserved for user-facing stdout.
fmt.Fprintf(
os.Stderr,
"Unable to search for an Azure Bot already bound to the deployed agent identity: %v\n",
err,
)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:1399
- azd-code-reviewer: The MsaAppId-conflict recovery is the central failure mode addressed by this PR, but no test exercises this branch. Add coverage with an injectable/factored bot client that verifies the retry switches both bot name and resource group, recomputes ownership/tags, and succeeds or propagates the retry error.
if isMsaAppIDAlreadyInUseError(err) {
if boundBot, findErr := client.FindByMsaAppID(ctx, identity.ClientID); findErr == nil &&
boundBot != nil && strings.TrimSpace(boundBot.Name) != "" {
Summary
This PR stabilizes Azure Bot resolution for Activity agents during
azd deploy.Activity agents bind their Azure Bot
MsaAppIdto the deployed agent version's instance identity. Previously, the bot name could be resolved too early or configured again inpostdeploy, which could causeazd deployto try creating a different Azure Bot name for an identity that was already bound to an existing bot. Azure Bot Service rejects that with:This change makes deploy-time bot resolution use the deployed agent identity as the source of truth.
Changes
Resolve Activity agent bot names in this order during deploy:
AGENT_<SERVICE>_BOT_NAMEfrom the azd environmentPersist the final selected bot name back to
.envso subsequent deploys reuse the same value.Seed
AGENT_<SERVICE>_BOT_NAMEduring provision when it is missing, using a readable default such as<agent-name>-bot.Remove the Activity bot configuration pass from
postdeployso bot setup has a single source of truth in the service target deploy path.Add
botservice.FindByMsaAppIDto find an existing accessible Azure Bot already bound to the deployed agent identity.Add recovery for
MsaAppId is already in use: if creating/updating the selected bot name fails because the identity is already bound elsewhere, deploy re-checks the identity-bound bot and retries with that bot.Persist deployed agent instance identity outputs to
.envfor diagnostics:AGENT_<SERVICE>_INSTANCE_IDENTITY_CLIENT_IDAGENT_<SERVICE>_INSTANCE_IDENTITY_PRINCIPAL_IDWhy
The Azure Bot
MsaAppIdmust be unique. If an Activity agent identity is already bound to an existing bot, deploy must reuse that bot name rather than attempting to create a new Bot Service resource with the same identity.Persisting both the selected bot name and the deployed instance identity also makes future conflicts much easier to diagnose from
.env.Validation
go test ./internal/cmd/... ./internal/project/...MsaAppID