fix: unify split toolbox diagnostics - #9563
Conversation
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 20 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
Unifies toolbox diagnostics for standalone azure.ai.toolbox services across next-step and Doctor flows.
Changes:
- Discovers and probes split toolbox endpoints.
- Adds source-aware deploy/provision guidance.
- Shares one assembled state snapshot across Doctor checks.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
nextstep/types.go |
Adds toolbox source and probe state. |
nextstep/state.go |
Probes canonical toolbox endpoints. |
nextstep/state_test.go |
Tests split toolbox state assembly. |
nextstep/resolver.go |
Adds source-aware remediation. |
nextstep/resolver_test.go |
Tests split toolbox guidance. |
nextstep/manifest.go |
Discovers standalone toolbox services. |
doctor/state_cache.go |
Caches assembled diagnostic state. |
doctor/state_cache_test.go |
Verifies cache reuse. |
doctor/checks_toolboxes.go |
Uses assembled toolbox probe results. |
doctor/checks_toolboxes_test.go |
Tests toolbox classifications. |
doctor/checks_manual_env.go |
Reuses cached state. |
doctor/checks_local.go |
Shares the state cache. |
doctor/checks_local_test.go |
Updates toolbox check expectations. |
doctor/checks_connections.go |
Reuses cached state. |
doctor.go |
Shares state with trailing guidance. |
💡 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 15 out of 15 changed files in this pull request and generated no new comments.
Suppressed comments (2)
cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/types.go:162
- azd-code-reviewer: The following paragraph still says all three flags are false when a manifest is absent or malformed, which now contradicts split-toolbox discovery:
HasToolboxescan be true without any manifest. Update the field documentation so callers do not infer the old manifest-only contract.
// HasModels, HasToolboxes, and HasConnections are aggregate flags.
// They describe resources. Models and connections come from
// agent manifests. Toolboxes include split services and manifest
// resources. Doctor checks skip when no matching resource exists,
// while resolvers can tailor remediation.
cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/state_cache.go:32
- azd-code-reviewer: This fallback does not actually cache across calls because
Dependenciesis received by value. WhenStateCacheis nil, every call creates a new cache and reruns state assembly, contrary to this method's contract. Persist the cache by using a pointer receiver.
func (deps Dependencies) AssembleAgentState(ctx context.Context) (*nextstep.State, []error) {
cache := deps.StateCache
if cache == nil {
cache = NewStateCache()
}
| } | ||
|
|
||
| split := make(map[string]ResourceRef) | ||
| for serviceName, svc := range projectCfg.Services { |
There was a problem hiding this comment.
[azd-code-reviewer] This collects every azure.ai.toolbox service in the project, even when no agent references it through uses (or when its deployment condition disables it). An unrelated toolbox with no endpoint then populates MissingToolboxEndpoints, so ResolveAfterInit blocks local-run guidance and tells the user to deploy a resource the agent does not need. Filter split toolboxes to the dependencies of the collected agent services, and honor deployment conditions or avoid treating disabled services as missing.
Why this is needed
Projects with a unified
azure.yamlcan declare toolboxes as standalonehost: azure.ai.toolboxservices. Agent diagnostics previously read only legacy manifest toolboxes, so split toolboxes were left out of state and their managed endpoint variables could be treated as manual configuration. Next-step commands could then be wrong, and Doctor could skip the check or report a misleading result.What changed
azd deployguidance for split toolbox endpoints while preserving provisioning precedence and avoiding duplicate deploy suggestions.Scope
This PR covers only the toolbox part of #8710. Model and connection migration, runtime behavior, and provisioning implementation are unchanged. Legacy toolbox migration remains a separate follow-up.
Fixes #9566
Related to #8710