fix(agents): fail fast on unready Foundry dependencies - #9326
Conversation
- Validate enabled direct and transitive dependencies before agent deploy. - Scope readiness markers to the active Foundry project.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
|
Azure Pipelines: Successfully started running 4 pipeline(s). 18 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
github-actions
Bot
added
ext-agents
There was a problem hiding this comment.
Pull request overview
Adds fail-fast validation for unavailable Foundry dependencies before agent deployment.
Changes:
- Recursively validates enabled
uses:dependencies. - Publishes project-scoped readiness markers.
- Rejects unavailable legacy bundled toolboxes with migration guidance.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
azure.ai.toolboxes/.../envkey.go |
Adds toolbox project marker keys. |
azure.ai.toolboxes/.../envkey_test.go |
Tests marker key generation. |
azure.ai.toolboxes/.../toolbox_env.go |
Publishes scoped toolbox readiness state. |
azure.ai.toolboxes/.../toolbox_env_test.go |
Tests endpoint scope extraction. |
azure.ai.skills/.../envkey.go |
Defines skill readiness keys. |
azure.ai.skills/.../envkey_test.go |
Tests skill keys. |
azure.ai.skills/.../skill_delete.go |
Clears skill readiness markers. |
azure.ai.skills/.../skill_delete_test.go |
Adds cleanup coverage. |
azure.ai.skills/.../service_target.go |
Publishes skill deployment markers. |
azure.ai.skills/.../service_target_test.go |
Tests marker publication. |
azure.ai.projects/.../foundry_provisioning_provider.go |
Scopes connection outputs to projects. |
azure.ai.agents/.../service_target_agent.go |
Runs validation and publishes agent markers. |
azure.ai.agents/.../service_target_agent_test.go |
Tests condition lookup and agent markers. |
azure.ai.agents/.../foundry_dependencies.go |
Implements dependency readiness validation. |
azure.ai.agents/.../foundry_dependencies_test.go |
Covers dependency and remediation scenarios. |
azure.ai.agents/.../envkey.go |
Defines shared readiness keys. |
azure.ai.agents/.../envkey_test.go |
Tests readiness keys. |
azure.ai.agents/.../codes.go |
Adds the dependency-not-ready error code. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:420
- The condition lookup precedence is opposite to azd core.
environment.Environment.Getenvchecks the azd.envfirst (cli/azd/pkg/environment/environment.go:188-198), but this checks the process environment first. If both define a condition differently, core can skip a dependency while this validator requires it, or this validator can skip readiness for a dependency core deployed. ReaddependencyEnvfirst and only then fall back to the process environment.
if value, ok := os.LookupEnv(name); ok {
return value
}
return p.dependencyEnv[name]
cli/azd/extensions/azure.ai.skills/internal/cmd/skill_delete_test.go:29
- This test calls the stub directly, so it passes even if
deleteAction.Runnever invokes marker cleanup, invokes it before a failed delete, or passes the wrong skill name. Exercise the delete action through an injectable resolver/client (or extract a testable delete core) and assert cleanup occurs only after API success.
// The API path is covered by client tests; this seam asserts deletion owns
// marker cleanup without requiring a live Foundry endpoint.
require.NoError(t, clearSkillMarkersFunc(t.Context(), "my-skill"))
require.True(t, called)
cli/azd/extensions/azure.ai.agents/internal/project/foundry_dependencies.go:178
- The remediation collapse produces dead-end suggestions for configuration failures: a disabled runtime toolbox and a toolbox service missing from
usesboth receive anazd deploysuggestion, which cannot change the condition or graph wiring and will repeat the same error. The migration branch also suppressesazd provisionwhen migration and provision failures are aggregated. Preserve per-failure remediation so these cases instruct users to enable/remove the dependency, add theusesedge, and run every required command.
suggestion := "run 'azd deploy --all', then retry the agent deployment"
if requiresMigration {
suggestion = "migrate bundled toolboxes to azure.ai.toolbox services, run 'azd deploy --all', " +
"then retry the agent deployment"
} else if requiresProvision && !requiresDeploy {
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go:2718
- This adds
AGENT_*_PROJECT_ENDPOINT, but whole-agent deletion still clears onlyNAME,VERSION, andENDPOINT(internal/cmd/delete.go:212-240). The stale scope marker can later override the legacy endpoint fallback and reject a valid redeployment from an older compatible publisher. Add this key to deletion cleanup and cover it in the delete tests.
azdext.SetEnvRequest{EnvName: p.env.Name, Key: envkey.AgentProjectEndpoint(serviceConfig.Name), Value: projectEndpoint},
cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_env.go:58
- The empty-value path is used after whole-toolbox deletion, but it clears only the MCP endpoint and leaves the new project-scope marker behind. Clear the commit marker first, then clear the project marker so deleted readiness state cannot leak into a later compatible deployment.
if value == "" {
return setValue(commitKey, "")
- Align condition evaluation with azd environment precedence. - Clear readiness markers after agent, skill, and toolbox deletion.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_env.go:65
- [azd-code-reviewer] This fallback scopes an endpoint to the active project even when the URL does not prove that relationship.
endpoint:accepts any non-empty expanded string, so a noncanonical or unrelated MCP URL without/toolboxes/gets a matching project marker and then passes the new readiness check. Reject endpoints whose project cannot be derived, or otherwise verify them againstFOUNDRY_PROJECT_ENDPOINTbefore publishing the commit marker.
if projectEndpoint == "" {
projectResp, err := c.Environment().GetValue(ctx, &azdext.GetEnvRequest{
EnvName: envName, Key: "FOUNDRY_PROJECT_ENDPOINT",
cli/azd/extensions/azure.ai.skills/internal/cmd/skill_delete.go:68
- [azd-code-reviewer] The deleted skill may come from
--project-endpoint, but cleanup always blanks markers in the active azd environment. Deletingmy-skillfrom project B while the active environment tracks the same-named skill in project A therefore removes A's valid readiness state. PassskillCtx.endpointinto cleanup and only clear markers whose stored project endpoint matches the deletion target.
if err := deleteSkillAndClearMarkers(ctx, skillCtx.client, a.flags.name); err != nil {
cli/azd/extensions/azure.ai.agents/internal/project/foundry_dependencies_test.go:214
- [azd-code-reviewer] This added line exceeds the repository's 125-character Go limit (
cli/azd/AGENTS.md:97-108), so thelllpreflight check will fail. Split the URL across concatenated strings.
envkey.ToolboxMCPEndpoint("legacy-tools"): "https://account.services.ai.azure.com/api/projects/old/toolboxes/legacy-tools/mcp",
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
A few things worth a look before this merges.
foundry_dependencies.go:243- the skill readiness check reads a marker that's written from a different resolution chain thanFOUNDRY_PROJECT_ENDPOINT, so a successful skill deploy can produce a marker the agent deploy then rejects.toolbox_env_test.go- five existing tests and a helper were removed, including coverage for code this PR doesn't touch.skill_delete_test.go:59- exercises the stub rather than any production code.service_target_agent.go:409- the condition truthiness list is duplicated from azd core with nothing keeping the two in sync.delete.go:172- the version marker gets cleared, butAGENT_<KEY>_ENDPOINTkeeps pointing at the deleted version.
Details inline.
- Normalize project identity and scope marker cleanup to its deployment target. - Preserve toolbox lifecycle coverage and clear stale agent endpoints.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/delete.go:225
- [azd-code-reviewer] Whole-agent deletion still leaves the per-protocol
AGENT_*_*_ENDPOINTvalues populated. Deployment writes these keys andShowActionreads them before the legacy base endpoint, so users and consumers can continue seeing endpoints for an agent that was successfully deleted. Append theDisplayableProtocolEnvSuffixeskeys here, asclearDeletedVersionMarkeralready does.
a.clearEnvVars(ctx, azdClient, serviceName, []string{
fmt.Sprintf("AGENT_%s_NAME", serviceKey),
fmt.Sprintf("AGENT_%s_VERSION", serviceKey),
fmt.Sprintf("AGENT_%s_ENDPOINT", serviceKey),
envkey.AgentProjectEndpoint(serviceName),
cli/azd/extensions/azure.ai.skills/internal/cmd/skill_delete.go:107
- [azd-code-reviewer] This comparison is stricter than the readiness comparison:
/api/projects/pand/projects/paliases identify the same Foundry project, but this helper rejects them. A delete through the alternate endpoint can therefore succeed while leaving both skill markers intact; a later agent deploy against the original endpoint can then treat the deleted skill as ready. Compare parsed host/project identity here assameProjectEndpointdoes, and cover the alias deletion case.
marker, err := client.Environment().GetValue(ctx, &azdext.GetEnvRequest{
EnvName: env.Environment.GetName(), Key: projectKey,
})
if err != nil || !sameSkillProjectEndpoint(marker.Value, projectEndpoint) {
return nil
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Incremental pass on d2a7ffb. All five items from my last review check out: the endpoint alias normalization, the restored toolbox tests, the skill marker key assertions, the isConditionTrue pointer comment, and the agent endpoint cleanup on version delete.
Three things in the new commit:
skill_delete.go:126- skill delete compares project endpoints strictly while the agents readiness check now compares them loosely, so a delete can leave markers behind that the agent validator then accepts.toolbox_env.go:64- blanking the project marker for a reused custom endpoint blocks agent deployment with a suggestion that can't clear it.skill_delete.go:106- the new marker guard swallows both the RPC error and the project mismatch without logging either.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/delete.go:224
- [azd-code-reviewer] A whole-agent delete still leaves every
AGENT_{KEY}_{PROTOCOL}_ENDPOINTvalue behind. Those values are published during deployment and are consumed by commands such asshow, so clearing only the base endpoint leaves stale URLs after the resource is gone. Add the displayable protocol keys here, as the version-delete path already does.
a.clearEnvVars(ctx, azdClient, serviceName, []string{
fmt.Sprintf("AGENT_%s_NAME", serviceKey),
fmt.Sprintf("AGENT_%s_VERSION", serviceKey),
fmt.Sprintf("AGENT_%s_ENDPOINT", serviceKey),
envkey.AgentProjectEndpoint(serviceName),
cli/azd/extensions/azure.ai.skills/internal/cmd/skill_delete.go:110
- [azd-code-reviewer] Swallowing a
GetValuefailure here can leave a non-empty skill version marker after the skill was deleted. Because agent readiness trusts that marker rather than querying Foundry, a later agent deployment can incorrectly accept the deleted skill as ready. Missing keys already return an empty successful response, so actual RPC/read failures should be propagated.
if err != nil {
log.Printf("skill marker cleanup skipped: cannot read %s: %v", projectKey, err)
return nil
}
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Two findings in the latest commit, both cases where a readiness marker claims more than it proves.
toolbox_env.go:62 - the reuse path stamps the active project onto a toolbox endpoint it never verified, so a toolbox living in a different Foundry project passes validation.
delete.go:223 - whole-agent delete leaves the protocol endpoint keys behind, while the version delete right below clears them.
| projectEndpoint := strings.TrimRight(strings.TrimSpace(projectScope), "/") | ||
| if projectEndpoint == "" { | ||
| projectEndpoint = toolboxProjectEndpoint(value) | ||
| } |
There was a problem hiding this comment.
The reuse path now stamps the active project onto a toolbox it never verified belongs there, so this marker stops meaning what the description says it means.
publishReuseEndpoint passes env["FOUNDRY_PROJECT_ENDPOINT"] as projectScope (service_target.go:222), and this takes it verbatim whenever it's non-empty. toolboxProjectEndpoint(value) only runs as a fallback, so a reuse endpoint that is project-scoped never gets attributed to the project it actually lives in.
TestPublishReuseEndpoint_WritesExpandedEndpoint shows it. The reused URL is https://acct.services.ai.azure.com/api/projects/p/toolboxes/research/versions/3/mcp and FOUNDRY_PROJECT_ENDPOINT is https://active.services.ai.azure.com/api/projects/current. Different account, different project. TOOLBOX_RESEARCH_PROJECT_ENDPOINT gets written as the active one, validateFoundryToolboxDependency (foundry_dependencies.go:307) then compares that marker against FOUNDRY_PROJECT_ENDPOINT, they're identical by construction, and the agent deploys wired to a toolbox in someone else's project.
Swapping the precedence keeps the fix for the custom-endpoint case and restores attribution when the URL can prove it:
| projectEndpoint := strings.TrimRight(strings.TrimSpace(projectScope), "/") | |
| if projectEndpoint == "" { | |
| projectEndpoint = toolboxProjectEndpoint(value) | |
| } | |
| projectEndpoint := toolboxProjectEndpoint(value) | |
| if projectEndpoint == "" { | |
| projectEndpoint = strings.TrimRight(strings.TrimSpace(projectScope), "/") | |
| } |
A bring-your-own endpoint with no /toolboxes/ segment still derives nothing, still falls back to the active project scope, and still deploys. A reuse URL under /projects/<other> now fails readiness, which is what "state from another Foundry project cannot satisfy validation" should mean.
If letting reuse always pass is the deliberate call, that's worth a line in the description, since it's the one hole left in the scoping model.
Either way this branch has no direct coverage. Every caller stubs setToolboxEndpointEnvFunc, so nothing drives the precedence through setToolboxEndpointEnv itself.
| serviceKey := toServiceKey(serviceName) | ||
| a.clearEnvVars(ctx, azdClient, serviceName, []string{ | ||
| fmt.Sprintf("AGENT_%s_NAME", serviceKey), | ||
| fmt.Sprintf("AGENT_%s_VERSION", serviceKey), | ||
| fmt.Sprintf("AGENT_%s_ENDPOINT", serviceKey), | ||
| envkey.AgentProjectEndpoint(serviceName), | ||
| }) |
There was a problem hiding this comment.
clearDeletedVersionMarker right below clears AGENT_<KEY>_ENDPOINT plus every DisplayableProtocolEnvSuffixes() variant, but this list stops at the base endpoint. Deleting one version cleans up more state than deleting the whole agent.
After azd ai agent delete <name>, AGENT_<KEY>_RESPONSES_ENDPOINT, AGENT_<KEY>_INVOCATIONS_ENDPOINT and AGENT_<KEY>_INVOCATIONS_WS_ENDPOINT still point at an agent that's gone, and show.go:289, helpers.go:931, invoke.go and session_carryover.go:151 all read those keys.
TestDeleteMarkerCleanup makes the gap visible: the version subtest asserts all three protocol keys land empty, the whole-agent subtest only asserts the project marker.
The loop is already written 25 lines down and project is already imported:
| serviceKey := toServiceKey(serviceName) | |
| a.clearEnvVars(ctx, azdClient, serviceName, []string{ | |
| fmt.Sprintf("AGENT_%s_NAME", serviceKey), | |
| fmt.Sprintf("AGENT_%s_VERSION", serviceKey), | |
| fmt.Sprintf("AGENT_%s_ENDPOINT", serviceKey), | |
| envkey.AgentProjectEndpoint(serviceName), | |
| }) | |
| serviceKey := toServiceKey(serviceName) | |
| keys := []string{ | |
| fmt.Sprintf("AGENT_%s_NAME", serviceKey), | |
| fmt.Sprintf("AGENT_%s_VERSION", serviceKey), | |
| fmt.Sprintf("AGENT_%s_ENDPOINT", serviceKey), | |
| envkey.AgentProjectEndpoint(serviceName), | |
| } | |
| for _, protocol := range project.DisplayableProtocolEnvSuffixes() { | |
| keys = append(keys, fmt.Sprintf("AGENT_%s_%s_ENDPOINT", serviceKey, protocol.Suffix)) | |
| } | |
| a.clearEnvVars(ctx, azdClient, serviceName, keys) |
cleanupAgentSessionState runs before this and only reads the base AGENT_<KEY>_ENDPOINT, so ordering is unaffected.
Summary
Fixes #8587.
Previously, agent deployment succeeded even when required Foundry resources were unavailable, producing an agent that failed at invocation time. This change validates declared dependencies before creating the agent version and returns actionable provisioning, deployment, configuration, or migration guidance when they are not ready.
Changes
uses:dependencies for projects, connections, toolboxes, skills, and agents.azure.ai.toolboxservices instead of silently dropping them.Design
Core
uses:edges continue to control deployment ordering; the agent target adds semantic readiness validation immediately before mutation. Provision-time dependencies (azure.ai.projectandazure.ai.connection) direct users toazd provision, while deploy-time dependencies direct users toazd deploy <service>orazd deploy --all.Readiness markers are scoped to
FOUNDRY_PROJECT_ENDPOINTso state from another Foundry project cannot satisfy validation. Existing endpoint markers remain compatible when their URL proves they belong to the active project.Manual Validation
azure.ai.toolboxservice against an existing Foundry project.foundry_dependency_not_ready, named the missingTOOLBOX_REVIEW_TOOLS_MCP_ENDPOINT, and suggested deployingreview-toolsbefore retrying the agent.