Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions cli/azd/extensions/azure.ai.agents/internal/cmd/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Exit codes:
Unredacted: flags.unredacted,
}

report, err := runAndRenderDoctorText(ctx, deps, opts, azdClient, os.Stdout, debug)
report, err := runAndRenderDoctorText(ctx, deps, opts, os.Stdout, debug)
if err != nil {
return err
}
Expand Down Expand Up @@ -109,7 +109,6 @@ func runAndRenderDoctorText(
ctx context.Context,
deps doctor.Dependencies,
opts doctor.Options,
azdClient *azdext.AzdClient,
w io.Writer,
debug bool,
) (doctor.Report, error) {
Expand All @@ -122,7 +121,6 @@ func runAndRenderDoctorText(
ctx,
deps,
opts,
azdClient,
func(result doctor.Result) error {
return renderer.writeCheck(result)
},
Expand All @@ -142,9 +140,11 @@ func runDoctorWithObserver(
ctx context.Context,
deps doctor.Dependencies,
opts doctor.Options,
azdClient *azdext.AzdClient,
observer doctor.ResultObserver,
) (doctor.Report, []nextstep.Suggestion, error) {
if deps.StateCache == nil {
deps.StateCache = doctor.NewStateCache()
}
// Keep local checks first so remote checks can inspect their prior
// results for skip-cascade decisions.
checks := append(doctor.NewLocalChecks(deps), doctor.NewRemoteChecks(deps)...)
Expand All @@ -160,20 +160,20 @@ func runDoctorWithObserver(
return report, nil, nil
}

trailing := resolveDoctorTrailing(ctx, azdClient)
trailing := resolveDoctorTrailing(ctx, deps)
return report, trailing, nil
}

// resolveDoctorTrailing returns the doctor's trailing Next block, or nil on
// error. It chooses deployed-agent suggestions when any service is deployed;
// otherwise it reuses the post-init guidance.
func resolveDoctorTrailing(ctx context.Context, azdClient *azdext.AzdClient) []nextstep.Suggestion {
if azdClient == nil {
func resolveDoctorTrailing(ctx context.Context, deps doctor.Dependencies) []nextstep.Suggestion {
if deps.AzdClient == nil {
return nil
}

state, _ := nextstep.AssembleStateFromSource(ctx, nextstep.NewSource(azdClient))
if len(state.Services) == 0 {
state, _ := deps.AssembleAgentState(ctx)
if state == nil || len(state.Services) == 0 {
// Avoid repeating the missing-service guidance already reported by
// `local.agent-service-detected`.
return nil
Expand All @@ -184,12 +184,12 @@ func resolveDoctorTrailing(ctx context.Context, azdClient *azdext.AzdClient) []n
// stay copy-paste correct.
return nextstep.ResolveAfterDeploy(
filterDeployedServices(state),
doctorCachedPayload(ctx, azdClient),
readmeExistsForProject(ctx, azdClient),
doctorCachedPayload(ctx, deps.AzdClient),
readmeExistsForProject(ctx, deps.AzdClient),
)
}

return nextstep.ResolveAfterInit(state, readmeExistsForProject(ctx, azdClient))
return nextstep.ResolveAfterInit(state, readmeExistsForProject(ctx, deps.AzdClient))
}

func anyServiceDeployed(services []nextstep.ServiceState) bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

"github.com/Azure/azure-sdk-for-go/sdk/azcore"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/azure/azure-dev/cli/azd/pkg/azdext"
)

// foundryConnectionsProbeTimeout caps the per-project connections
Expand Down Expand Up @@ -114,13 +113,7 @@ func newCheckConnections(deps Dependencies) Check {
}
}

assembler := deps.assembleState
if assembler == nil {
assembler = func(c context.Context, client *azdext.AzdClient) (*nextstep.State, []error) {
return nextstep.AssembleState(c, client)
}
}
state, errs := assembler(ctx, deps.AzdClient)
state, errs := deps.AssembleAgentState(ctx)
if state == nil {
cause := "unknown error"
if len(errs) > 0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,16 @@ type Dependencies struct {
AgentAPIVersion string

// assembleState is a test seam: when non-nil it replaces the
// production `nextstep.AssembleState` call inside the
// `local.manual-env-vars` check, letting unit tests inject a
// pre-computed State without standing up a temp project on disk.
// production `nextstep.AssembleState` call, letting unit tests inject
// a pre-computed State without standing up a temp project on disk.
// Lowercase so external packages cannot reach it. Production code
// (NewLocalChecks via the Cobra wiring) leaves it nil.
assembleState func(ctx context.Context, client *azdext.AzdClient) (*nextstep.State, []error)

// StateCache shares one assembled state across Doctor checks and
// trailing guidance during a single invocation.
StateCache *StateCache

// probeAuth is a test seam: when non-nil it replaces the
// production `realProbeAuth` call inside the `remote.auth` check,
// letting unit tests inject controlled token-acquisition outcomes
Expand Down Expand Up @@ -174,6 +177,9 @@ type Dependencies struct {
// endpoint env vars; it is local because it does not call ARM /
// Foundry (only the active azd environment).
func NewLocalChecks(deps Dependencies) []Check {
if deps.StateCache == nil {
deps.StateCache = NewStateCache()
}
return []Check{
newCheckGRPCAndVersion(deps),
newCheckProjectConfig(deps),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ func TestNewLocalChecks_OrderAndIDs(t *testing.T) {
{"local.project-endpoint-set", "FOUNDRY_PROJECT_ENDPOINT set", false},
{"local.agent-yaml-valid", "agent definition valid (per service)", false},
{"local.manual-env-vars", "manual env vars set", false},
{"local.toolboxes", "Manifest toolboxes have endpoint env vars set", false},
{"local.toolboxes", "Configured toolboxes have endpoint env vars set", false},
}
for i, w := range want {
require.Equal(t, w.id, checks[i].ID, "index %d", i)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ import (
"fmt"
"slices"
"strings"

"azureaiagent/internal/cmd/nextstep"

"github.com/azure/azure-dev/cli/azd/pkg/azdext"
)

// newCheckManualEnvVars produces Check `local.manual-env-vars` — the
Expand Down Expand Up @@ -90,13 +86,7 @@ func newCheckManualEnvVars(deps Dependencies) Check {
}
}

assembler := deps.assembleState
if assembler == nil {
assembler = func(c context.Context, client *azdext.AzdClient) (*nextstep.State, []error) {
return nextstep.AssembleState(c, client)
}
}
state, errs := assembler(ctx, deps.AzdClient)
state, errs := deps.AssembleAgentState(ctx)
if state == nil {
// AssembleState always returns a non-nil State even when errs
// is non-empty — but defend against a future contract change
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ import (
type toolboxEnvLookupFn func(ctx context.Context, key string) (value string, err error)

// newCheckToolboxes produces Check `local.toolboxes` (P5.1 C14).
// For each `ToolboxResource` declared in any service's
// `agent.manifest.yaml` (collected by the C2 manifest walker), the
// For each toolbox collected during next-step state assembly, the
// check verifies that the canonical
// `TOOLBOX_<NORMALIZED_NAME>_MCP_ENDPOINT` env var is set to a
// non-empty value in the active azd environment.
Expand All @@ -44,7 +43,7 @@ type toolboxEnvLookupFn func(ctx context.Context, key string) (value string, err
// skips in this state, so the toolbox check would falsely Pass.
// - `local.azure-yaml` / `local.agent-service-detected` failed →
// no services to walk; walker output is unreliable.
// - state.HasToolboxes == false → no manifest toolbox declarations;
// - state.HasToolboxes == false → no toolbox declarations;
// the check has nothing to verify.
//
// # Why this check is not gated on `remote.auth` /
Expand All @@ -69,7 +68,7 @@ type toolboxEnvLookupFn func(ctx context.Context, key string) (value string, err
func newCheckToolboxes(deps Dependencies) Check {
return Check{
ID: "local.toolboxes",
Name: "Manifest toolboxes have endpoint env vars set",
Name: "Configured toolboxes have endpoint env vars set",
Remote: false,
Fn: func(ctx context.Context, _ Options, prior []Result) Result {
if deps.AzdClient == nil {
Expand All @@ -94,13 +93,7 @@ func newCheckToolboxes(deps Dependencies) Check {
}
}

assembler := deps.assembleState
if assembler == nil {
assembler = func(c context.Context, client *azdext.AzdClient) (*nextstep.State, []error) {
return nextstep.AssembleState(c, client)
}
}
state, errs := assembler(ctx, deps.AzdClient)
state, errs := deps.AssembleAgentState(ctx)
if state == nil {
// AssembleState always returns a non-nil State even when errs
// is non-empty (state.go), but defend against a future contract
Expand All @@ -119,20 +112,114 @@ func newCheckToolboxes(deps Dependencies) Check {
if !state.HasToolboxes {
return Result{
Status: StatusSkip,
Message: "skipped: no toolbox resources declared in any service's agent.manifest.yaml.",
Message: "skipped: no configured toolbox resources were found.",
}
}

if state.ToolboxEndpointsChecked {
if len(state.ToolboxEndpointErrors) > 0 {
return Result{
Status: StatusFail,
Message: fmt.Sprintf(
"could not read toolbox endpoint values: %s",
strings.Join(state.ToolboxEndpointErrors, "; ")),
Suggestion: "Verify the active azd environment is accessible, then re-run " +
"`azd ai agent doctor`.",
Details: map[string]any{
"toolboxEndpointErrors": state.ToolboxEndpointErrors,
},
}
}
return classifyToolboxState(state.Toolboxes, state.MissingToolboxEndpoints)
}

// Keep the old seam for callers constructing partial states. Production
// assembly always sets ToolboxEndpointsChecked and never reads again.
lookup := deps.lookupToolboxEnv
if lookup == nil {
lookup = makeRealToolboxEnvLookup(deps.AzdClient)
}

return classifyToolboxEndpoints(ctx, state.Toolboxes, lookup)
},
}
}

func classifyToolboxState(
toolboxes, missing []nextstep.ResourceRef,
) Result {
missingKeys := make(map[string]struct{}, len(missing))
missingUnique := make([]nextstep.ResourceRef, 0, len(missing))
for _, toolbox := range missing {
key := envkey.ToolboxMCPEndpoint(toolbox.Name)
if _, duplicate := missingKeys[key]; duplicate {
continue
}
missingKeys[key] = struct{}{}
missingUnique = append(missingUnique, toolbox)
}
seen := make(map[string]struct{}, len(toolboxes))
matched := 0
for _, toolbox := range toolboxes {
key := envkey.ToolboxMCPEndpoint(toolbox.Name)
if _, duplicate := seen[key]; duplicate {
continue
}
seen[key] = struct{}{}
if _, ok := missingKeys[key]; !ok {
matched++
}
}
return classifyToolboxResults(missingUnique, matched)
}

func classifyToolboxResults(
missing []nextstep.ResourceRef,
matched int,
) Result {
if len(missing) == 0 {
return Result{
Status: StatusPass,
Message: fmt.Sprintf("all %d declared toolbox(es) have an MCP endpoint set.", matched),
Details: map[string]any{"matchedCount": matched},
}
}

slices.SortFunc(missing, func(a, b nextstep.ResourceRef) int {
if a.Name != b.Name {
return strings.Compare(a.Name, b.Name)
}
return strings.Compare(a.ServiceName, b.ServiceName)
})
var names []string
hasSplit := false
hasLegacy := false
for _, toolbox := range missing {
names = append(names, fmt.Sprintf("%s (env %s, service %s)",
toolbox.Name, envkey.ToolboxMCPEndpoint(toolbox.Name), toolbox.ServiceName))
hasSplit = hasSplit || toolbox.ToolboxSource == nextstep.ToolboxSourceSplit
hasLegacy = hasLegacy || toolbox.ToolboxSource != nextstep.ToolboxSourceSplit
}
suggestion := "Run `azd provision` to materialize toolbox infrastructure, or " +
"`azd env set <ENV_VAR> <endpoint>` to point at an existing toolbox."
switch {
case hasSplit && hasLegacy:
suggestion = "Run `azd deploy` for split toolbox services and `azd provision` " +
"for legacy toolbox resources, or set an existing endpoint."
case hasSplit:
suggestion = "Run `azd deploy` to materialize split toolbox services."
}
return Result{
Status: StatusFail,
Message: fmt.Sprintf("%d declared toolbox(es) have no MCP endpoint set in the azd environment: %s",
len(missing), strings.Join(names, ", ")),
Suggestion: suggestion,
Details: map[string]any{
"missingToolboxes": missing,
"matchedCount": matched,
},
}
}

// normalizeToolboxName / toolboxEndpointKey have been replaced by the
// shared `internal/pkg/envkey` package. See envkey.ToolboxMCPEndpoint.

Expand Down
Loading
Loading