fix: add unified toolbox diagnostics - #9809
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 24 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 3 pipeline(s). 24 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go:176
- This invalid-condition issue is already recorded in
ToolboxDependencyErrors, so also adding it toToolboxLoadErrorsmakes Next-step emit two different repair actions and makes Doctor take the generic load-error branch before its dependency-specific branch. Keep condition failures in the dependency list and block endpoint probing when either dependency or load errors exist.
This issue also appears on line 349 of the same file.
recordToolboxLoadIssue(state, issue)
cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go:349
- This duplicates the agent-condition failure across
ToolboxDependencyErrorsandToolboxLoadErrors. An assembled state therefore produces both generic load guidance and dependency guidance, while Doctor hides the more specific dependency result behind its load-error check. Record it only as a dependency error and gate probing on dependency errors separately.
recordToolboxLoadIssue(state, issue)
cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/toolboxes.go:400
- Resolving a root
$refthrough the full agent map recursively resolves unrelated sections too. A missing nested connection$ref, for example, becomes aToolboxLoadErrorand suppresses every toolbox probe, whereas the non-root path below intentionally resolves onlytoolboxesand the new unrelated-ref test expects that isolation. Load the root agent definition, then resolve only its toolbox subtree so connection failures remain connection diagnostics.
raw := props.AsMap()
if _, hasRootRef := raw["$ref"]; hasRootRef {
return resolveAgentConnectionProperties(props, projectRoot, source)
}
There was a problem hiding this comment.
🟡 Changes recommended
Duplicate error classification and lost multi-agent ownership can produce misleading or incomplete remediation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go:176
- This condition failure is already added to
ToolboxDependencyErrorsabove. Also adding it toToolboxLoadErrorsmakes Next-step emit two different repair entries for the same issue, and Doctor takes the load-error branch first instead of its dependency-specific branch. Keep malformed dependency conditions in one category.
This issue also appears on line 349 of the same file.
cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go:349
- This same issue is already stored in
ToolboxDependencyErrorson lines 345–348. Duplicating it inToolboxLoadErrorsproduces duplicate Next-step actions and causes Doctor to misclassify the malformed dependency condition as a toolbox-file load failure. Keep the issue in the dependency category only.
recordToolboxLoadIssue(state, issue)
- Files reviewed: 15/15 changed files
- Comments generated: 1
- Review effort level: Balanced
| key := envkey.ToolboxMCPEndpoint(ref.Name) | ||
| existing, found := collected[key] | ||
| if !found || toolboxSourcePriority(ref.ToolboxSource) > | ||
| toolboxSourcePriority(existing.ToolboxSource) { | ||
| collected[key] = ref |
azure.ai.agents PR buildNote This is an unsigned development build. Install it only if you trust this PR. Install the extension: azd ext install "https://azuresdkartifacts.z5.web.core.windows.net/azd/extensions/pr/9809/azure-ai-agents.zip"
|
Why this change is needed
Agents can declare bundled toolboxes in unified
azure.yaml, while older projects still useagent.manifest.yaml. Next-step and Doctor previously did not represent those sources consistently, so missing endpoints and broken references could produce incomplete guidance or an incorrect legacy fallback.Approach
split > bundled > legacy.$referrors.Validation
go test ./...go build ./...golangci-lintchecksEnd-to-end validation
azd ai agent doctor --local-only --no-prompt(bundled toolbox, missing endpoint)TOOLBOX_RESEARCH_TOOLS_MCP_ENDPOINTand provided bundled-toolbox migration guidance.azd ai agent doctor --local-only --no-prompt(endpoint configured)local.toolboxesas passing.azd ai agent doctor --local-only --no-prompt(invalid$ref)Fixes #9565