diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_identity_roles.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_identity_roles.go index a223d1b01bc..8cf57f42a5e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_identity_roles.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_identity_roles.go @@ -144,7 +144,7 @@ func newCheckAgentIdentityRoles(deps Dependencies) Check { if priorBlocked(prior, "local.agent-service-detected") { return Result{ Status: StatusSkip, - Message: "skipped: no `azure.ai.agent` service in " + + Message: "skipped: no `microsoft.foundry` service in " + "azure.yaml (see check " + "`local.agent-service-detected`).", } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_status.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_status.go index e689c98444a..ff8d722b922 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_status.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_agent_status.go @@ -209,7 +209,7 @@ func newCheckAgentStatus(deps Dependencies) Check { if priorBlocked(prior, "local.agent-service-detected") { return Result{ Status: StatusSkip, - Message: "skipped: no `azure.ai.agent` service in " + + Message: "skipped: no `microsoft.foundry` service in " + "azure.yaml (see check " + "`local.agent-service-detected`).", } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections.go index d3536124af1..202e85e7b03 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections.go @@ -37,12 +37,11 @@ type foundryConnectionsProbeFn func( ) ([]string, error) // newCheckConnections produces Check `remote.connections` (P5.1 -// C15). For each `ConnectionResource` declared in any service's -// `agent.manifest.yaml` (collected by the C2 manifest walker), the -// check queries the Foundry project's connection list and verifies a -// connection with the matching name exists. The check Passes when -// every manifest-declared connection has a corresponding entry; -// Fails when one or more are missing. +// C15). For each connection declared in any `microsoft.foundry` +// service's `connections` list in azure.yaml, the check queries the +// Foundry project's connection list and verifies a connection with the +// matching name exists. The check Passes when every declared connection +// has a corresponding entry; Fails when one or more are missing. // // # Skip cascade // @@ -135,7 +134,7 @@ func newCheckConnections(deps Dependencies) Check { if !state.HasConnections { return Result{ Status: StatusSkip, - Message: "skipped: no connection resources declared in any service's agent.manifest.yaml.", + Message: "skipped: no connection resources declared in any microsoft.foundry service in azure.yaml.", } } @@ -288,10 +287,10 @@ func classifyConnections( return Result{ Status: StatusFail, Message: fmt.Sprintf( - "%d connection(s) referenced by agent.manifest.yaml are missing on project %s: %s", + "%d connection(s) referenced by azure.yaml are missing on project %s: %s", len(missing), project, sb.String()), Suggestion: "Run `azd provision` to create the missing connection(s), " + - "or update the agent.manifest.yaml `resources[].name` entries to " + + "or update the `connections[].name` entries in azure.yaml to " + "match connections that already exist on the Foundry project.", Details: map[string]any{ "missingConnections": missing, diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local.go index f38e01f3bac..b8db45faba3 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local.go @@ -184,12 +184,12 @@ type Dependencies struct { } // NewLocalChecks returns the canonical sequence of local doctor checks -// in execution order. Phase 4.2 covered checks 1-3; Phase 4.3 added -// checks 4-6 (agent service detected, project endpoint set, agent.yaml -// valid). Phase 5 C9 appends check 7 (manual env vars set). Phase 5 -// C14 appends check 8 (`local.toolboxes`) which reads per-toolbox MCP -// endpoint env vars; it is local because it does not call ARM / -// Foundry (only the active azd environment). +// in execution order: gRPC/version, azure.yaml present, environment +// selected, agent service detected, project endpoint set, manual env +// vars, and toolbox endpoints. The unified design removed the standalone +// `agent.yaml` file, so the former per-service `local.agent-yaml-valid` +// check no longer applies — structural validity is covered by +// `local.azure-yaml`. func NewLocalChecks(deps Dependencies) []Check { return []Check{ newCheckGRPCAndVersion(deps), @@ -197,7 +197,6 @@ func NewLocalChecks(deps Dependencies) []Check { newCheckEnvironmentSelected(deps), newCheckAgentServiceDetected(deps), newCheckProjectEndpointSet(deps), - newCheckAgentYAMLValid(deps), newCheckManualEnvVars(deps), newCheckToolboxes(deps), } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local_test.go index 69a5a6f9fce..7a26079ce66 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local_test.go @@ -443,7 +443,7 @@ func TestNewLocalChecks_OrderAndIDs(t *testing.T) { t.Parallel() checks := NewLocalChecks(Dependencies{}) - require.Len(t, checks, 8) + require.Len(t, checks, 7) want := []struct { id string @@ -455,9 +455,8 @@ func TestNewLocalChecks_OrderAndIDs(t *testing.T) { {"local.environment-selected", "azd environment selected", false}, {"local.agent-service-detected", "agent service in azure.yaml", false}, {"local.project-endpoint-set", "FOUNDRY_PROJECT_ENDPOINT set", false}, - {"local.agent-yaml-valid", "agent.yaml 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", "Toolboxes have endpoint env vars set", false}, } for i, w := range want { require.Equal(t, w.id, checks[i].ID, "index %d", i) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env.go index 7a26416ee6e..af0208c0025 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env.go @@ -18,12 +18,12 @@ import ( // "manual config values not set" diagnostic. // // "Manual" env vars are values referenced by `${...}` syntax inside an -// agent.yaml whose names are NOT declared as outputs of the project's -// infrastructure (Bicep / Terraform). They are operator-supplied: -// third-party API keys, model deployment names, hand-rolled connection -// strings. They have to be set in the active azd environment before -// `azd ai agent run` (local) or `azd deploy` (Azure) can resolve the -// agent.yaml — otherwise the running agent sees the literal `${KEY}` +// agent's `env` block in azure.yaml whose names are NOT declared as +// outputs of the project's infrastructure (Bicep / Terraform). They are +// operator-supplied: third-party API keys, model deployment names, +// hand-rolled connection strings. They have to be set in the active azd +// environment before `azd ai agent run` (local) or `azd deploy` (Azure) +// can resolve them — otherwise the running agent sees the literal `${KEY}` // string and almost certainly fails on first use. // // The classification of "manual" vs "infra" lives in nextstep's @@ -42,12 +42,12 @@ import ( // - deps.AzdClient is nil (gRPC channel unavailable). Check // `local.grpc-extension` will already have failed with the actionable // error. -// - `local.agent-yaml-valid` failed or was skipped. A broken agent.yaml -// produces an empty MissingManualVars (the classifier can't extract -// references it can't parse), which would mislead the user into -// thinking nothing was missing. This guard transitively covers the -// azure-yaml → agent-service-detected → agent-yaml-valid arm of the -// local-check chain (each step's own skip-cascade propagates here). +// - `local.agent-service-detected` failed or was skipped. With no +// Foundry service there are no agents to extract env references from, +// which would produce an empty MissingManualVars and mislead the user +// into thinking nothing was missing. This guard transitively covers +// the azure-yaml → agent-service-detected arm of the local-check +// chain (each step's own skip-cascade propagates here). // - `local.environment-selected` failed or was skipped. // `nextstep.AssembleState` early-exits its `detectMissingVars` block // when no env is selected (state.go: `if project != nil && envName != ""`). @@ -73,8 +73,8 @@ func newCheckManualEnvVars(deps Dependencies) Check { if deps.AzdClient == nil { return Result{Status: StatusSkip, Message: "skipped: azd extension not reachable"} } - if priorBlocked(prior, "local.agent-yaml-valid") { - return Result{Status: StatusSkip, Message: "skipped: agent.yaml check failed or skipped"} + if priorBlocked(prior, "local.agent-service-detected") { + return Result{Status: StatusSkip, Message: "skipped: no microsoft.foundry service detected or upstream check blocked"} } if priorBlocked(prior, "local.environment-selected") { // Without an azd env, AssembleState's detectMissingVars @@ -82,7 +82,7 @@ func newCheckManualEnvVars(deps Dependencies) Check { // would be empty and the check would falsely Pass. return Result{ Status: StatusSkip, - Message: "skipped: no azd environment selected (cannot resolve agent.yaml variables)", + Message: "skipped: no azd environment selected (cannot resolve azure.yaml variables)", } } @@ -132,7 +132,7 @@ func newCheckManualEnvVars(deps Dependencies) Check { return Result{ Status: StatusFail, Message: fmt.Sprintf( - "%d manual env var(s) referenced by agent.yaml are not set in the azd environment: %s", + "%d manual env var(s) referenced by azure.yaml are not set in the azd environment: %s", len(missing), strings.Join(missing, ", ")), Suggestion: suggestion, Details: map[string]any{ diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env_test.go index a3cfc386071..ffbe26b8909 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_manual_env_test.go @@ -41,7 +41,7 @@ func TestCheckManualEnvVars_NoClient_Skips(t *testing.T) { require.Contains(t, got.Message, "azd extension not reachable") } -func TestCheckManualEnvVars_PriorAgentYAMLFailed_Skips(t *testing.T) { +func TestCheckManualEnvVars_PriorAgentDetectionFailed_Skips(t *testing.T) { t.Parallel() client := newTestAzdClient(t, &fakeProjectServer{}, &fakeEnvironmentServer{}) @@ -53,21 +53,21 @@ func TestCheckManualEnvVars_PriorAgentYAMLFailed_Skips(t *testing.T) { // here asserts the cascade short-circuits before the // assembler is reached. assembleState: func(context.Context, *azdext.AzdClient) (*nextstep.State, []error) { - t.Fatalf("assembler should not be called when local.agent-yaml-valid failed") + t.Fatalf("assembler should not be called when local.agent-service-detected failed") return nil, nil }, }) - prior := []Result{{ID: "local.agent-yaml-valid", Status: StatusFail}} + prior := []Result{{ID: "local.agent-service-detected", Status: StatusFail}} got := check.Fn(t.Context(), Options{}, prior) require.Equal(t, StatusSkip, got.Status) - require.Contains(t, got.Message, "agent.yaml check failed") + require.Contains(t, got.Message, "no microsoft.foundry service detected") } -func TestCheckManualEnvVars_PriorAgentYAMLSkipped_AlsoSkips(t *testing.T) { +func TestCheckManualEnvVars_PriorAgentDetectionSkipped_AlsoSkips(t *testing.T) { // Covers the cascade: a deeper upstream (e.g. azure-yaml) failed, - // agent-yaml-valid was therefore skipped, and this check must + // agent-service-detected was therefore skipped, and this check must // inherit the skip rather than running on a half-loaded project. // Without this propagation users would see a misleading // "no manual env vars are missing" Pass underneath the real bug. @@ -82,7 +82,7 @@ func TestCheckManualEnvVars_PriorAgentYAMLSkipped_AlsoSkips(t *testing.T) { }, }) - prior := []Result{{ID: "local.agent-yaml-valid", Status: StatusSkip}} + prior := []Result{{ID: "local.agent-service-detected", Status: StatusSkip}} got := check.Fn(t.Context(), Options{}, prior) require.Equal(t, StatusSkip, got.Status) @@ -288,11 +288,9 @@ func TestCheckManualEnvVars_NonFatalErrorsButStateOK_Passes(t *testing.T) { } func TestNewLocalChecks_IncludesManualEnvVarsLast(t *testing.T) { - // Pin C9's insertion point: the manual-env-vars check must follow - // agent-yaml-valid so its skip-cascade against the upstream chain - // is exercised by the runner's prior-results slice. Locks the - // ordering invariant that the design's "checks 1-7" table relies - // on for failure-cascade coherence. + // Pin the manual-env-vars insertion point: it must follow + // agent-service-detected so its skip-cascade against the upstream + // chain is exercised by the runner's prior-results slice. t.Parallel() checks := NewLocalChecks(Dependencies{}) @@ -304,17 +302,17 @@ func TestNewLocalChecks_IncludesManualEnvVarsLast(t *testing.T) { } require.Contains(t, ids, "local.manual-env-vars") - var yamlIdx, manualIdx int = -1, -1 + var detectIdx, manualIdx int = -1, -1 for i, id := range ids { switch id { - case "local.agent-yaml-valid": - yamlIdx = i + case "local.agent-service-detected": + detectIdx = i case "local.manual-env-vars": manualIdx = i } } - require.NotEqual(t, -1, yamlIdx, "agent-yaml-valid must be in NewLocalChecks") + require.NotEqual(t, -1, detectIdx, "agent-service-detected must be in NewLocalChecks") require.NotEqual(t, -1, manualIdx, "manual-env-vars must be in NewLocalChecks") - require.Greater(t, manualIdx, yamlIdx, - "manual-env-vars must come after agent-yaml-valid for the skip-cascade") + require.Greater(t, manualIdx, detectIdx, + "manual-env-vars must come after agent-service-detected for the skip-cascade") } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project.go index b3058d1a922..ab6291e160b 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project.go @@ -6,22 +6,16 @@ package doctor import ( "context" "fmt" - "os" "sort" "strings" - "azureaiagent/internal/pkg/agents/agent_yaml" - "azureaiagent/internal/pkg/paths" - "github.com/azure/azure-dev/cli/azd/pkg/azdext" ) -// agentHost is the value used in azure.yaml for an azure.ai.agent service. -// Must stay in sync with cmd.AiAgentHost ("azure.ai.agent") in -// `internal/cmd/init.go`; duplicated here so the doctor package does not -// have to import cmd (which would form an import cycle once the Cobra -// wiring lands in Phase 4.4). -const agentHost = "azure.ai.agent" +// agentHost is the value used in azure.yaml for a Foundry service under the +// unified design. Duplicated here so the doctor package does not have to +// import cmd (which would form an import cycle). +const agentHost = "microsoft.foundry" // projectEndpointVar is the azd environment variable that points at the // Foundry project. Must stay in sync with the rest of the extension @@ -30,7 +24,7 @@ const projectEndpointVar = "FOUNDRY_PROJECT_ENDPOINT" // newCheckAgentServiceDetected produces Check `local.agent-service-detected`. // It re-fetches the project config and counts services whose `host` is -// `azure.ai.agent`. Pass surfaces the count and service names so users +// `microsoft.foundry`. Pass surfaces the count and service names so users // can verify the doctor saw what they expected; Fail tells them to run // `azd ai agent init` to scaffold one. Skips when the gRPC client is // unavailable or when `local.azure-yaml` failed. @@ -48,7 +42,7 @@ func newCheckAgentServiceDetected(deps Dependencies) Check { resp, err := deps.AzdClient.Project().Get(ctx, &azdext.EmptyRequest{}) if err != nil { - suggestion := "Run `azd ai agent init` to add an azure.ai.agent service to azure.yaml." + suggestion := "Run `azd ai agent init` to add a microsoft.foundry service to azure.yaml." if isTransportFailure(err) { suggestion = "Re-run via `azd ai agent doctor`; the extension cannot reach azd's gRPC channel." } @@ -78,8 +72,8 @@ func newCheckAgentServiceDetected(deps Dependencies) Check { if len(agentServices) == 0 { return Result{ Status: StatusFail, - Message: "no `azure.ai.agent` service found in azure.yaml", - Suggestion: "Run `azd ai agent init` to add an azure.ai.agent service to azure.yaml.", + Message: "no `microsoft.foundry` service found in azure.yaml", + Suggestion: "Run `azd ai agent init` to add a microsoft.foundry service to azure.yaml.", } } return Result{ @@ -154,124 +148,6 @@ func newCheckProjectEndpointSet(deps Dependencies) Check { } } -// newCheckAgentYAMLValid produces Check `local.agent-yaml-valid`. For -// each agent service in azure.yaml, it reads `//agent.yaml` -// and parses it as `agent_yaml.ContainerAgent`. Fails when any service's -// file is missing, unreadable, or fails to parse — collecting all errors -// rather than short-circuiting so multi-service projects get a single -// actionable report. -// -// Skips when the gRPC client is unavailable or when -// `local.agent-service-detected` failed (no services to validate). The -// suggestion mirrors the spec's "fix YAML" guidance. -func newCheckAgentYAMLValid(deps Dependencies) Check { - return Check{ - ID: "local.agent-yaml-valid", - Name: "agent.yaml valid (per service)", - Fn: func(ctx context.Context, _ Options, prior []Result) Result { - if deps.AzdClient == nil { - return Result{Status: StatusSkip, Message: "skipped: azd extension not reachable"} - } - if priorBlocked(prior, "local.agent-service-detected") { - return Result{Status: StatusSkip, Message: "skipped: no agent services detected or upstream check blocked"} - } - - resp, err := deps.AzdClient.Project().Get(ctx, &azdext.EmptyRequest{}) - if err != nil { - suggestion := "Run from a directory containing `azure.yaml`, or initialize one with `azd init`." - if isTransportFailure(err) { - suggestion = "Re-run via `azd ai agent doctor`; the extension cannot reach azd's gRPC channel." - } - return Result{ - Status: StatusFail, - Message: fmt.Sprintf("failed to get project config: %v", err), - Suggestion: suggestion, - } - } - if resp == nil || resp.Project == nil { - return Result{ - Status: StatusFail, - Message: "failed to get project config (is there an azure.yaml?)", - Suggestion: "Run from a directory containing `azure.yaml`, or initialize one with `azd init`.", - } - } - - projectPath := resp.Project.Path - // Collect agent service entries in a stable order. protobuf - // `Services` is a map, so iteration order is non-deterministic - // — sorting by service name keeps the failure list (and the - // validatedPaths Detail) reproducible. - type agentSvc struct { - name string - rel string - } - var agents []agentSvc - for _, s := range resp.Project.Services { - if s == nil || s.Host != agentHost { - continue - } - agents = append(agents, agentSvc{name: s.Name, rel: s.RelativePath}) - } - sort.Slice(agents, func(i, j int) bool { return agents[i].name < agents[j].name }) - - var validatedPaths []string - var failures []string - for _, a := range agents { - yamlPath, err := paths.JoinAllowRoot(projectPath, a.rel, "agent.yaml") - if err != nil { - failures = append(failures, fmt.Sprintf("%s: %v", a.name, err)) - continue - } - if pathErr := validateAgentYAML(yamlPath); pathErr != nil { - failures = append(failures, fmt.Sprintf("%s: %v", a.name, pathErr)) - continue - } - validatedPaths = append(validatedPaths, yamlPath) - } - - if len(failures) > 0 { - return Result{ - Status: StatusFail, - Message: fmt.Sprintf( - "agent.yaml validation failed for %d service(s): %s", - len(failures), strings.Join(failures, "; ")), - Suggestion: "Fix the YAML syntax or ensure agent.yaml exists in each service directory.", - Details: map[string]any{ - "failures": failures, - "validatedPaths": validatedPaths, - }, - } - } - - return Result{ - Status: StatusPass, - Message: fmt.Sprintf("agent.yaml valid for %d service(s)", len(validatedPaths)), - Details: map[string]any{ - "validatedPaths": validatedPaths, - }, - } - }, - } -} - -// validateAgentYAML reads the file at path and runs the same validation -// (`agent_yaml.ValidateAgentDefinition`) that the deploy path uses, so a -// PASS here implies the manifest will not be rejected by deploy for any -// of: missing/invalid `kind`, missing/invalid `name`, or kind-specific -// structural problems. Returns the underlying read/validate error -// verbatim so the caller can attribute it to the offending service. -func validateAgentYAML(path string) error { - //nolint:gosec // path is validated under the project root before this helper is called. - data, err := os.ReadFile(path) - if err != nil { - return fmt.Errorf("read %s: %w", path, err) - } - if err := agent_yaml.ValidateAgentDefinition(data); err != nil { - return fmt.Errorf("validate %s: %w", path, err) - } - return nil -} - // priorBlocked reports whether the prior results contain a Fail or Skip // entry for the given check ID. Used for skip-cascade decisions across // the local-checks chain: when an upstream check is skipped (e.g. diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project_test.go index abb41d5864c..a061e43025a 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_project_test.go @@ -5,8 +5,6 @@ package doctor import ( "errors" - "os" - "path/filepath" "testing" "github.com/azure/azure-dev/cli/azd/pkg/azdext" @@ -104,7 +102,7 @@ func TestCheckAgentServiceDetected_NoAgentService_Fails(t *testing.T) { got := check.Fn(t.Context(), Options{}, nil) require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Message, "no `azure.ai.agent` service found") + require.Contains(t, got.Message, "no `microsoft.foundry` service found") require.Contains(t, got.Suggestion, "azd ai agent init") } @@ -285,235 +283,6 @@ func TestCheckProjectEndpointSet_ValidValue_Passes(t *testing.T) { require.Equal(t, endpoint, got.Details["projectEndpoint"]) } -// ---- Check `local.agent-yaml-valid` ---- - -func TestCheckAgentYAMLValid_NoClient_Skips(t *testing.T) { - t.Parallel() - - check := newCheckAgentYAMLValid(Dependencies{AzdClient: nil}) - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusSkip, got.Status) -} - -func TestCheckAgentYAMLValid_PriorAgentDetectionFailed_Skips(t *testing.T) { - t.Parallel() - - client := newTestAzdClient(t, &fakeProjectServer{}, &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - prior := []Result{{ID: "local.agent-service-detected", Status: StatusFail}} - got := check.Fn(t.Context(), Options{}, prior) - - require.Equal(t, StatusSkip, got.Status) - require.Contains(t, got.Message, "no agent services detected") -} - -func TestCheckAgentYAMLValid_PriorAgentDetectionSkipped_AlsoSkips(t *testing.T) { - // Covers the cascade: azure-yaml fails -> agent-service-detected skips -> - // agent-yaml-valid must also skip. Without this propagation, check 6 - // would re-fetch the project (failing identically to check 2) and - // surface a duplicate failure for the same root cause. - t.Parallel() - - client := newTestAzdClient(t, - // Server set up to fail if reached, to ensure the guard short-circuits. - &fakeProjectServer{err: errors.New("should not be called")}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - prior := []Result{{ID: "local.agent-service-detected", Status: StatusSkip}} - got := check.Fn(t.Context(), Options{}, prior) - - require.Equal(t, StatusSkip, got.Status) - require.Contains(t, got.Message, "no agent services detected or upstream check blocked") -} - -func TestCheckAgentYAMLValid_GRPCError_Fails(t *testing.T) { - t.Parallel() - - client := newTestAzdClient(t, - &fakeProjectServer{err: errors.New("rpc boom")}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Message, "failed to get project config") -} - -func TestCheckAgentYAMLValid_TransportError_SwapsSuggestion(t *testing.T) { - t.Parallel() - - client := newTestAzdClient(t, - &fakeProjectServer{err: status.Error(codes.Unavailable, "transport boom")}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Suggestion, "gRPC channel") -} - -func TestCheckAgentYAMLValid_OneServiceValid_Passes(t *testing.T) { - t.Parallel() - - projectPath := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectPath, "src", "agent"), 0o750)) - writeYAML(t, projectPath, "src/agent/agent.yaml", ` -kind: hosted -name: echo-agent -language: python -entrypoint: main.py -protocols: - - protocol: invocations - version: "1" -`) - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "echo-agent": {Name: "echo-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusPass, got.Status) - require.Contains(t, got.Message, "agent.yaml valid for 1 service(s)") -} - -func TestCheckAgentYAMLValid_NonAgentServicesIgnored(t *testing.T) { - t.Parallel() - - projectPath := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectPath, "src", "agent"), 0o750)) - writeYAML(t, projectPath, "src/agent/agent.yaml", "kind: hosted\nname: echo\nlanguage: python\n") - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "api": {Name: "api", Host: "containerapp", RelativePath: "src/api"}, - "echo-agent": {Name: "echo-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusPass, got.Status, "api service has no agent.yaml — must be skipped, not failed") - paths, ok := got.Details["validatedPaths"].([]string) - require.True(t, ok) - require.Len(t, paths, 1) - require.Contains(t, paths[0], "src"+string(filepath.Separator)+"agent") -} - -func TestCheckAgentYAMLValid_MissingFile_Fails(t *testing.T) { - t.Parallel() - - projectPath := t.TempDir() - // Note: no agent.yaml file created. - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "echo-agent": {Name: "echo-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Message, "echo-agent") - require.Contains(t, got.Suggestion, "Fix the YAML") - failures, ok := got.Details["failures"].([]string) - require.True(t, ok) - require.Len(t, failures, 1) -} - -func TestCheckAgentYAMLValid_MalformedYAML_Fails(t *testing.T) { - t.Parallel() - - projectPath := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectPath, "src", "agent"), 0o750)) - writeYAML(t, projectPath, "src/agent/agent.yaml", "name: echo\n bad-indent: oops\n: missing-key\n") - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "echo-agent": {Name: "echo-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Message, "echo-agent") - failures, ok := got.Details["failures"].([]string) - require.True(t, ok) - require.Len(t, failures, 1) -} - -func TestCheckAgentYAMLValid_MixedValidAndInvalid_Fails(t *testing.T) { - t.Parallel() - - projectPath := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectPath, "src", "ok"), 0o750)) - require.NoError(t, os.MkdirAll(filepath.Join(projectPath, "src", "bad"), 0o750)) - writeYAML(t, projectPath, "src/ok/agent.yaml", "kind: hosted\nname: ok-agent\nlanguage: python\n") - // bad: malformed yaml (mapping key with no value, broken indent). - writeYAML(t, projectPath, "src/bad/agent.yaml", "name: bad\n : nope\n\t- tabs-here\n") - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "ok-agent": {Name: "ok-agent", Host: agentHost, RelativePath: "src/ok"}, - "bad-agent": {Name: "bad-agent", Host: agentHost, RelativePath: "src/bad"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Message, "1 service(s)") // 1 failure - require.Contains(t, got.Message, "bad-agent") - require.NotContains(t, got.Message, "ok-agent: ") // ok-agent should not be in the failures list - - failures, ok := got.Details["failures"].([]string) - require.True(t, ok) - require.Len(t, failures, 1) - - validated, ok := got.Details["validatedPaths"].([]string) - require.True(t, ok) - require.Len(t, validated, 1) -} - // ---- helper: priorBlocked ---- func TestPriorBlocked(t *testing.T) { @@ -551,103 +320,3 @@ func TestPriorBlocked(t *testing.T) { }) } } - -func TestCheckAgentYAMLValid_MissingKind_Fails(t *testing.T) { - // Without explicit `kind:`, ValidateAgentDefinition rejects the manifest - // because kind is required. Doctor must catch this pre-flight rather - // than letting deploy be the first place that surfaces it. - t.Parallel() - - projectPath := t.TempDir() - writeYAML(t, projectPath, "src/agent/agent.yaml", "name: echo-agent\nlanguage: python\n") - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "echo-agent": {Name: "echo-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - require.Contains(t, got.Message, "echo-agent") - failures, ok := got.Details["failures"].([]string) - require.True(t, ok) - require.Len(t, failures, 1) - require.Contains(t, failures[0], "kind") -} - -func TestCheckAgentYAMLValid_InvalidKind_Fails(t *testing.T) { - // A `kind` that isn't in ValidAgentKinds() (hosted/workflow) must be - // rejected. Bare yaml.Unmarshal would silently accept this; the - // production deploy path rejects it via ValidateAgentDefinition. - t.Parallel() - - projectPath := t.TempDir() - writeYAML(t, projectPath, "src/agent/agent.yaml", "kind: nonsense\nname: echo-agent\nlanguage: python\n") - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "echo-agent": {Name: "echo-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - failures, ok := got.Details["failures"].([]string) - require.True(t, ok) - require.Len(t, failures, 1) - require.Contains(t, failures[0], "kind") -} - -func TestCheckAgentYAMLValid_InvalidName_Fails(t *testing.T) { - // Agent name must match `^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?$` - // (DNS-style). An underscore is invalid for deployable agent names. - // Doctor must surface this before deploy, not after. - t.Parallel() - - projectPath := t.TempDir() - writeYAML(t, projectPath, "src/agent/agent.yaml", "kind: hosted\nname: My_Agent\nlanguage: python\n") - - client := newTestAzdClient(t, - &fakeProjectServer{resp: &azdext.GetProjectResponse{ - Project: &azdext.ProjectConfig{ - Path: projectPath, - Services: map[string]*azdext.ServiceConfig{ - "my-agent": {Name: "my-agent", Host: agentHost, RelativePath: "src/agent"}, - }, - }, - }}, - &fakeEnvironmentServer{}) - check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) - - got := check.Fn(t.Context(), Options{}, nil) - - require.Equal(t, StatusFail, got.Status) - failures, ok := got.Details["failures"].([]string) - require.True(t, ok) - require.Len(t, failures, 1) - require.Contains(t, failures[0], "name") -} - -// writeYAML is a tiny test helper that writes the given content to -// / after ensuring the parent directory exists. -func writeYAML(t *testing.T, root, rel, content string) { - t.Helper() - full := filepath.Join(root, filepath.FromSlash(rel)) - require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o750)) - require.NoError(t, os.WriteFile(full, []byte(content), 0o600)) -} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes.go index aba74aac93b..096d2546997 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes.go @@ -26,9 +26,8 @@ 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 -// check verifies that the canonical +// For each toolbox declared in any `microsoft.foundry` service's +// `toolboxes` list in azure.yaml, the check verifies that the canonical // `TOOLBOX__MCP_ENDPOINT` env var is set to a // non-empty value in the active azd environment. // @@ -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 in azure.yaml; // the check has nothing to verify. // // # Why this check is not gated on `remote.auth` / @@ -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: "Toolboxes have endpoint env vars set", Remote: false, Fn: func(ctx context.Context, _ Options, prior []Result) Result { if deps.AzdClient == nil { @@ -119,7 +118,7 @@ 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 toolbox resources declared in any microsoft.foundry service in azure.yaml.", } } @@ -217,7 +216,7 @@ func classifyToolboxEndpoints( return Result{ Status: StatusFail, Message: fmt.Sprintf( - "%d toolbox(es) declared in agent.manifest.yaml have no MCP endpoint set in the azd environment: %s", + "%d toolbox(es) declared in azure.yaml have no MCP endpoint set in the azd environment: %s", len(missing), sb.String()), Suggestion: "Run `azd provision` to materialize toolbox infrastructure, or " + "`azd env set ` to point at an existing toolbox.", diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/foundry_config.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/foundry_config.go new file mode 100644 index 00000000000..4b32cdec45b --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/foundry_config.go @@ -0,0 +1,284 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package nextstep + +import ( + "encoding/json" + "fmt" + "os" + "path" + "strings" + + "azureaiagent/internal/pkg/paths" + + "go.yaml.in/yaml/v3" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/types/known/structpb" +) + +// foundry_config.go is the single decode seam for the unified azure.yaml +// design (host: microsoft.foundry). Everything the nextstep package knows +// about the on-disk/over-the-wire shape of a Foundry service lives here, so +// that when the upstream design is finalized the rest of the package +// (state assembly, resolvers, doctor consumers) is insulated from +// field-shape churn — only the structs below and decodeFoundryService need +// to change. +// +// Data flow (per the engineering spec, PR #8590): core azd parses +// azure.yaml and forwards each service's top-level non-standard keys +// (endpoint, deployments, connections, toolboxes, skills, routines, agents) +// verbatim in ServiceConfig.AdditionalProperties as a structpb.Struct (no +// env substitution applied). We round-trip that struct through JSON, then +// resolve any data-side `$ref:` file includes — which core does NOT expand +// — by reading the referenced sibling YAML file from disk and applying the +// spec's overlay + path-rebasing rules. +// +// nextstep is a leaf package (internal/project imports it), so the model is +// defined locally here rather than reusing internal/project types — reusing +// them would create an import cycle. + +// agentKind values mirror the design's `kind:` field. +const ( + agentKindHosted = "hosted" + agentKindPrompt = "prompt" +) + +// foundryServiceConfig is the typed view of a single `host: +// microsoft.foundry` service entry's Foundry-scoped state. Field shapes +// mirror the unified-design YAML keys; nextstep consumes only the subset it +// needs (names, kinds, protocols, env, endpoint, and connection/deployment +// identifiers). +type foundryServiceConfig struct { + // Endpoint, when non-empty, points at an EXISTING Foundry project + // (provision is skipped). Its presence is a "project already exists" + // signal for the next-step resolver. + Endpoint string + Deployments []foundryDeployment + Connections []foundryConnection + Toolboxes []foundryToolbox + Skills []foundrySkill + Routines []foundryRoutine + Agents []foundryAgent +} + +// foundryDeployment is one model deployment declared at the service level. +type foundryDeployment struct { + Name string `json:"name,omitempty"` + Model foundryDeployModel `json:"model"` +} + +// foundryDeployModel is the `model:` block of a deployment. +type foundryDeployModel struct { + Name string `json:"name,omitempty"` + Format string `json:"format,omitempty"` + Version string `json:"version,omitempty"` +} + +// foundryConnection is one project connection. nextstep only needs the +// name plus category/target for doctor remediation detail strings. +type foundryConnection struct { + Name string `json:"name,omitempty"` + Category string `json:"category,omitempty"` + Target string `json:"target,omitempty"` +} + +// foundryToolbox is one toolbox. nextstep only needs the name (the +// per-toolbox MCP endpoint env var is derived from it by envkey). +type foundryToolbox struct { + Name string `json:"name,omitempty"` +} + +// foundrySkill is one skill. Only the name is consumed today. +type foundrySkill struct { + Name string `json:"name,omitempty"` +} + +// foundryRoutine is one scheduled routine. Only the name is consumed today. +type foundryRoutine struct { + Name string `json:"name,omitempty"` +} + +// foundryAgent is one agent nested under the service. `kind: hosted` +// agents carry code/runtime fields (project, startupCommand, protocols); +// `kind: prompt` agents are pure config. Env values may contain ${VAR} +// (azd client-side) or ${{...}} (Foundry server-side) references, which +// arrive verbatim (core applies no substitution to AdditionalProperties). +type foundryAgent struct { + Name string `json:"name,omitempty"` + Kind string `json:"kind,omitempty"` + Description string `json:"description,omitempty"` + Project string `json:"project,omitempty"` + StartupCommand string `json:"startupCommand,omitempty"` + Protocols []foundryProtocol `json:"protocols,omitempty"` + Env map[string]string `json:"env,omitempty"` +} + +// foundryProtocol is one `protocols[]` entry under an agent. +type foundryProtocol struct { + Protocol string `json:"protocol,omitempty"` + Version string `json:"version,omitempty"` +} + +// foundryServiceRaw is the first-stage decode of the service properties. +// Every `oneOf` array (per the spec, each item may be an inline object or a +// `$ref` file include) is kept as raw maps so $ref resolution + overlay can +// happen at the map level before typed conversion. +type foundryServiceRaw struct { + Endpoint string `json:"endpoint,omitempty"` + Deployments []map[string]any `json:"deployments,omitempty"` + Connections []map[string]any `json:"connections,omitempty"` + Toolboxes []map[string]any `json:"toolboxes,omitempty"` + Skills []map[string]any `json:"skills,omitempty"` + Routines []map[string]any `json:"routines,omitempty"` + Agents []map[string]any `json:"agents,omitempty"` +} + +// decodeFoundryService converts a service's AdditionalProperties +// (structpb.Struct, as forwarded by core azd over gRPC) into the typed +// foundryServiceConfig, resolving any `$ref:` file includes relative to +// projectPath. +// +// Best-effort by contract: a nil struct yields an empty config; a decode +// error is returned so callers can surface it in --debug logs, but the +// (possibly partial) config is still returned. `$ref` resolution failures +// are silent — an unresolved item is dropped (its empty Name causes every +// downstream consumer to skip it). +func decodeFoundryService(props *structpb.Struct, projectPath string) (foundryServiceConfig, error) { + var cfg foundryServiceConfig + if props == nil { + return cfg, nil + } + + raw, err := protojson.Marshal(props) + if err != nil { + return cfg, fmt.Errorf("marshal service properties: %w", err) + } + var rawCfg foundryServiceRaw + if err := json.Unmarshal(raw, &rawCfg); err != nil { + return cfg, fmt.Errorf("decode foundry service config: %w", err) + } + + cfg.Endpoint = rawCfg.Endpoint + cfg.Deployments = decodeFoundryItems[foundryDeployment](rawCfg.Deployments, projectPath) + cfg.Connections = decodeFoundryItems[foundryConnection](rawCfg.Connections, projectPath) + cfg.Toolboxes = decodeFoundryItems[foundryToolbox](rawCfg.Toolboxes, projectPath) + cfg.Skills = decodeFoundryItems[foundrySkill](rawCfg.Skills, projectPath) + cfg.Routines = decodeFoundryItems[foundryRoutine](rawCfg.Routines, projectPath) + cfg.Agents = decodeFoundryItems[foundryAgent](rawCfg.Agents, projectPath) + return cfg, nil +} + +// decodeFoundryItems resolves each raw array item ($ref + overlay) and +// converts it to the typed element T. Items that fail typed conversion are +// skipped. Returns nil for an empty input so callers see the same shape as +// an absent key. +func decodeFoundryItems[T any](items []map[string]any, projectPath string) []T { + if len(items) == 0 { + return nil + } + out := make([]T, 0, len(items)) + for _, item := range items { + if item == nil { + continue + } + merged, ok := resolveFoundryItem(item, projectPath) + if !ok { + continue + } + var typed T + if mapToTyped(merged, &typed) { + out = append(out, typed) + } + } + return out +} + +// resolveFoundryItem applies the spec's `$ref` rules to one array item: +// - Inline item (no `$ref`): returned unchanged. +// - `$ref` item: the referenced sibling YAML file is loaded; its relative +// `project:` path is rebased to the referenced file's own directory +// (per the spec, split-file paths are relative to that file, not +// azure.yaml); then any sibling keys next to `$ref` overlay (shallow, +// top-level) on top of the loaded map. A `$ref` that cannot be read is +// dropped (ok=false). +func resolveFoundryItem(item map[string]any, projectPath string) (map[string]any, bool) { + ref, hasRef := item["$ref"].(string) + if !hasRef || strings.TrimSpace(ref) == "" { + return item, true + } + + loaded, ok := loadRefMap(projectPath, ref) + if !ok { + return nil, false + } + rebaseRefProject(loaded, path.Dir(filepathToSlash(ref))) + + // Shallow top-level overlay: sibling keys (except $ref) replace the + // loaded value. Scalars and arrays replace wholesale, matching the + // spec's "easy to predict" rule. + for k, v := range item { + if k == "$ref" { + continue + } + loaded[k] = v + } + return loaded, true +} + +// loadRefMap reads the YAML file at refPath (resolved path-safely under +// projectPath) into a generic map. All failure modes (path escape, missing +// file, malformed YAML) return ok=false. +func loadRefMap(projectPath, refPath string) (map[string]any, bool) { + resolved, err := paths.JoinAllowRoot(projectPath, refPath) + if err != nil { + return nil, false + } + data, err := os.ReadFile(resolved) //nolint:gosec // path is validated under the project root + if err != nil || len(data) == 0 { + return nil, false + } + var m map[string]any + if err := yaml.Unmarshal(data, &m); err != nil || m == nil { + return nil, false + } + return m, true +} + +// rebaseRefProject rewrites a loaded item's relative `project:` path so it +// is relative to the project root, given the directory that holds the +// referenced file. Absolute paths and non-string values are left alone. +// nextstep only consumes `project` (for README discovery); `instructions` +// and nested `$ref` rebasing are intentionally out of scope here. +func rebaseRefProject(m map[string]any, baseDir string) { + if baseDir == "" || baseDir == "." { + return + } + p, ok := m["project"].(string) + if !ok || p == "" { + return + } + slashed := filepathToSlash(p) + if path.IsAbs(slashed) { + return + } + m["project"] = path.Clean(baseDir + "/" + slashed) +} + +// mapToTyped converts a generic map into the typed element via a JSON +// round-trip (the element structs carry json tags matching the YAML keys). +// Returns false on any marshal/unmarshal error so the caller can skip a +// malformed entry rather than surface a zero value as if it were real. +func mapToTyped[T any](m map[string]any, out *T) bool { + b, err := json.Marshal(m) + if err != nil { + return false + } + return json.Unmarshal(b, out) == nil +} + +// filepathToSlash normalizes OS path separators to forward slashes so the +// slash-based path package operates correctly on Windows-authored refs. +func filepathToSlash(p string) string { + return strings.ReplaceAll(p, "\\", "/") +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/foundry_config_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/foundry_config_test.go new file mode 100644 index 00000000000..02746f3d8fd --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/foundry_config_test.go @@ -0,0 +1,144 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package nextstep + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" +) + +func TestDecodeFoundryService_InlineShape(t *testing.T) { + props, err := structpb.NewStruct(map[string]any{ + "endpoint": "https://acct.services.ai.azure.com/api/projects/p", + "deployments": []any{ + map[string]any{ + "name": "gpt-4.1-mini", + "model": map[string]any{"format": "OpenAI", "name": "gpt-4.1-mini", "version": "2025-04-14"}, + }, + }, + "connections": []any{ + map[string]any{"name": "search-conn", "category": "CognitiveSearch", "target": "https://x"}, + }, + "toolboxes": []any{ + map[string]any{"name": "support-toolbox"}, + }, + "skills": []any{map[string]any{"name": "triage"}}, + "routines": []any{map[string]any{"name": "nightly"}}, + "agents": []any{ + map[string]any{ + "name": "basic-agent", + "kind": "hosted", + "project": "src/basic-agent", + "startupCommand": "python main.py", + "protocols": []any{map[string]any{"protocol": "responses", "version": "1.0.0"}}, + "env": map[string]any{"FOUNDRY_MODEL_DEPLOYMENT_NAME": "gpt-4.1-mini"}, + }, + }, + }) + require.NoError(t, err) + + cfg, err := decodeFoundryService(props, "") + require.NoError(t, err) + + assert.Equal(t, "https://acct.services.ai.azure.com/api/projects/p", cfg.Endpoint) + + require.Len(t, cfg.Deployments, 1) + assert.Equal(t, "gpt-4.1-mini", cfg.Deployments[0].Name) + assert.Equal(t, "OpenAI", cfg.Deployments[0].Model.Format) + + require.Len(t, cfg.Connections, 1) + assert.Equal(t, "search-conn", cfg.Connections[0].Name) + assert.Equal(t, "CognitiveSearch", cfg.Connections[0].Category) + + require.Len(t, cfg.Toolboxes, 1) + assert.Equal(t, "support-toolbox", cfg.Toolboxes[0].Name) + + require.Len(t, cfg.Skills, 1) + assert.Equal(t, "triage", cfg.Skills[0].Name) + + require.Len(t, cfg.Routines, 1) + assert.Equal(t, "nightly", cfg.Routines[0].Name) + + require.Len(t, cfg.Agents, 1) + agent := cfg.Agents[0] + assert.Equal(t, "basic-agent", agent.Name) + assert.Equal(t, agentKindHosted, agent.Kind) + assert.Equal(t, "src/basic-agent", agent.Project) + assert.Equal(t, "python main.py", agent.StartupCommand) + require.Len(t, agent.Protocols, 1) + assert.Equal(t, "responses", agent.Protocols[0].Protocol) + assert.Equal(t, "gpt-4.1-mini", agent.Env["FOUNDRY_MODEL_DEPLOYMENT_NAME"]) +} + +func TestDecodeFoundryService_NilProps(t *testing.T) { + cfg, err := decodeFoundryService(nil, "") + require.NoError(t, err) + assert.Empty(t, cfg.Agents) + assert.Empty(t, cfg.Deployments) + assert.Empty(t, cfg.Endpoint) +} + +func TestDecodeFoundryService_ResolvesRefsWithRebaseAndOverlay(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(root, "agents"), 0o750)) + require.NoError(t, os.MkdirAll(filepath.Join(root, "toolboxes"), 0o750)) + // project: ../src/support is relative to the agents/ folder; after + // rebasing it must resolve to the project-root-relative src/support. + require.NoError(t, os.WriteFile(filepath.Join(root, "agents", "support.yaml"), []byte( + "name: support-agent\nkind: hosted\nproject: ../src/support\n"+ + "protocols:\n - protocol: responses\n version: \"1.0.0\"\n"+ + "env:\n FOUNDRY_MODEL_DEPLOYMENT_NAME: gpt-4.1\n"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(root, "toolboxes", "research.yaml"), []byte( + "name: research-toolbox\n"), 0o600)) + + props, err := structpb.NewStruct(map[string]any{ + "agents": []any{ + map[string]any{"name": "inline-agent", "kind": "prompt"}, + // $ref with a sibling overlay key: description should override. + map[string]any{"$ref": "./agents/support.yaml", "description": "overridden"}, + }, + "toolboxes": []any{ + map[string]any{"$ref": "./toolboxes/research.yaml"}, + }, + }) + require.NoError(t, err) + + cfg, err := decodeFoundryService(props, root) + require.NoError(t, err) + + require.Len(t, cfg.Agents, 2) + assert.Equal(t, "inline-agent", cfg.Agents[0].Name) + + resolved := cfg.Agents[1] + assert.Equal(t, "support-agent", resolved.Name, "$ref agent should be loaded") + assert.Equal(t, agentKindHosted, resolved.Kind) + assert.Equal(t, "src/support", resolved.Project, "relative project should rebase to the ref file's dir") + assert.Equal(t, "overridden", resolved.Description, "sibling key should overlay the loaded file") + assert.Equal(t, "gpt-4.1", resolved.Env["FOUNDRY_MODEL_DEPLOYMENT_NAME"]) + + require.Len(t, cfg.Toolboxes, 1) + assert.Equal(t, "research-toolbox", cfg.Toolboxes[0].Name, "$ref toolbox should be loaded") +} + +func TestDecodeFoundryService_UnresolvableRefDropped(t *testing.T) { + root := t.TempDir() + props, err := structpb.NewStruct(map[string]any{ + "agents": []any{ + map[string]any{"name": "keep-me", "kind": "prompt"}, + map[string]any{"$ref": "./agents/missing.yaml"}, + }, + }) + require.NoError(t, err) + + cfg, err := decodeFoundryService(props, root) + require.NoError(t, err) + // The unresolvable ref is dropped; the inline agent survives. + require.Len(t, cfg.Agents, 1) + assert.Equal(t, "keep-me", cfg.Agents[0].Name) +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go deleted file mode 100644 index 61128166ce5..00000000000 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go +++ /dev/null @@ -1,196 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package nextstep - -import ( - "cmp" - "os" - "slices" - - "azureaiagent/internal/pkg/agents/agent_yaml" - "azureaiagent/internal/pkg/paths" -) - -// manifestFileNames are the candidate manifest filenames the walker -// probes, in the same precedence order init / deploy paths use: -// agent.manifest.yaml wins over agent.manifest.yml. The non-manifest -// agent.yaml is deliberately NOT in this list — that file describes -// the running container (env vars, protocols) and never declares -// resources; mistakenly walking it would surface zero resources for -// every service that uses only agent.yaml (init-pending or -// agent.yaml-only templates). -var manifestFileNames = []string{ - "agent.manifest.yaml", - "agent.manifest.yml", -} - -// populateManifestResources walks each service's agent.manifest.yaml -// (when present) and aggregates the declared model/toolbox/connection -// resources onto state. The walker is strictly best-effort: missing -// files, unreadable bytes, malformed YAML, and unknown resource kinds -// are all silently skipped so an in-flight `azd ai agent init` (which -// rewrites the manifest mid-flight) or a template with no manifest -// (e.g., a bare agent.yaml) never blocks the rest of state assembly. -// -// Aggregation rules: -// -// - Has* flags are true when at least one resource of the matching -// kind is found across all services. -// - Slices are sorted by Name (ties broken by ServiceName) and the -// pair (ServiceName, Name) is the de-duplication key — the same -// name appearing under two services surfaces twice; the same name -// listed twice in one service collapses to one entry. This -// matches the doctor-check expectation that per-service failures -// remain individually addressable. -// - The Detail field carries a kind-specific summary (model id, -// connection target/category, empty for toolboxes) so doctor -// remediation lines have enough context to be actionable without -// re-parsing the manifest. -// -// Resource enumeration uses agent_yaml.ExtractResourceDefinitions -// directly (rather than LoadAndValidateAgentManifest) so a manifest -// with an absent / partial `template` block — common during init — -// still surfaces its `resources:` declarations. -func populateManifestResources(projectPath string, state *State) { - if state == nil || projectPath == "" || len(state.Services) == 0 { - return - } - - models := map[resourceKey]ResourceRef{} - toolboxes := map[resourceKey]ResourceRef{} - connections := map[resourceKey]ResourceRef{} - - for _, svc := range state.Services { - data := readManifestBytes(projectPath, svc.RelativePath) - if data == nil { - continue - } - resources, err := agent_yaml.ExtractResourceDefinitions(data) - if err != nil { - continue - } - for _, resource := range resources { - switch r := resource.(type) { - case agent_yaml.ModelResource: - if r.Name == "" { - continue - } - k := resourceKey{service: svc.Name, name: r.Name} - if _, dup := models[k]; dup { - continue - } - models[k] = ResourceRef{ - Name: r.Name, - ServiceName: svc.Name, - Detail: r.Id, - } - case agent_yaml.ToolboxResource: - if r.Name == "" { - continue - } - k := resourceKey{service: svc.Name, name: r.Name} - if _, dup := toolboxes[k]; dup { - continue - } - toolboxes[k] = ResourceRef{ - Name: r.Name, - ServiceName: svc.Name, - } - case agent_yaml.ConnectionResource: - if r.Name == "" { - continue - } - k := resourceKey{service: svc.Name, name: r.Name} - if _, dup := connections[k]; dup { - continue - } - connections[k] = ResourceRef{ - Name: r.Name, - ServiceName: svc.Name, - Detail: connectionDetail(r), - } - } - } - } - - state.ModelRefs = sortedResourceRefs(models) - state.Toolboxes = sortedResourceRefs(toolboxes) - state.Connections = sortedResourceRefs(connections) - state.HasModels = len(state.ModelRefs) > 0 - state.HasToolboxes = len(state.Toolboxes) > 0 - state.HasConnections = len(state.Connections) > 0 -} - -// readManifestBytes returns the first manifest file's contents under -// `//` (probing the names in -// manifestFileNames order) or nil if none exists / is readable. All -// failure modes — empty paths, missing directory, permission errors, -// truly empty file — return nil because every doctor / resolver -// consumer treats nil as "no manifest discovered for this service" -// and degrades gracefully. -func readManifestBytes(projectPath, relativePath string) []byte { - if projectPath == "" { - return nil - } - for _, name := range manifestFileNames { - manifestPath, err := paths.JoinAllowRoot(projectPath, relativePath, name) - if err != nil { - return nil - } - data, err := os.ReadFile(manifestPath) //nolint:gosec // path is validated under the project root - if err == nil && len(data) > 0 { - return data - } - } - return nil -} - -// connectionDetail renders the kind-specific identifier doctor -// remediation messages quote when a connection is missing or -// misconfigured. Empty-category and empty-target manifests fall back -// to whichever side is populated so we never emit a useless -// " | " separator with both halves blank. -func connectionDetail(r agent_yaml.ConnectionResource) string { - category := string(r.Category) - target := r.Target - switch { - case category != "" && target != "": - return category + " | " + target - case category != "": - return category - default: - return target - } -} - -// resourceKey is the (service, name) dedup key for the per-kind -// resource maps populated by populateManifestResources. Declared at -// package level so sortedResourceRefs can name the map type -// explicitly in its signature without a divergent anonymous-struct -// re-declaration. -type resourceKey struct { - service string - name string -} - -// sortedResourceRefs flattens the dedup map into a slice sorted by -// Name (ties broken by ServiceName). Callers consume the result by -// iterating in order, so the determinism is load-bearing for both -// doctor output snapshots and downstream display. -func sortedResourceRefs(m map[resourceKey]ResourceRef) []ResourceRef { - if len(m) == 0 { - return nil - } - out := make([]ResourceRef, 0, len(m)) - for _, v := range m { - out = append(out, v) - } - slices.SortFunc(out, func(a, b ResourceRef) int { - if c := cmp.Compare(a.Name, b.Name); c != 0 { - return c - } - return cmp.Compare(a.ServiceName, b.ServiceName) - }) - return out -} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest_test.go deleted file mode 100644 index 2a9691c4da9..00000000000 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest_test.go +++ /dev/null @@ -1,385 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package nextstep - -import ( - "context" - "fmt" - "os" - "path/filepath" - "testing" - - "azureaiagent/internal/pkg/agents/agent_yaml" - - "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -const manifestThreeKinds = ` -template: - kind: containerAgent - name: hello -resources: - - name: gpt-4o - kind: model - id: azureml://registries/azure-openai/models/gpt-4o/versions/2024-08-06 - - name: web-search - kind: toolbox - tools: - - id: tool-1 - - name: bing-conn - kind: connection - category: BingLLMSearch - target: https://api.bing.microsoft.com/ - authType: ApiKey -` - -const manifestNoResources = ` -template: - kind: containerAgent - name: hello -` - -const manifestModelsOnly = ` -resources: - - name: gpt-4o-mini - kind: model - id: azureml://registries/azure-openai/models/gpt-4o-mini/versions/2024-07-18 -` - -// writeManifest writes data to //agent.manifest.yaml, -// creating intermediate directories as needed. -func writeManifest(t *testing.T, projectRoot, rel, data string) { - t.Helper() - dir := filepath.Join(projectRoot, rel) - require.NoError(t, os.MkdirAll(dir, 0o750)) - path := filepath.Join(dir, "agent.manifest.yaml") - require.NoError(t, os.WriteFile(path, []byte(data), 0o600)) -} - -func TestAssembleState_ManifestWalker_AllThreeKinds(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - writeManifest(t, projectRoot, "src/echo", manifestThreeKinds) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - - assert.True(t, state.HasModels) - assert.True(t, state.HasToolboxes) - assert.True(t, state.HasConnections) - - require.Len(t, state.ModelRefs, 1) - assert.Equal(t, "gpt-4o", state.ModelRefs[0].Name) - assert.Equal(t, "echo", state.ModelRefs[0].ServiceName) - assert.Contains(t, state.ModelRefs[0].Detail, "gpt-4o") - - require.Len(t, state.Toolboxes, 1) - assert.Equal(t, "web-search", state.Toolboxes[0].Name) - assert.Equal(t, "echo", state.Toolboxes[0].ServiceName) - assert.Empty(t, state.Toolboxes[0].Detail) - - require.Len(t, state.Connections, 1) - assert.Equal(t, "bing-conn", state.Connections[0].Name) - assert.Equal(t, "echo", state.Connections[0].ServiceName) - assert.Equal(t, "BingLLMSearch | https://api.bing.microsoft.com/", state.Connections[0].Detail) -} - -func TestAssembleState_ManifestWalker_RootRelativePath(t *testing.T) { - t.Parallel() - - for _, rel := range []string{"", "."} { - t.Run(fmt.Sprintf("rel=%q", rel), func(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - writeManifest(t, projectRoot, rel, manifestModelsOnly) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: rel}, - }, - }, - } - - state, errs := assembleState(context.Background(), src) - - require.Empty(t, errs) - assert.True(t, state.HasModels) - require.Len(t, state.ModelRefs, 1) - assert.Equal(t, "gpt-4o-mini", state.ModelRefs[0].Name) - }) - } -} - -func TestAssembleState_ManifestWalker_MissingManifestNoError(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - // Service exists in azure.yaml but its directory has no manifest file - // at all. Walker must degrade silently. - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "src/echo"), 0o750)) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, errs := assembleState(context.Background(), src) - - for _, err := range errs { - assert.NotContains(t, err.Error(), "manifest") - } - assert.False(t, state.HasModels) - assert.False(t, state.HasToolboxes) - assert.False(t, state.HasConnections) - assert.Nil(t, state.ModelRefs) - assert.Nil(t, state.Toolboxes) - assert.Nil(t, state.Connections) -} - -func TestAssembleState_ManifestWalker_RejectsTraversal(t *testing.T) { - t.Parallel() - - parent := t.TempDir() - projectRoot := filepath.Join(parent, "project") - outside := filepath.Join(parent, "outside") - require.NoError(t, os.MkdirAll(projectRoot, 0o750)) - require.NoError(t, os.MkdirAll(outside, 0o750)) - require.NoError(t, os.WriteFile(filepath.Join(outside, "agent.manifest.yaml"), []byte(manifestThreeKinds), 0o600)) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "../outside"}, - }, - }, - } - - state, errs := assembleState(context.Background(), src) - - require.Empty(t, errs) - assert.False(t, state.HasModels) - assert.False(t, state.HasToolboxes) - assert.False(t, state.HasConnections) -} - -func TestAssembleState_ManifestWalker_MalformedManifestNoError(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - writeManifest(t, projectRoot, "src/echo", "::: this is not valid yaml :::") - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - assert.False(t, state.HasModels) - assert.False(t, state.HasToolboxes) - assert.False(t, state.HasConnections) -} - -func TestAssembleState_ManifestWalker_NoResourcesKey(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - writeManifest(t, projectRoot, "src/echo", manifestNoResources) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - assert.False(t, state.HasModels) - assert.False(t, state.HasToolboxes) - assert.False(t, state.HasConnections) -} - -func TestAssembleState_ManifestWalker_AggregatesAcrossServices(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - writeManifest(t, projectRoot, "src/a", manifestModelsOnly) - writeManifest(t, projectRoot, "src/b", manifestThreeKinds) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "a": {Name: "a", Host: agentHost, RelativePath: "src/a"}, - "b": {Name: "b", Host: agentHost, RelativePath: "src/b"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - assert.True(t, state.HasModels) - assert.True(t, state.HasToolboxes) - assert.True(t, state.HasConnections) - - require.Len(t, state.ModelRefs, 2) - // Sorted by Name ascending: gpt-4o (from "b") < gpt-4o-mini (from "a"). - assert.Equal(t, "gpt-4o", state.ModelRefs[0].Name) - assert.Equal(t, "b", state.ModelRefs[0].ServiceName) - assert.Equal(t, "gpt-4o-mini", state.ModelRefs[1].Name) - assert.Equal(t, "a", state.ModelRefs[1].ServiceName) -} - -func TestAssembleState_ManifestWalker_DedupSameServiceSameName(t *testing.T) { - t.Parallel() - - const dupManifest = ` -resources: - - name: gpt-4o - kind: model - id: first - - name: gpt-4o - kind: model - id: second -` - projectRoot := t.TempDir() - writeManifest(t, projectRoot, "src/echo", dupManifest) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - require.Len(t, state.ModelRefs, 1) - // First occurrence wins; subsequent dup is skipped silently. - assert.Equal(t, "first", state.ModelRefs[0].Detail) -} - -func TestAssembleState_ManifestWalker_PrefersYamlOverYml(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "src/echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "src/echo", "agent.manifest.yaml"), - []byte(manifestModelsOnly), - 0o600, - )) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "src/echo", "agent.manifest.yml"), - []byte(manifestThreeKinds), - 0o600, - )) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - // .yaml winner has models only, no toolboxes / connections. - assert.True(t, state.HasModels) - assert.False(t, state.HasToolboxes) - assert.False(t, state.HasConnections) - require.Len(t, state.ModelRefs, 1) - assert.Equal(t, "gpt-4o-mini", state.ModelRefs[0].Name) -} - -func TestAssembleState_ManifestWalker_IgnoresAgentYamlOnly(t *testing.T) { - t.Parallel() - - // agent.yaml (not agent.manifest.yaml) describes the container; it is - // not a manifest. The walker must NOT mistake it for one even when the - // content happens to parse: a service with only agent.yaml should - // surface no resources. - projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "src/echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "src/echo", "agent.yaml"), - []byte(manifestThreeKinds), - 0o600, - )) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "src/echo"}, - }, - }, - } - - state, _ := assembleState(context.Background(), src) - assert.False(t, state.HasModels) - assert.False(t, state.HasToolboxes) - assert.False(t, state.HasConnections) -} - -func TestConnectionDetail(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - category string - target string - want string - }{ - {"both populated", "AzureOpenAI", "https://x.openai.azure.com/", "AzureOpenAI | https://x.openai.azure.com/"}, - {"only category", "AzureOpenAI", "", "AzureOpenAI"}, - {"only target", "", "https://x.openai.azure.com/", "https://x.openai.azure.com/"}, - {"both empty", "", "", ""}, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - r := agent_yaml.ConnectionResource{ - Category: agent_yaml.CategoryKind(tc.category), - Target: tc.target, - } - assert.Equal(t, tc.want, connectionDetail(r)) - }) - } -} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver.go index 03a80c46da6..9a0cef94773 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver.go @@ -150,25 +150,6 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) }) } - // Placeholder fix-ups always come first when present: they are broken - // state in agent.yaml itself and block both `run` and `deploy`. The - // user has to edit agent.yaml (or define a matching parameter in - // agent.manifest.yaml) — `azd env set` cannot reach them. - hasPlaceholders := len(state.UnresolvedPlaceholders) > 0 - if hasPlaceholders { - placeholders := slices.Clone(state.UnresolvedPlaceholders) - slices.Sort(placeholders) - limit := min(len(placeholders), maxFixupLines) - for _, name := range placeholders[:limit] { - out = append(out, Suggestion{ - Command: fmt.Sprintf("edit agent.yaml: replace {{%s}} with the actual value", name), - Description: "agent.yaml has unresolved manifest placeholders", - Priority: priority, - }) - priority++ - } - } - hasToolboxEndpoints := len(state.MissingToolboxEndpoints) > 0 hasManualVars := len(state.MissingManualVars) > 0 @@ -248,24 +229,13 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) // Follow-up: once the user finishes the steps above (provision // for toolboxes, env-set for manual vars), the next productive // command is `azd ai agent run` and the invoke-local secondary. - // Suppressed when placeholders are also unresolved because - // literal `{{NAME}}` values in agent.yaml still break the local - // agent — the user must finish the placeholder fix-ups first; - // the trailing `azd deploy` reminder still applies. - if !hasPlaceholders { - out = append(out, Suggestion{ - Command: "azd ai agent run", - Description: runFollowUpDescription(hasToolboxEndpoints, hasManualVars), - Priority: priority, - }) - priority++ - out, _ = appendInvokeLocalSecondary(out, state, readmeExists, priority) - } - case hasPlaceholders: - // Only unresolved placeholders remain — do not emit - // `azd ai agent run` because running locally with literal - // `{{NAME}}` values produces a broken agent. The placeholder - // fix-ups above already tell the user what to do. + out = append(out, Suggestion{ + Command: "azd ai agent run", + Description: runFollowUpDescription(hasToolboxEndpoints, hasManualVars), + Priority: priority, + }) + priority++ + out, _ = appendInvokeLocalSecondary(out, state, readmeExists, priority) default: out = append(out, Suggestion{ Command: "azd ai agent run", diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver_test.go index 2bb5a3168d9..43a963e52a6 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver_test.go @@ -363,45 +363,23 @@ func TestResolveAfterInit_EverythingReady_EmitsInvokeLocalSecondary(t *testing.T // payload that may not match the user's chosen service. assert.Equal(t, `azd ai agent invoke --local ''`, out[1].Command) }) - - t.Run("placeholders present → invoke-local secondary suppressed (with run)", func(t *testing.T) { - // Placeholders block local run entirely — the spec's default - // branch is gated on !hasPlaceholders, so neither `run` nor - // the invoke-local follow-up should appear when literal - // {{NAME}} values would land in the running container. - t.Parallel() - state := &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"FOO"}, - } - out := ResolveAfterInit(state, nil) - for _, s := range out { - assert.NotContains(t, s.Command, "azd ai agent invoke --local", - "invoke --local must not be emitted while placeholders are unresolved") - assert.NotEqual(t, "azd ai agent run", s.Command, - "azd ai agent run must not be emitted while placeholders are unresolved") - } - }) } -// TestResolveAfterInit_ToolboxReproRendersAllCategories locks the full -// regression for the toolbox-sample bug end-to-end: the state contains -// BOTH an unresolved manifest placeholder AND a manifest-declared -// toolbox whose azd-injected endpoint env var is unset. The rendered -// "Next:" block must surface the placeholder fix-up AND route the -// missing toolbox endpoint to `azd provision` (NOT to `azd env set`), -// plus the trailing `azd deploy` reminder. +// TestResolveAfterInit_ToolboxEndpointRoutesToProvision locks the toolbox +// regression: a declared toolbox whose azd-injected endpoint env var is +// unset must route to `azd provision` (NOT `azd env set`), surface +// `azd ai agent doctor` as an existence-check follow-up, and keep the +// trailing `azd deploy` reminder. // // Before #8198's toolbox-endpoint partition this would have rendered -// "azd env set TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT " — see -// the test body's NotContains assertion below for the historical bug -// shape we're locking out. -func TestResolveAfterInit_ToolboxReproRendersAllCategories(t *testing.T) { +// "azd env set TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT " — see the +// NotContains assertion below for the historical bug shape we're locking +// out. +func TestResolveAfterInit_ToolboxEndpointRoutesToProvision(t *testing.T) { t.Parallel() state := &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"}, + HasProjectEndpoint: true, MissingToolboxEndpoints: []ResourceRef{ {Name: "web-search-tools", ServiceName: "agent"}, }, @@ -411,9 +389,6 @@ func TestResolveAfterInit_ToolboxReproRendersAllCategories(t *testing.T) { require.NoError(t, PrintAllNext(&buf, ResolveAfterInit(state, nil))) rendered := buf.String() - assert.Contains(t, rendered, - "edit agent.yaml: replace {{TOOLBOX_ENDPOINT}} with the actual value", - "placeholder fix-up missing") assert.Contains(t, rendered, "azd provision", "toolbox-endpoint branch should route to azd provision") assert.Contains(t, rendered, "azd ai agent doctor", @@ -421,8 +396,7 @@ func TestResolveAfterInit_ToolboxReproRendersAllCategories(t *testing.T) { // Historical bug: the resolver used to emit `azd env set` for // toolbox-derived endpoint vars. Those vars are azd-managed // outputs of `azd provision`, not operator-supplied, so the - // `azd env set` shape is wrong here regardless of the user's - // `` placeholder gripe. + // `azd env set` shape is wrong here. assert.NotContains(t, rendered, "azd env set TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT", "toolbox endpoint var must not be routed through `azd env set`") @@ -530,43 +504,6 @@ func TestResolveAfterInit_ToolboxAndManualVarsCoexist(t *testing.T) { assert.Contains(t, rendered, "azd deploy", "trailing deploy reminder missing") } -// TestResolveAfterInit_ToolboxAndManualVarsCoexistWithPlaceholders -// verifies the run/invoke suppression also fires for the coexistence -// case: when placeholders are also unresolved, the run + invoke-local -// pair must be suppressed regardless of which sub-branches contributed -// guidance above them. -func TestResolveAfterInit_ToolboxAndManualVarsCoexistWithPlaceholders(t *testing.T) { - t.Parallel() - - state := &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"AGENT_NAME"}, - MissingToolboxEndpoints: []ResourceRef{ - {Name: "web-search-tools", ServiceName: "agent"}, - }, - MissingManualVars: []string{"MY_API_KEY"}, - } - - var buf strings.Builder - require.NoError(t, PrintAllNext(&buf, ResolveAfterInit(state, nil))) - rendered := buf.String() - - assert.Contains(t, rendered, - "edit agent.yaml: replace {{AGENT_NAME}} with the actual value", - "placeholder fix-up missing") - assert.Contains(t, rendered, "azd provision", - "coexistence+placeholders: toolbox sub-branch must still emit provision") - assert.Contains(t, rendered, "azd ai agent doctor", - "coexistence+placeholders: toolbox sub-branch must still emit doctor follow-up") - assert.Contains(t, rendered, "azd env set MY_API_KEY ", - "coexistence+placeholders: manual sub-branch must still surface env-set") - assert.NotContains(t, rendered, "azd ai agent run", - "coexistence+placeholders: run must be suppressed while placeholders are unresolved") - assert.NotContains(t, rendered, "azd ai agent invoke --local", - "coexistence+placeholders: invoke-local must be suppressed while placeholders are unresolved") - assert.Contains(t, rendered, "azd deploy", "trailing deploy reminder missing") -} - // TestRunFollowUpDescription exercises the helper directly so future // changes to the description text (or a regression in the conditional // branching) get caught even when the higher-level resolver tests @@ -613,120 +550,6 @@ func TestRunFollowUpDescription(t *testing.T) { } } -func TestResolveAfterInit_UnresolvedPlaceholders(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - state *State - wantPlaceholders []string // expected `{{NAME}}` names in order - wantMiddle string // expected non-trailing, non-placeholder primary (e.g., "azd provision", "azd env set FOO", or "" if none) - wantHasRun bool // expect `azd ai agent run` to appear? - wantHasDeploy bool // expect `azd deploy` trailing? - }{ - { - name: "placeholders alone → edit lines + deploy, no run", - state: &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - }, - wantPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - wantHasRun: false, - wantHasDeploy: true, - }, - { - name: "placeholders + missing manual vars → both surfaced, no run", - state: &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - MissingManualVars: []string{"TOOLBOX_MCP_ENDPOINT"}, - }, - wantPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - wantMiddle: "azd env set TOOLBOX_MCP_ENDPOINT", - wantHasRun: false, - wantHasDeploy: true, - }, - { - name: "placeholders + project endpoint missing → placeholders + provision", - state: &State{ - HasProjectEndpoint: false, - UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - }, - wantPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - wantMiddle: "azd provision", - wantHasRun: false, - wantHasDeploy: true, - }, - { - name: "multiple placeholders sorted ascending", - state: &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"CHARLIE", "ALPHA", "BRAVO"}, - }, - wantPlaceholders: []string{"ALPHA", "BRAVO", "CHARLIE"}, - wantHasRun: false, - wantHasDeploy: true, - }, - { - name: "more than three placeholders capped at three", - state: &State{ - HasProjectEndpoint: true, - UnresolvedPlaceholders: []string{"P1", "P2", "P3", "P4", "P5"}, - }, - wantPlaceholders: []string{"P1", "P2", "P3"}, - wantHasRun: false, - wantHasDeploy: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - out := ResolveAfterInit(tt.state, nil) - require.NotEmpty(t, out) - - // Walk the output: - // 1. leading run of placeholder fix-ups (one per wantPlaceholders[i]) - // 2. optional middle command (provision / env set) - // 3. optional `azd ai agent run` - // 4. trailing `azd deploy` - for i, name := range tt.wantPlaceholders { - require.Less(t, i, len(out)) - assert.Equal(t, - "edit agent.yaml: replace {{"+name+"}} with the actual value", - out[i].Command, - ) - } - - // The middle (if any) sits just past the placeholders. - if tt.wantMiddle != "" { - idx := len(tt.wantPlaceholders) - require.Less(t, idx, len(out)) - assert.True(t, - strings.HasPrefix(out[idx].Command, tt.wantMiddle), - "middle suggestion %q does not have prefix %q", - out[idx].Command, tt.wantMiddle, - ) - } - - hasRun := false - hasDeploy := false - for _, s := range out { - switch { - case s.Command == "azd ai agent run": - hasRun = true - case s.Command == "azd deploy" && s.Trailing: - hasDeploy = true - } - } - assert.Equal(t, tt.wantHasRun, hasRun, - "presence of `azd ai agent run` mismatched") - assert.Equal(t, tt.wantHasDeploy, hasDeploy, - "presence of trailing `azd deploy` mismatched") - }) - } -} - func TestResolveAfterRun(t *testing.T) { t.Parallel() diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state.go index 8f83adba1bb..e788afeb3d9 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state.go @@ -8,29 +8,26 @@ import ( "errors" "fmt" "maps" - "os" "path/filepath" "regexp" "slices" "strings" - "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/envkey" - "azureaiagent/internal/pkg/paths" "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "go.yaml.in/yaml/v3" ) const ( - // agentHost matches the value set in azure.yaml for an azure.ai.agent - // service. Duplicated here (rather than imported from the parent cmd - // package) so nextstep stays free of upward dependencies; Phase 2 will - // wire cmd → nextstep, so the reverse import would close a cycle. - agentHost = "azure.ai.agent" + // agentHost matches the `host:` value of a Foundry service entry in + // azure.yaml under the unified design. Duplicated here (rather than + // imported from the parent cmd package) so nextstep stays a leaf + // package — cmd and project both import nextstep, so a reverse import + // would close a cycle. + agentHost = "microsoft.foundry" // agentVersionVarFormat is the env-var name that signals a deployed - // agent service. Filled with the upper-cased service key. + // agent. Filled with the upper-cased agent key. agentVersionVarFormat = "AGENT_%s_VERSION" // projectEndpointVar is the env-var that carries the Foundry project @@ -62,28 +59,22 @@ const ( azureLocationVar = "AZURE_LOCATION" ) -// envVarRefPattern captures ${VAR} references inside YAML string values. +// envVarRefPattern captures ${VAR} references inside string values. // Group 1 is the variable name. Group 2 captures the optional default -// tail `:-fallback`; when group 2 is non-empty the agent.yaml author -// explicitly opted into a fallback and the variable is therefore not -// required at deploy time (the runtime expander `drone/envsubst` honors -// `:-` semantics). `extractAgentYamlEnvRefs` skips refs with a non-empty -// group 2 so they never surface in the missing-vars hints; the variable -// is reported as missing only when authored as the bare `${VAR}` form. -// Variable names follow the standard shell convention: leading letter or -// underscore, then alphanumeric or underscore. +// tail `:-fallback`; when group 2 is non-empty the author explicitly +// opted into a fallback and the variable is therefore not required at +// deploy time (the runtime expander `drone/envsubst` honors `:-` +// semantics). `extractEnvRefs` skips refs with a non-empty group 2 so +// they never surface in the missing-vars hints; the variable is reported +// as missing only when authored as the bare `${VAR}` form. Variable names +// follow the standard shell convention: leading letter or underscore, +// then alphanumeric or underscore. +// +// Note the leading `\$\{` does not match the `${{...}}` Foundry +// server-side form (the second `{` is not a valid variable-name start), +// so those references are intentionally left alone. var envVarRefPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)(:-[^}]*)?\}`) -// placeholderPattern aliases agent_yaml.PlaceholderPattern. nextstep -// surfaces the same placeholders that agent_yaml's -// injectParameterValues warns about, so the two MUST stay in lockstep. -// Keeping a single shared regex (defined in agent_yaml, where the -// substitution logic lives) makes that constraint explicit and avoids -// drift if the placeholder syntax is ever broadened again. See -// agent_yaml/placeholders.go for the full rationale on the regex -// shape (hyphens, dots, whitespace in capture group). -var placeholderPattern = agent_yaml.PlaceholderPattern - // Source is the read-only view of azd that AssembleState needs. // // The production implementation wraps an *azdext.AzdClient via NewSource; @@ -279,27 +270,26 @@ func assembleState(ctx context.Context, src Source, opts ...Option) (*State, []e errs = append(errs, fmt.Errorf("load project: %w", err)) } - state.Services = collectServices(ctx, src, envName, project, &errs) + // collectFoundry decodes each Foundry service entry once and fills + // both state.Services (one entry per agent) and the aggregate + // resource refs (ModelRefs / Toolboxes / Connections + Has* flags). + collectFoundry(ctx, src, envName, project, state, &errs) if project != nil && envName != "" { - state.MissingInfraVars, state.MissingManualVars, state.UnresolvedPlaceholders = detectMissingVars( + state.MissingInfraVars, state.MissingManualVars = detectMissingVars( ctx, src, envName, project.Path, state.Services, &errs, ) populateOpenAPIPayload(ctx, cfg, project.Path, envName, state) } - if project != nil && len(state.Services) > 0 { - populateManifestResources(project.Path, state) - } - // Partition toolbox-derived endpoint vars out of MissingManualVars - // into MissingToolboxEndpoints. This must run AFTER - // populateManifestResources because it depends on state.Toolboxes - // being populated — without the manifest's toolbox list we cannot - // tell a toolbox-derived var ("TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT" - // for a manifest-declared `web-search-tools` toolbox) apart from a - // generic user-named variable that happens to start with TOOLBOX_. - // See MissingToolboxEndpoints docs (types.go) for the rationale. + // into MissingToolboxEndpoints. This must run AFTER collectFoundry + // because it depends on state.Toolboxes being populated — without the + // toolbox list we cannot tell a toolbox-derived var + // ("TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT" for a declared + // `web-search-tools` toolbox) apart from a generic user-named variable + // that happens to start with TOOLBOX_. See MissingToolboxEndpoints + // docs (types.go) for the rationale. partitionToolboxEndpointVars(state) return state, errs @@ -422,63 +412,114 @@ func populateOpenAPIPayload( state.OpenAPIPayload = payload } -func collectServices( +// collectFoundry decodes each `host: microsoft.foundry` service entry once +// and populates state with: +// +// - state.Services — one entry PER AGENT (the unit the resolvers show +// and invoke), with the owning Foundry service name recorded in +// ServiceName. +// - state.ModelRefs / Toolboxes / Connections + Has* flags — the +// aggregate Foundry resources declared across all Foundry services. +// - state.HasProjectEndpoint — also set true when any Foundry service +// declares an explicit `endpoint:` (reusing an existing project). +// +// The decode source is the service's AdditionalProperties (forwarded by +// core over gRPC); there is no on-disk re-parse of azure.yaml. Decode +// errors are surfaced into errs but never abort assembly — a partially +// populated config still yields useful guidance. +func collectFoundry( ctx context.Context, src Source, envName string, project *azdext.ProjectConfig, + state *State, errs *[]error, -) []ServiceState { +) { if project == nil || len(project.Services) == 0 { - return nil + return } - services := make([]ServiceState, 0, len(project.Services)) + models := map[resourceKey]ResourceRef{} + toolboxes := map[resourceKey]ResourceRef{} + connections := map[resourceKey]ResourceRef{} + + var services []ServiceState for _, svc := range project.Services { if svc == nil || svc.Host != agentHost { continue } - services = append(services, ServiceState{ - Name: svc.Name, - Host: svc.Host, - RelativePath: svc.RelativePath, - Protocol: loadServiceProtocol(project.Path, svc.RelativePath), - IsDeployed: isDeployed(ctx, src, envName, svc.Name, errs), - }) + cfg, err := decodeFoundryService(svc.AdditionalProperties, project.Path) + if err != nil { + *errs = append(*errs, fmt.Errorf("decode service %q: %w", svc.Name, err)) + } + if cfg.Endpoint != "" { + state.HasProjectEndpoint = true + } + + for _, agent := range cfg.Agents { + if agent.Name == "" { + continue + } + services = append(services, ServiceState{ + Name: agent.Name, + Kind: agent.Kind, + Host: svc.Host, + ServiceName: svc.Name, + Protocol: preferredProtocol(agent.Protocols), + RelativePath: agent.Project, + IsDeployed: isDeployed(ctx, src, envName, agent.Name, errs), + Env: agent.Env, + }) + } + + for _, d := range cfg.Deployments { + addResourceRef(models, svc.Name, d.Name, deploymentDetail(d)) + } + for _, tb := range cfg.Toolboxes { + addResourceRef(toolboxes, svc.Name, tb.Name, "") + } + for _, c := range cfg.Connections { + addResourceRef(connections, svc.Name, c.Name, connectionDetail(c)) + } } slices.SortFunc(services, func(a, b ServiceState) int { - return strings.Compare(a.Name, b.Name) + if c := strings.Compare(a.Name, b.Name); c != 0 { + return c + } + return strings.Compare(a.ServiceName, b.ServiceName) }) - return services + state.Services = services + + state.ModelRefs = sortedResourceRefs(models) + state.Toolboxes = sortedResourceRefs(toolboxes) + state.Connections = sortedResourceRefs(connections) + state.HasModels = len(state.ModelRefs) > 0 + state.HasToolboxes = len(state.Toolboxes) > 0 + state.HasConnections = len(state.Connections) > 0 } -// loadServiceProtocol returns the protocol the service's agent.yaml declares -// for next-step hint purposes. The lookup is best-effort: missing or -// malformed manifests, empty protocols sections, or any I/O error all return -// an empty string, and the resolver falls back to ProtocolResponses. When the -// manifest declares multiple protocols, ProtocolResponses wins over -// ProtocolInvocations so the suggested payload works on the broadest set of -// agents. -func loadServiceProtocol(projectPath, relativePath string) string { - if projectPath == "" { - return "" - } - manifestPath, err := paths.JoinAllowRoot(projectPath, relativePath, "agent.yaml") - if err != nil { - return "" - } - data, err := os.ReadFile(manifestPath) //nolint:gosec // path is validated under the project root - if err != nil { - return "" +// addResourceRef inserts a ResourceRef into m keyed on (service, name), +// skipping empty names and (service, name) duplicates so the same resource +// declared twice within one service collapses to one entry. +func addResourceRef(m map[resourceKey]ResourceRef, serviceName, name, detail string) { + if name == "" { + return } - var hosted agent_yaml.ContainerAgent - if err := yaml.Unmarshal(data, &hosted); err != nil { - return "" + k := resourceKey{service: serviceName, name: name} + if _, dup := m[k]; dup { + return } + m[k] = ResourceRef{Name: name, ServiceName: serviceName, Detail: detail} +} +// preferredProtocol picks the protocol the next-step hints use for an +// agent: ProtocolResponses wins over ProtocolInvocations (so the suggested +// payload works on the broadest set of agents). Empty when neither is +// declared. +func preferredProtocol(protocols []foundryProtocol) string { sawInvocations := false - for _, p := range hosted.Protocols { + for _, p := range protocols { switch strings.TrimSpace(p.Protocol) { case ProtocolResponses: return ProtocolResponses @@ -492,72 +533,109 @@ func loadServiceProtocol(projectPath, relativePath string) string { return "" } -// detectMissingVars walks each service's agent.yaml environment_variables -// section and partitions the trouble-spots into three lists: +// deploymentDetail renders the kind-specific ResourceRef.Detail for a model +// deployment: "/" when both are present, else whichever side +// is populated. +func deploymentDetail(d foundryDeployment) string { + switch { + case d.Model.Format != "" && d.Model.Name != "": + return d.Model.Format + "/" + d.Model.Name + case d.Model.Format != "": + return d.Model.Format + default: + return d.Model.Name + } +} + +// connectionDetail renders the kind-specific identifier doctor remediation +// messages quote for a connection. Empty-category and empty-target entries +// fall back to whichever side is populated so we never emit a useless +// " | " separator with both halves blank. +func connectionDetail(c foundryConnection) string { + switch { + case c.Category != "" && c.Target != "": + return c.Category + " | " + c.Target + case c.Category != "": + return c.Category + default: + return c.Target + } +} + +// resourceKey is the (service, name) dedup key for the per-kind resource +// maps populated by collectFoundry. +type resourceKey struct { + service string + name string +} + +// sortedResourceRefs flattens the dedup map into a slice sorted by Name +// (ties broken by ServiceName). The determinism is load-bearing for doctor +// output snapshots and downstream display. +func sortedResourceRefs(m map[resourceKey]ResourceRef) []ResourceRef { + if len(m) == 0 { + return nil + } + out := make([]ResourceRef, 0, len(m)) + for _, v := range m { + out = append(out, v) + } + slices.SortFunc(out, func(a, b ResourceRef) int { + if c := strings.Compare(a.Name, b.Name); c != 0 { + return c + } + return strings.Compare(a.ServiceName, b.ServiceName) + }) + return out +} + +// detectMissingVars scans each agent's `env` map (from azure.yaml) and +// partitions the unset ${VAR} references into two lists: // -// 1. infra: unset ${VAR} refs that name a top-level output of +// 1. infra: unset ${VAR} refs that name a top-level output of // /infra/main.bicep (provision outputs) -// 2. manual: unset ${VAR} refs that do NOT name a Bicep output -// (user inputs the user must `azd env set`) -// 3. placeholders: surviving {{NAME}} Mustache placeholders (init failed -// to substitute these from agent.manifest.yaml's parameters block) +// 2. manual: unset ${VAR} refs that do NOT name a Bicep output (user +// inputs the user must `azd env set`) // -// Only bare-form ${VAR} refs participate in (1) and (2): when the -// agent.yaml author supplies an explicit fallback via `${VAR:-default}`, -// the deploy-time resolver substitutes the fallback and the variable is -// not required. `extractAgentYamlEnvRefs` filters defaulted refs out. +// Only bare-form ${VAR} refs participate: when the author supplies an +// explicit fallback via `${VAR:-default}`, the deploy-time resolver +// substitutes the fallback and the variable is not required. +// `extractEnvRefs` filters defaulted refs out. ${{...}} Foundry +// server-side references are not matched and never surface here. // -// Classification rule for ${VAR}: a variable is an infra var iff its -// name is declared as a top-level `output` in `/infra/ -// main.bicep`. azd's Bicep provider writes those output names verbatim -// into `.azure//.env` after `azd provision` succeeds (see -// cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go around the -// `outputs[key] = ...` write and pkg/infra/provisioning/manager.go's -// `UpdateEnvironment` → `env.DotenvSet(key, ...)`), so set membership -// is a precise signal of "this variable is provided by `azd provision`." -// Everything else is treated as a user-supplied manual variable that -// the user must set via `azd env set`. This mirrors the spec wording in -// issue #7975 ("Walk azure.yaml service configs; collect ${...} -// references that map to known Bicep outputs"). +// Classification rule for ${VAR}: a variable is an infra var iff its name +// is declared as a top-level `output` in `/infra/main.bicep`. +// azd's Bicep provider writes those output names verbatim into +// `.azure//.env` after `azd provision` succeeds, so set membership is +// a precise signal of "this variable is provided by `azd provision`." +// Everything else is treated as a user-supplied manual variable that the +// user must set via `azd env set`. // // When `infra/main.bicep` is missing or declares no outputs, the // Bicep-output set is empty and every unresolved bare ref lands in the -// manual bucket. This is the conservative answer: the resolver will -// emit `azd env set ` hints, which a user can always -// follow. If the project is actually backed by a Bicep template whose -// outputs are not yet declared, the fix is to declare the missing -// output — not to guess based on the variable name. +// manual bucket. This is the conservative answer: the resolver emits +// `azd env set ` hints, which a user can always follow. // -// {{NAME}} placeholders are reported separately because the user cannot -// fix them with `azd env set` — the placeholder is literally inside -// agent.yaml and would land in the container as `{{NAME}}` at deploy -// time. The resolver surfaces an "edit agent.yaml" suggestion for each. -// -// All three result lists are deduplicated and sorted ascending. Read -// errors on individual agent.yaml files are silent: the resolver should -// fall back to the default branch rather than emit guidance that -// mentions variables we cannot prove are needed. Transport errors from -// src.EnvValue are appended to errs so AssembleState's caller can -// surface them in --debug logs without aborting the snapshot. +// Both result lists are deduplicated and sorted ascending. Transport +// errors from src.EnvValue are appended to errs so AssembleState's caller +// can surface them in --debug logs without aborting the snapshot. func detectMissingVars( ctx context.Context, src Source, envName, projectPath string, services []ServiceState, errs *[]error, -) (infra, manual, placeholders []string) { +) (infra, manual []string) { if envName == "" || projectPath == "" || len(services) == 0 { - return nil, nil, nil + return nil, nil } bicepOutputs := bicepOutputSet(projectPath) seenInfra := make(map[string]struct{}) seenManual := make(map[string]struct{}) - seenPlaceholder := make(map[string]struct{}) for _, svc := range services { - refs, phs := extractAgentYamlEnvRefs(projectPath, svc.RelativePath) - for _, name := range refs { + for _, name := range extractEnvRefs(svc.Env) { if _, ok := seenInfra[name]; ok { continue } @@ -578,15 +656,11 @@ func detectMissingVars( seenManual[name] = struct{}{} } } - for _, name := range phs { - seenPlaceholder[name] = struct{}{} - } } infra = slices.Sorted(maps.Keys(seenInfra)) manual = slices.Sorted(maps.Keys(seenManual)) - placeholders = slices.Sorted(maps.Keys(seenPlaceholder)) - return infra, manual, placeholders + return infra, manual } // bicepOutputSet returns the Bicep-output names declared by @@ -603,44 +677,19 @@ func bicepOutputSet(projectPath string) map[string]struct{} { return set } -// extractAgentYamlEnvRefs returns two lists from the service's -// agent.yaml environment_variables block: -// -// 1. refs: unique bare-form ${VAR} names. Refs that supply a fallback -// via `${VAR:-default}` are skipped — the deploy-time expander -// honors the default, so the variable is not required and never -// warrants a missing-var hint. -// 2. placeholders: unique {{NAME}} Mustache-style placeholders that -// init's manifest processing failed to substitute. These would land -// in the container literally as `{{NAME}}` at deploy time. -// -// Order matches first appearance in the file. Missing or malformed -// manifests return nil for both — consistent with loadServiceProtocol's -// best-effort contract. -func extractAgentYamlEnvRefs(projectPath, relativePath string) (refs, placeholders []string) { - if projectPath == "" { - return nil, nil - } - manifestPath, err := paths.JoinAllowRoot(projectPath, relativePath, "agent.yaml") - if err != nil { - return nil, nil - } - data, err := os.ReadFile(manifestPath) //nolint:gosec // path is validated under the project root - if err != nil { - return nil, nil - } - var hosted agent_yaml.ContainerAgent - if err := yaml.Unmarshal(data, &hosted); err != nil { - return nil, nil - } - if hosted.EnvironmentVariables == nil { - return nil, nil +// extractEnvRefs returns the unique bare-form ${VAR} names referenced in an +// agent's env map. Refs that carry a `:-default` fallback are skipped (the +// deploy-time expander honors the default, so the variable is not +// required). ${{...}} Foundry server-side references are not matched by the +// pattern and are therefore ignored. +func extractEnvRefs(env map[string]string) []string { + if len(env) == 0 { + return nil } - - seenRef := make(map[string]struct{}) - seenPh := make(map[string]struct{}) - for _, ev := range *hosted.EnvironmentVariables { - for _, m := range envVarRefPattern.FindAllStringSubmatch(ev.Value, -1) { + seen := make(map[string]struct{}) + var refs []string + for _, value := range env { + for _, m := range envVarRefPattern.FindAllStringSubmatch(value, -1) { if m[2] != "" { // Variable carries an explicit `:-fallback` default; the // deploy-time resolver honors it, so the user does not need @@ -649,34 +698,35 @@ func extractAgentYamlEnvRefs(projectPath, relativePath string) (refs, placeholde continue } name := m[1] - if _, ok := seenRef[name]; ok { + if _, ok := seen[name]; ok { continue } - seenRef[name] = struct{}{} + seen[name] = struct{}{} refs = append(refs, name) } - for _, m := range placeholderPattern.FindAllStringSubmatch(ev.Value, -1) { - name := m[1] - if _, ok := seenPh[name]; ok { - continue - } - seenPh[name] = struct{}{} - placeholders = append(placeholders, name) - } } - return refs, placeholders + return refs } +// isDeployed reports whether the named agent has a recorded deploy version +// in the active environment (AGENT__VERSION non-empty). +// +// TODO(unify true-up): under the unified design the deploy marker is +// per-agent. The exact env-var convention the new microsoft.foundry service +// target writes is owned by that rework (PR #8590 §2.6/§2.8) and is not yet +// pinned. We assume AGENT__VERSION here, mirroring the legacy +// per-service writer; verify and adjust against the service target once it +// lands. func isDeployed( ctx context.Context, src Source, - envName, serviceName string, + envName, agentName string, errs *[]error, ) bool { - if envName == "" || serviceName == "" { + if envName == "" || agentName == "" { return false } - key := fmt.Sprintf(agentVersionVarFormat, serviceKey(serviceName)) + key := fmt.Sprintf(agentVersionVarFormat, serviceKey(agentName)) value, err := src.EnvValue(ctx, envName, key) if err != nil { *errs = append(*errs, fmt.Errorf("read %s: %w", key, err)) @@ -685,7 +735,7 @@ func isDeployed( return value != "" } -// serviceKey converts a service name into the env-var key fragment used by +// serviceKey converts an agent name into the env-var key fragment used by // the deploy-time env-var writer in service_target_agent.go. It mirrors // AgentServiceTargetProvider.getServiceKey verbatim. func serviceKey(name string) string { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state_test.go index 4663ad937f2..d01b2f27758 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/state_test.go @@ -13,6 +13,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" ) // fakeSource is a hand-rolled Source for table-driven tests. @@ -40,6 +41,32 @@ func (f *fakeSource) EnvValue(_ context.Context, envName, key string) (string, e return f.values[envName+"/"+key], nil } +// foundrySvc builds a `host: microsoft.foundry` ServiceConfig whose +// AdditionalProperties carry the given top-level Foundry keys (deployments, +// connections, toolboxes, skills, routines, agents, endpoint). This mirrors +// how core azd forwards the unified service entry over gRPC. +func foundrySvc(t *testing.T, name string, props map[string]any) *azdext.ServiceConfig { + t.Helper() + s, err := structpb.NewStruct(props) + require.NoError(t, err) + return &azdext.ServiceConfig{Name: name, Host: agentHost, AdditionalProperties: s} +} + +// oneAgentSvc builds a Foundry service named `name` containing a single +// hosted agent named `agentName`, with the given project dir (omitted when +// empty) and env map (omitted when nil). +func oneAgentSvc(t *testing.T, name, agentName, project string, env map[string]any) *azdext.ServiceConfig { + t.Helper() + agent := map[string]any{"name": agentName, "kind": agentKindHosted} + if project != "" { + agent["project"] = project + } + if env != nil { + agent["env"] = env + } + return foundrySvc(t, name, map[string]any{"agents": []any{agent}}) +} + func TestAssembleState(t *testing.T) { t.Parallel() @@ -62,7 +89,7 @@ func TestAssembleState(t *testing.T) { errCount: 2, }, { - name: "endpoint set, no services: HasProjectEndpoint true", + name: "endpoint env var set, no services: HasProjectEndpoint true", src: &fakeSource{ envName: "dev", values: map[string]string{"dev/FOUNDRY_PROJECT_ENDPOINT": "https://x.services.ai.azure.com"}, @@ -77,6 +104,24 @@ func TestAssembleState(t *testing.T) { ) }, }, + { + name: "explicit endpoint field on service marks HasProjectEndpoint", + src: &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "proj": foundrySvc(t, "proj", map[string]any{ + "endpoint": "https://acct.services.ai.azure.com/api/projects/p", + "agents": []any{map[string]any{"name": "a1", "kind": "prompt"}}, + }), + }, + }, + values: map[string]string{}, + }, + assert: func(t *testing.T, state *State, _ []error) { + assert.True(t, state.HasProjectEndpoint, "endpoint: field should mark the project as present") + }, + }, { name: "Azure context vars set: MissingAzureContextVars stays empty", src: &fakeSource{ @@ -93,12 +138,12 @@ func TestAssembleState(t *testing.T) { }, }, { - name: "endpoint unset, one undeployed service", + name: "endpoint unset, one undeployed agent", src: &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "./src/echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "./src/echo", nil), }, }, values: map[string]string{}, @@ -107,20 +152,26 @@ func TestAssembleState(t *testing.T) { assert.False(t, state.HasProjectEndpoint) require.Len(t, state.Services, 1) assert.Equal(t, "echo", state.Services[0].Name) + assert.Equal(t, agentKindHosted, state.Services[0].Kind) assert.Equal(t, agentHost, state.Services[0].Host) + assert.Equal(t, "proj", state.Services[0].ServiceName) assert.Equal(t, "./src/echo", state.Services[0].RelativePath) assert.False(t, state.Services[0].IsDeployed) }, }, { - name: "multiple services: deployed flag follows AGENT__VERSION, alphabetical order", + name: "multiple agents: deployed flag follows AGENT__VERSION, alphabetical order", src: &fakeSource{ envName: "prod", project: &azdext.ProjectConfig{ Services: map[string]*azdext.ServiceConfig{ - "chat-bot": {Name: "chat-bot", Host: agentHost}, - "echo": {Name: "echo", Host: agentHost}, - "my service": {Name: "my service", Host: agentHost}, + "proj": foundrySvc(t, "proj", map[string]any{ + "agents": []any{ + map[string]any{"name": "chat-bot", "kind": "hosted"}, + map[string]any{"name": "echo", "kind": "hosted"}, + map[string]any{"name": "my service", "kind": "hosted"}, + }, + }), }, }, values: map[string]string{ @@ -140,12 +191,12 @@ func TestAssembleState(t *testing.T) { }, }, { - name: "non-agent services are filtered out", + name: "non-foundry services are filtered out", src: &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost}, + "proj": oneAgentSvc(t, "proj", "echo", "", nil), "web": {Name: "web", Host: "appservice"}, "worker": {Name: "worker", Host: "containerapp"}, }, @@ -166,7 +217,9 @@ func TestAssembleState(t *testing.T) { src: &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ - Services: map[string]*azdext.ServiceConfig{"echo": {Name: "echo", Host: agentHost}}, + Services: map[string]*azdext.ServiceConfig{ + "proj": oneAgentSvc(t, "proj", "echo", "", nil), + }, }, valueErr: errors.New("gRPC unavailable"), }, @@ -178,7 +231,7 @@ func TestAssembleState(t *testing.T) { }, // One error each for FOUNDRY_PROJECT_ENDPOINT, // AI_AGENT_PENDING_PROVISION, AZURE_SUBSCRIPTION_ID, - // AZURE_LOCATION + one per service lookup + // AZURE_LOCATION + one per agent lookup // (AGENT_ECHO_VERSION) = 5. errCount: 5, }, @@ -256,7 +309,7 @@ func TestAssembleState_NilServiceEntriesAreIgnored(t *testing.T) { envName: "dev", project: &azdext.ProjectConfig{ Services: map[string]*azdext.ServiceConfig{ - "good": {Name: "good", Host: agentHost}, + "good": oneAgentSvc(t, "good", "good-agent", "", nil), "nil": nil, }, }, @@ -265,15 +318,15 @@ func TestAssembleState_NilServiceEntriesAreIgnored(t *testing.T) { state, errs := assembleState(context.Background(), src) assert.Empty(t, errs) require.Len(t, state.Services, 1) - assert.Equal(t, "good", state.Services[0].Name) + assert.Equal(t, "good-agent", state.Services[0].Name) } func TestAgentHostConstant(t *testing.T) { t.Parallel() - // agentHost must remain in sync with cmd.AiAgentHost ("azure.ai.agent"). - // Pinning the literal here guards against accidental drift while the - // duplication exists; Phase 2's nextstep wiring should retire it. - assert.Equal(t, "azure.ai.agent", agentHost) + // agentHost must match the unified-design host kind. Pinning the + // literal guards against accidental drift while the duplication with + // the doctor package and cmd.AiAgentHost exists. + assert.Equal(t, "microsoft.foundry", agentHost) } func TestServiceKey(t *testing.T) { @@ -441,10 +494,6 @@ func TestAssembleState_WithLiveOpenAPIProbe_PrefersLiveOverCache(t *testing.T) { func TestAssembleState_WithLiveOpenAPIProbe_FallsBackToCacheOnError(t *testing.T) { t.Parallel() - // Live probe returns an error; the cache (when present and - // well-formed) must take over silently — the design budget for - // the live probe is 3 s and a failed fetch shouldn't deprive - // the user of the cached sample. projectRoot := t.TempDir() configDir := filepath.Join(projectRoot, ".azure", "dev") require.NoError(t, os.MkdirAll(configDir, 0o750)) @@ -479,10 +528,6 @@ func TestAssembleState_WithLiveOpenAPIProbe_FallsBackToCacheOnError(t *testing.T func TestAssembleState_WithLiveOpenAPIProbe_FallsBackToCacheOnEmptyBody(t *testing.T) { t.Parallel() - // Live probe returns nil bytes with no error (e.g., agent - // exposed /openapi.json but the body was empty after read). - // Treat identically to an error — empty body is unusable for - // example extraction and the cache must take over. projectRoot := t.TempDir() configDir := filepath.Join(projectRoot, ".azure", "dev") require.NoError(t, os.MkdirAll(configDir, 0o750)) @@ -515,9 +560,6 @@ func TestAssembleState_WithLiveOpenAPIProbe_FallsBackToCacheOnEmptyBody(t *testi func TestAssembleState_WithLiveOpenAPIProbe_LiveWorksEvenWithoutCacheProbe(t *testing.T) { t.Parallel() - // The live probe must not require WithOpenAPIProbe to be set — - // `run` may surface a payload from the freshly-started agent - // even when no prior `invoke` has populated the on-disk cache. projectRoot := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, ".azure", "dev"), 0o750)) @@ -543,8 +585,6 @@ func TestAssembleState_WithLiveOpenAPIProbe_LiveWorksEvenWithoutCacheProbe(t *te func TestAssembleState_WithLiveOpenAPIProbe_LiveFailureWithoutCacheLeavesUnset(t *testing.T) { t.Parallel() - // Live probe errors AND no cache present → resolver must fall - // back to the protocol-generic literal (HasOpenAPI=false). projectRoot := t.TempDir() require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, ".azure", "dev"), 0o750)) @@ -569,148 +609,51 @@ func TestAssembleState_WithLiveOpenAPIProbe_LiveFailureWithoutCacheLeavesUnset(t assert.Empty(t, state.OpenAPIPayload) } -func TestLoadServiceProtocol(t *testing.T) { +func TestPreferredProtocol(t *testing.T) { t.Parallel() tests := []struct { - name string - manifest string // raw agent.yaml content; empty string means "do not write the file" - manifestRel string // override relativePath in the call (for missing-dir cases) - want string + name string + protocols []foundryProtocol + want string }{ + {"single responses", []foundryProtocol{{Protocol: "responses"}}, ProtocolResponses}, + {"single invocations", []foundryProtocol{{Protocol: "invocations"}}, ProtocolInvocations}, { - name: "single responses protocol", - manifest: `kind: hostedAgent -protocols: - - protocol: responses - version: "1.0.0" -`, - want: ProtocolResponses, - }, - { - name: "single invocations protocol", - manifest: `kind: hostedAgent -protocols: - - protocol: invocations - version: "1.0.0" -`, - want: ProtocolInvocations, - }, - { - name: "responses wins when both declared", - manifest: `kind: hostedAgent -protocols: - - protocol: invocations - version: "1.0.0" - - protocol: responses - version: "1.0.0" -`, - want: ProtocolResponses, - }, - { - name: "empty protocols section", - manifest: `kind: hostedAgent -protocols: [] -`, - want: "", - }, - { - name: "unknown protocol value silently ignored", - manifest: `kind: hostedAgent -protocols: - - protocol: pigeon-mail - version: "1.0.0" -`, - want: "", - }, - { - name: "malformed yaml returns empty", - manifest: "this: is: not: valid: yaml: at: all: [", - want: "", - }, - { - name: "missing file returns empty", - manifestRel: "does-not-exist", - want: "", + "responses wins when both declared", + []foundryProtocol{{Protocol: "invocations"}, {Protocol: "responses"}}, + ProtocolResponses, }, + {"empty list", nil, ""}, + {"unknown value ignored", []foundryProtocol{{Protocol: "pigeon-mail"}}, ""}, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - projectRoot := t.TempDir() - relPath := "echo" - if tt.manifestRel != "" { - relPath = tt.manifestRel - } else { - svcDir := filepath.Join(projectRoot, relPath) - require.NoError(t, os.MkdirAll(svcDir, 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(svcDir, "agent.yaml"), - []byte(tt.manifest), - 0o600, - )) - } - got := loadServiceProtocol(projectRoot, relPath) - assert.Equal(t, tt.want, got) + assert.Equal(t, tt.want, preferredProtocol(tt.protocols)) }) } } -func TestLoadServiceProtocol_EmptyArgs(t *testing.T) { - t.Parallel() - - assert.Equal(t, "", loadServiceProtocol("", "echo")) -} - -func TestLoadServiceProtocol_RootRelativePath(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "agent.yaml"), - []byte("kind: hostedAgent\nprotocols:\n - protocol: invocations\n version: \"1.0.0\"\n"), - 0o600, - )) - - assert.Equal(t, ProtocolInvocations, loadServiceProtocol(projectRoot, "")) - assert.Equal(t, ProtocolInvocations, loadServiceProtocol(projectRoot, ".")) -} - -func TestLoadServiceProtocol_RejectsTraversal(t *testing.T) { +func TestAssembleState_PopulatesProtocolFromAgents(t *testing.T) { t.Parallel() - parent := t.TempDir() - projectRoot := filepath.Join(parent, "project") - outside := filepath.Join(parent, "outside") - require.NoError(t, os.MkdirAll(projectRoot, 0o750)) - require.NoError(t, os.MkdirAll(outside, 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(outside, "agent.yaml"), - []byte("kind: hostedAgent\nprotocols:\n - protocol: invocations\n version: \"1.0.0\"\n"), - 0o600, - )) - - assert.Equal(t, "", loadServiceProtocol(projectRoot, "../outside")) -} - -func TestAssembleState_PopulatesProtocolFromAgentYaml(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte("kind: hostedAgent\nprotocols:\n - protocol: invocations\n version: \"1.0.0\"\n"), - 0o600, - )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ - Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": foundrySvc(t, "proj", map[string]any{ + "agents": []any{ + map[string]any{ + "name": "echo", + "kind": "hosted", + "protocols": []any{ + map[string]any{"protocol": "invocations", "version": "1.0.0"}, + map[string]any{"protocol": "responses", "version": "1.0.0"}, + }, + }, + }, + }), }, }, } @@ -718,269 +661,39 @@ func TestAssembleState_PopulatesProtocolFromAgentYaml(t *testing.T) { state, errs := assembleState(context.Background(), src) require.Empty(t, errs) require.Len(t, state.Services, 1) - assert.Equal(t, ProtocolInvocations, state.Services[0].Protocol) + assert.Equal(t, ProtocolResponses, state.Services[0].Protocol) } -func TestExtractAgentYamlEnvRefs(t *testing.T) { +func TestExtractEnvRefs(t *testing.T) { t.Parallel() tests := []struct { - name string - manifest string - wantRefs []string - wantPlaceholders []string + name string + env map[string]string + want []string }{ + {"nil env", nil, nil}, + {"no refs", map[string]string{"A": "static"}, nil}, + {"single bare ref", map[string]string{"A": "${MY_KEY}"}, []string{"MY_KEY"}}, + {"defaulted ref skipped", map[string]string{"A": "${MY_KEY:-fallback}"}, nil}, { - name: "single bare reference", - manifest: `kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} -`, - wantRefs: []string{"FOUNDRY_PROJECT_ENDPOINT"}, - }, - { - name: "reference with default tail is skipped", - manifest: `kind: hostedAgent -environment_variables: - - name: MODEL - value: ${AZURE_AI_MODEL_DEPLOYMENT_NAME:-gpt-4o-mini} -`, - wantRefs: nil, - }, - { - name: "bare ref alongside defaulted ref returns only the bare one", - manifest: `kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} - - name: MODEL - value: ${AZURE_AI_MODEL_DEPLOYMENT_NAME:-gpt-4o-mini} -`, - wantRefs: []string{"FOUNDRY_PROJECT_ENDPOINT"}, - }, - { - name: "multiple references in one value", - manifest: `kind: hostedAgent -environment_variables: - - name: CONN - value: postgresql://${DB_HOST}:5432/${DB_NAME} -`, - wantRefs: []string{"DB_HOST", "DB_NAME"}, - }, - { - name: "duplicate references deduplicated by first appearance", - manifest: `kind: hostedAgent -environment_variables: - - name: A - value: ${X}-${X} - - name: B - value: ${X} -`, - wantRefs: []string{"X"}, - }, - { - name: "no environment_variables block", - manifest: `kind: hostedAgent -protocols: - - protocol: responses - version: "1.0.0" -`, - wantRefs: nil, - }, - { - name: "literal value with no ${} reference", - manifest: `kind: hostedAgent -environment_variables: - - name: STATIC - value: hardcoded -`, - wantRefs: nil, - }, - { - name: "malformed yaml returns nil", - manifest: "this: is: not: valid: yaml: at: all: [", - wantRefs: nil, - }, - { - name: "mustache placeholder surfaced separately", - manifest: `kind: hostedAgent -environment_variables: - - name: TOOLBOX_ENDPOINT - value: '{{TOOLBOX_ENDPOINT}}' -`, - wantPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - }, - { - name: "mustache placeholder with internal whitespace", - manifest: `kind: hostedAgent -environment_variables: - - name: KEY - value: '{{ MY_KEY }}' -`, - wantPlaceholders: []string{"MY_KEY"}, - }, - { - name: "duplicate placeholders deduplicated", - manifest: `kind: hostedAgent -environment_variables: - - name: A - value: '{{X}}-{{X}}' - - name: B - value: '{{X}}' -`, - wantPlaceholders: []string{"X"}, - }, - { - name: "ref and placeholder coexist in same manifest", - manifest: `kind: hostedAgent -environment_variables: - - name: TOOLBOX_ENDPOINT - value: '{{TOOLBOX_ENDPOINT}}' - - name: MCP_ENDPOINT - value: ${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT} -`, - wantRefs: []string{"TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT"}, - wantPlaceholders: []string{"TOOLBOX_ENDPOINT"}, - }, - { - // Manifest parameter names are not constrained to env-var - // identifier shape — parameters.go:injectParameterValues - // substitutes the raw YAML key without validating it. - // A surviving `{{toolbox-endpoint}}` (hyphen) must therefore - // still be flagged or the user gets no Next: hint. - name: "mustache placeholder with hyphen in name", - manifest: `kind: hostedAgent -environment_variables: - - name: TOOLBOX_ENDPOINT - value: '{{toolbox-endpoint}}' -`, - wantPlaceholders: []string{"toolbox-endpoint"}, - }, - { - name: "mustache placeholder with dot in name", - manifest: `kind: hostedAgent -environment_variables: - - name: COMPONENT - value: '{{my.component.id}}' -`, - wantPlaceholders: []string{"my.component.id"}, - }, - { - // Empty placeholder body must not be flagged — it cannot - // correspond to a manifest parameter and is more likely - // stray literal text. - name: "empty mustache braces are ignored", - manifest: `kind: hostedAgent -environment_variables: - - name: NOISE - value: 'preamble {{}} suffix' -`, - wantPlaceholders: nil, - }, - { - // Whitespace-only placeholder body is similarly garbage — - // must not be flagged. - name: "whitespace-only mustache braces are ignored", - manifest: `kind: hostedAgent -environment_variables: - - name: NOISE - value: 'preamble {{ }} suffix' -`, - wantPlaceholders: nil, + "foundry server-side ref ignored", + map[string]string{"A": "${{connections.x.credentials.key}}"}, + nil, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - projectRoot := t.TempDir() - svcDir := filepath.Join(projectRoot, "echo") - require.NoError(t, os.MkdirAll(svcDir, 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(svcDir, "agent.yaml"), - []byte(tt.manifest), - 0o600, - )) - gotRefs, gotPlaceholders := extractAgentYamlEnvRefs(projectRoot, "echo") - assert.Equal(t, tt.wantRefs, gotRefs, "refs") - assert.Equal(t, tt.wantPlaceholders, gotPlaceholders, "placeholders") + assert.ElementsMatch(t, tt.want, extractEnvRefs(tt.env)) }) } } -func TestExtractAgentYamlEnvRefs_MissingFileOrArgs(t *testing.T) { - t.Parallel() - - for _, args := range [][2]string{ - {"", "echo"}, - {t.TempDir(), ""}, - {t.TempDir(), "missing"}, - } { - refs, placeholders := extractAgentYamlEnvRefs(args[0], args[1]) - assert.Nil(t, refs) - assert.Nil(t, placeholders) - } -} - -func TestExtractAgentYamlEnvRefs_RejectsTraversal(t *testing.T) { - t.Parallel() - - parent := t.TempDir() - projectRoot := filepath.Join(parent, "project") - outside := filepath.Join(parent, "outside") - require.NoError(t, os.MkdirAll(projectRoot, 0o750)) - require.NoError(t, os.MkdirAll(outside, 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(outside, "agent.yaml"), - []byte("kind: hostedAgent\nenvironment_variables:\n - name: SECRET\n value: ${OUTSIDE_SECRET}\n"), - 0o600, - )) - - refs, placeholders := extractAgentYamlEnvRefs(projectRoot, "../outside") - - assert.Nil(t, refs) - assert.Nil(t, placeholders) -} - -func TestExtractAgentYamlEnvRefs_RootRelativePath(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "agent.yaml"), - []byte("kind: hostedAgent\nenvironment_variables:\n - name: SECRET\n value: ${ROOT_SECRET}\n"), - 0o600, - )) - - for _, rel := range []string{"", "."} { - refs, placeholders := extractAgentYamlEnvRefs(projectRoot, rel) - - assert.Equal(t, []string{"ROOT_SECRET"}, refs) - assert.Nil(t, placeholders) - } -} - func TestAssembleState_PopulatesMissingVars(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} - - name: MODEL - value: ${AZURE_AI_MODEL_DEPLOYMENT_NAME} - - name: KEY - value: ${MY_API_KEY} - - name: STATIC - value: hardcoded -`), - 0o600, - )) // infra/main.bicep declares both AZURE_* names as outputs, so they // route to MissingInfraVars when unset. MY_API_KEY has no Bicep // output and routes to MissingManualVars. @@ -998,7 +711,12 @@ output AZURE_AI_MODEL_DEPLOYMENT_NAME string = '' project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}", + "MODEL": "${AZURE_AI_MODEL_DEPLOYMENT_NAME}", + "KEY": "${MY_API_KEY}", + "STATIC": "hardcoded", + }), }, }, values: map[string]string{ @@ -1013,40 +731,32 @@ output AZURE_AI_MODEL_DEPLOYMENT_NAME string = '' assert.Equal(t, []string{"MY_API_KEY"}, state.MissingManualVars) } -func TestAssembleState_MissingVarsDedupedAcrossServices(t *testing.T) { +func TestAssembleState_MissingVarsDedupedAcrossAgents(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - manifest := []byte(`kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} - - name: KEY - value: ${MY_API_KEY} -`) - for _, rel := range []string{"echo", "ping"} { - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, rel), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, rel, "agent.yaml"), - manifest, - 0o600, - )) - } require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "infra"), 0o750)) require.NoError(t, os.WriteFile( filepath.Join(projectRoot, "infra", "main.bicep"), - []byte(`output FOUNDRY_PROJECT_ENDPOINT string = '' -`), + []byte("output FOUNDRY_PROJECT_ENDPOINT string = ''\n"), 0o600, )) + env := map[string]any{ + "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}", + "KEY": "${MY_API_KEY}", + } src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, - "ping": {Name: "ping", Host: agentHost, RelativePath: "ping"}, + "proj": foundrySvc(t, "proj", map[string]any{ + "agents": []any{ + map[string]any{"name": "echo", "kind": "hosted", "env": env}, + map[string]any{"name": "ping", "kind": "hosted", "env": env}, + }, + }), }, }, } @@ -1061,25 +771,15 @@ func TestAssembleState_AllVarsSetLeavesMissingEmpty(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} - - name: KEY - value: ${MY_API_KEY} -`), - 0o600, - )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}", + "KEY": "${MY_API_KEY}", + }), }, }, values: map[string]string{ @@ -1098,28 +798,10 @@ func TestAssembleState_DefaultedRefsAreExcludedFromMissingVars(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - // Both refs use POSIX ${VAR:-default} syntax; the deploy-time expander - // honors the default so neither variable is required. The bare-form - // ENDPOINT ref is unset and IS required, so it still surfaces. - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} - - name: MODEL - value: ${AZURE_AI_MODEL_DEPLOYMENT_NAME:-gpt-4o-mini} - - name: KEY - value: ${MY_API_KEY:-dev-fallback} -`), - 0o600, - )) require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "infra"), 0o750)) require.NoError(t, os.WriteFile( filepath.Join(projectRoot, "infra", "main.bicep"), - []byte(`output FOUNDRY_PROJECT_ENDPOINT string = '' -`), + []byte("output FOUNDRY_PROJECT_ENDPOINT string = ''\n"), 0o600, )) @@ -1128,11 +810,16 @@ environment_variables: project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + // Both MODEL and KEY use ${VAR:-default}; the deploy-time + // expander honors the default, so neither is required. The + // bare-form ENDPOINT ref is unset and IS required. + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}", + "MODEL": "${AZURE_AI_MODEL_DEPLOYMENT_NAME:-gpt-4o-mini}", + "KEY": "${MY_API_KEY:-dev-fallback}", + }), }, }, - // Intentionally leave AZURE_AI_MODEL_DEPLOYMENT_NAME and MY_API_KEY - // unset; defaulted refs must not surface them as missing. values: map[string]string{}, } @@ -1146,23 +833,14 @@ func TestAssembleState_MissingVarTransportErrorSurfaced(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: KEY - value: ${MY_API_KEY} -`), - 0o600, - )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "KEY": "${MY_API_KEY}", + }), }, }, valueErr: errors.New("gRPC unavailable"), @@ -1176,86 +854,33 @@ environment_variables: assert.Empty(t, state.MissingManualVars) } -func TestAssembleState_PopulatesUnresolvedPlaceholders(t *testing.T) { - t.Parallel() - - // Reproduces the toolbox-sample bug: agent.manifest.yaml processing - // leaves a {{NAME}} placeholder behind in agent.yaml, while a separate - // env var ref is also unset. The resolver should see both. - projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: TOOLBOX_ENDPOINT - value: '{{TOOLBOX_ENDPOINT}}' - - name: MCP_ENDPOINT - value: ${TOOLBOX_MCP_ENDPOINT} -`), - 0o600, - )) - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, - }, - }, - } - - state, errs := assembleState(context.Background(), src) - require.Empty(t, errs) - assert.Empty(t, state.MissingInfraVars) - assert.Equal(t, []string{"TOOLBOX_MCP_ENDPOINT"}, state.MissingManualVars) - assert.Equal(t, []string{"TOOLBOX_ENDPOINT"}, state.UnresolvedPlaceholders) -} - // TestAssembleState_PartitionsToolboxEndpointVars locks the partition -// behavior added for the toolbox-sample post-init UX: when a service -// has a manifest-declared toolbox AND agent.yaml references the -// canonical TOOLBOX__MCP_ENDPOINT env var, the missing-var -// classifier must route the entry into MissingToolboxEndpoints +// behavior: when a Foundry service declares a toolbox AND an agent's env +// references the canonical TOOLBOX__MCP_ENDPOINT env var, the +// missing-var classifier routes the entry into MissingToolboxEndpoints // (provision-managed) rather than MissingManualVars (operator-supplied). -// Non-toolbox manual vars in the same agent.yaml must still appear in -// MissingManualVars. +// Non-toolbox manual vars must still appear in MissingManualVars. func TestAssembleState_PartitionsToolboxEndpointVars(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - // agent.manifest.yaml declares the toolbox by name; envkey derives - // "TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT" from "web-search-tools", - // matching the ${...} ref in agent.yaml below. - writeManifest(t, projectRoot, "echo", ` -template: - kind: containerAgent - name: hello -resources: - - name: web-search-tools - kind: toolbox - tools: - - id: tool-1 -`) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: MCP_ENDPOINT - value: ${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT} - - name: API_KEY - value: ${MY_API_KEY} -`), - 0o600, - )) - + // The toolbox "web-search-tools" derives the canonical env var + // "TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT", matching the ${...} ref in + // the agent env below. src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": foundrySvc(t, "proj", map[string]any{ + "toolboxes": []any{map[string]any{"name": "web-search-tools"}}, + "agents": []any{ + map[string]any{"name": "echo", "kind": "hosted", "env": map[string]any{ + "MCP_ENDPOINT": "${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT}", + "API_KEY": "${MY_API_KEY}", + }}, + }, + }), }, }, } @@ -1263,41 +888,30 @@ environment_variables: state, errs := assembleState(context.Background(), src) require.Empty(t, errs) assert.Empty(t, state.MissingInfraVars) - // Toolbox endpoint var moved into the dedicated bucket; the - // generic manual-var bucket only carries the truly user-supplied - // API key. + // Toolbox endpoint var moved into the dedicated bucket; the generic + // manual-var bucket only carries the truly user-supplied API key. assert.Equal(t, []string{"MY_API_KEY"}, state.MissingManualVars) require.Len(t, state.MissingToolboxEndpoints, 1) assert.Equal(t, "web-search-tools", state.MissingToolboxEndpoints[0].Name) - assert.Equal(t, "echo", state.MissingToolboxEndpoints[0].ServiceName) + assert.Equal(t, "proj", state.MissingToolboxEndpoints[0].ServiceName) } -// TestAssembleState_ToolboxEndpointWithoutManifestStaysManual locks -// the partition's guard: a TOOLBOX_*_MCP_ENDPOINT-shaped variable -// whose name does NOT match a manifest-declared toolbox is treated -// as a generic user variable and stays in MissingManualVars. The -// partition is a no-op when no manifest toolbox claims the key. -func TestAssembleState_ToolboxEndpointWithoutManifestStaysManual(t *testing.T) { +// TestAssembleState_ToolboxEndpointWithoutDeclarationStaysManual locks the +// partition's guard: a TOOLBOX_*_MCP_ENDPOINT-shaped variable whose name +// does NOT match a declared toolbox is treated as a generic user variable +// and stays in MissingManualVars. +func TestAssembleState_ToolboxEndpointWithoutDeclarationStaysManual(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: MCP_ENDPOINT - value: ${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT} -`), - 0o600, - )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "MCP_ENDPOINT": "${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT}", + }), }, }, } @@ -1308,64 +922,10 @@ environment_variables: assert.Empty(t, state.MissingToolboxEndpoints) } -func TestAssembleState_PlaceholdersDedupedAcrossServices(t *testing.T) { - t.Parallel() - - projectRoot := t.TempDir() - manifest := []byte(`kind: hostedAgent -environment_variables: - - name: A - value: '{{SHARED_PLACEHOLDER}}' -`) - for _, rel := range []string{"echo", "ping"} { - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, rel), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, rel, "agent.yaml"), - manifest, - 0o600, - )) - } - - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, - "ping": {Name: "ping", Host: agentHost, RelativePath: "ping"}, - }, - }, - } - - state, errs := assembleState(context.Background(), src) - require.Empty(t, errs) - assert.Equal(t, []string{"SHARED_PLACEHOLDER"}, state.UnresolvedPlaceholders) -} - -// TestAssembleState_NonAzurePrefixBicepOutputIsInfra is the B1 fix proof. -// It locks issue #7975 State Inputs line 74 ("HasUnresolvedInfraVars = -// agent.yaml ${VAR} refs that map to known Bicep outputs are unset in -// azd env"). Pre-C1, the resolver split on the AZURE_ prefix; this -// test guarantees the new classifier is set-membership based and -// correctly routes a non-AZURE_ Bicep output to MissingInfraVars. func TestAssembleState_NonAzurePrefixBicepOutputIsInfra(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: TOOLBOX - value: ${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT} - - name: BING - value: ${BING_GROUNDING_CONNECTION_ID} - - name: KEY - value: ${MY_API_KEY} -`), - 0o600, - )) require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "infra"), 0o750)) require.NoError(t, os.WriteFile( filepath.Join(projectRoot, "infra", "main.bicep"), @@ -1380,7 +940,11 @@ output BING_GROUNDING_CONNECTION_ID string = '' project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "TOOLBOX": "${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT}", + "BING": "${BING_GROUNDING_CONNECTION_ID}", + "KEY": "${MY_API_KEY}", + }), }, }, } @@ -1395,35 +959,19 @@ output BING_GROUNDING_CONNECTION_ID string = '' assert.Equal(t, []string{"MY_API_KEY"}, state.MissingManualVars) } -// TestAssembleState_NoBicepFileEverythingManual locks the conservative -// fallback: when infra/main.bicep is missing, every unset ref lands in -// MissingManualVars. Notably this includes AZURE_*-prefixed names — -// without the prefix shortcut, AZURE_ has no special meaning anymore. func TestAssembleState_NoBicepFileEverythingManual(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: ENDPOINT - value: ${FOUNDRY_PROJECT_ENDPOINT} - - name: TOOLBOX - value: ${TOOLBOX_MCP_ENDPOINT} - - name: KEY - value: ${MY_API_KEY} -`), - 0o600, - )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": oneAgentSvc(t, "proj", "echo", "", map[string]any{ + "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}", + "KEY": "${MY_API_KEY}", + }), }, }, } @@ -1431,96 +979,51 @@ environment_variables: state, errs := assembleState(context.Background(), src) require.Empty(t, errs) assert.Empty(t, state.MissingInfraVars) - assert.Equal( - t, - []string{"FOUNDRY_PROJECT_ENDPOINT", "MY_API_KEY", "TOOLBOX_MCP_ENDPOINT"}, - state.MissingManualVars, - ) + assert.Equal(t, []string{"FOUNDRY_PROJECT_ENDPOINT", "MY_API_KEY"}, state.MissingManualVars) } -// TestAssembleState_DeclaredAndSetBicepOutputNotSurfaced locks the -// sanity case: a ref that maps to a Bicep output AND is set in the -// current env is not missing from either bucket. -func TestAssembleState_DeclaredAndSetBicepOutputNotSurfaced(t *testing.T) { +// TestAssembleState_AggregatesResources verifies collectFoundry surfaces +// model deployments, toolboxes, and connections (with detail strings) and +// the matching Has* flags from the unified service entry. +func TestAssembleState_AggregatesResources(t *testing.T) { t.Parallel() - projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: TOOLBOX - value: ${TOOLBOX_MCP_ENDPOINT} -`), - 0o600, - )) - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "infra"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "infra", "main.bicep"), - []byte(`output TOOLBOX_MCP_ENDPOINT string = '' -`), - 0o600, - )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ - Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "proj": foundrySvc(t, "proj", map[string]any{ + "deployments": []any{ + map[string]any{ + "name": "gpt-4.1-mini", + "model": map[string]any{"format": "OpenAI", "name": "gpt-4.1-mini"}, + }, + }, + "toolboxes": []any{map[string]any{"name": "support-toolbox"}}, + "connections": []any{ + map[string]any{"name": "search-conn", "category": "CognitiveSearch", "target": "https://x"}, + }, + "agents": []any{map[string]any{"name": "a1", "kind": "prompt"}}, + }), }, }, - values: map[string]string{ - "dev/TOOLBOX_MCP_ENDPOINT": "https://mcp.example/x", - }, } state, errs := assembleState(context.Background(), src) require.Empty(t, errs) - assert.Empty(t, state.MissingInfraVars) - assert.Empty(t, state.MissingManualVars) -} -// TestAssembleState_UndeclaredRefIsManualEvenWithBicepFile locks the -// other half of set-membership classification: when infra/main.bicep -// exists but does NOT declare a ref'd var, the var lands in -// MissingManualVars (not MissingInfraVars). -func TestAssembleState_UndeclaredRefIsManualEvenWithBicepFile(t *testing.T) { - t.Parallel() + require.True(t, state.HasModels) + require.Len(t, state.ModelRefs, 1) + assert.Equal(t, "gpt-4.1-mini", state.ModelRefs[0].Name) + assert.Equal(t, "OpenAI/gpt-4.1-mini", state.ModelRefs[0].Detail) + assert.Equal(t, "proj", state.ModelRefs[0].ServiceName) - projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "echo", "agent.yaml"), - []byte(`kind: hostedAgent -environment_variables: - - name: KEY - value: ${MY_API_KEY} -`), - 0o600, - )) - // Bicep file exists but doesn't declare MY_API_KEY → manual. - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "infra"), 0o750)) - require.NoError(t, os.WriteFile( - filepath.Join(projectRoot, "infra", "main.bicep"), - []byte(`output FOUNDRY_PROJECT_ENDPOINT string = '' -`), - 0o600, - )) + require.True(t, state.HasToolboxes) + require.Len(t, state.Toolboxes, 1) + assert.Equal(t, "support-toolbox", state.Toolboxes[0].Name) - src := &fakeSource{ - envName: "dev", - project: &azdext.ProjectConfig{ - Path: projectRoot, - Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, - }, - }, - } - - state, errs := assembleState(context.Background(), src) - require.Empty(t, errs) - assert.Empty(t, state.MissingInfraVars) - assert.Equal(t, []string{"MY_API_KEY"}, state.MissingManualVars) + require.True(t, state.HasConnections) + require.Len(t, state.Connections, 1) + assert.Equal(t, "search-conn", state.Connections[0].Name) + assert.Equal(t, "CognitiveSearch | https://x", state.Connections[0].Detail) } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/types.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/types.go index 52c91382201..b9d8f5c515a 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/types.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/types.go @@ -109,18 +109,12 @@ type State struct { // state.Toolboxes / state.ModelRefs / state.Connections. MissingToolboxEndpoints []ResourceRef - // UnresolvedPlaceholders names {{NAME}} Mustache-style placeholders - // still present (literally) inside agent.yaml's environment_variables - // values. These are left over from init's manifest processing when - // agent.manifest.yaml declares a placeholder without a matching - // parameter (or the user skipped the prompt). Unlike Missing*Vars, - // these cannot be supplied via `azd env set` — the literal `{{X}}` - // would still be in agent.yaml at deploy time. The resolver surfaces - // a distinct "edit agent.yaml" suggestion for each. - UnresolvedPlaceholders []string - - // Services is the per-service snapshot derived from azure.yaml plus - // the azd environment (for IsDeployed). + // Services is the per-AGENT snapshot derived from azure.yaml plus the + // azd environment (for IsDeployed). Under the unified design a single + // `host: microsoft.foundry` service entry declares N agents; each + // agent becomes one ServiceState entry here (the unit the resolvers + // show and invoke). See ServiceState for the per-agent fields and the + // owning-service back-reference. Services []ServiceState // AgentStatus is the remote agent version status as reported by the @@ -144,58 +138,57 @@ type State struct { // prepend a `cd ` suggestion to the Next: block. CreatedFolderDisplay string - // HasModels, HasToolboxes, HasConnections are aggregate flags - // derived from each azure.ai.agent service's agent.manifest.yaml - // (when present). They are true when at least one resource of the - // matching kind is declared across all services. Doctor checks that - // only make sense in the presence of these resources gate-skip - // themselves on the matching Has* flag; resolvers can use them to - // tailor remediation suggestions. + // HasModels, HasToolboxes, HasConnections are aggregate flags derived + // from each `host: microsoft.foundry` service's top-level + // `deployments` / `toolboxes` / `connections` declarations in + // azure.yaml. They are true when at least one resource of the + // matching kind is declared across all Foundry services. Doctor + // checks that only make sense in the presence of these resources + // gate-skip themselves on the matching Has* flag; resolvers can use + // them to tailor remediation suggestions. // - // All three flags are false when the manifest file is missing, - // malformed, or declares no resources — the walker is deliberately - // silent on those failure modes so a missing/in-flight manifest - // never blocks the rest of state assembly. + // All three flags are false when no Foundry service declares the + // matching resource kind. HasModels bool HasToolboxes bool HasConnections bool // ModelRefs, Toolboxes, Connections list every resource of the - // matching kind found across all services' agent.manifest.yaml - // files. Entries are sorted by Name (ties broken by ServiceName) - // and deduplicated on (ServiceName, Name) so callers can render - // them deterministically. The slices are nil when the matching - // Has* flag is false. + // matching kind found across all Foundry services' azure.yaml + // declarations (`deployments` / `toolboxes` / `connections`). + // Entries are sorted by Name (ties broken by ServiceName) and + // deduplicated on (ServiceName, Name) so callers can render them + // deterministically. The slices are nil when the matching Has* flag + // is false. ModelRefs []ResourceRef Toolboxes []ResourceRef Connections []ResourceRef } -// ResourceRef is a slim summary of a manifest resource that the -// nextstep package surfaces to doctor checks and resolvers. The -// shape intentionally elides agent_yaml.ModelResource / -// ToolboxResource / ConnectionResource details that doctor checks -// don't consume today — keeping the surface small so future -// manifest schema changes don't ripple through the resolver / doctor -// boundary. Add fields here only when a doctor check or resolver -// branch needs them. +// ResourceRef is a slim summary of a Foundry resource (model deployment, +// toolbox, or connection) that the nextstep package surfaces to doctor +// checks and resolvers. The shape intentionally elides the full +// deployment/toolbox/connection details that doctor checks don't consume +// today — keeping the surface small so future schema changes don't ripple +// through the resolver / doctor boundary. Add fields here only when a +// doctor check or resolver branch needs them. type ResourceRef struct { - // Name is the resource's manifest-declared name (the `name:` - // field on the manifest's `resources[]` entry). Doctor checks - // match by this name when looking up Foundry deployments / - // connections / toolboxes. + // Name is the resource's declared name (the `name:` field on the + // `deployments[]` / `toolboxes[]` / `connections[]` entry in the + // Foundry service). Doctor checks match by this name when looking up + // Foundry deployments / connections / toolboxes. Name string - // ServiceName is the azd service that declared the resource (the - // service entry under `services:` in azure.yaml whose - // agent.manifest.yaml contains this entry). When the same logical - // resource is declared by multiple services they appear as - // separate entries — doctor checks key on (ServiceName, Name) so - // per-service failures are surfaced individually. + // ServiceName is the `host: microsoft.foundry` service entry (under + // `services:` in azure.yaml) that declared the resource. When the + // same logical resource is declared by multiple Foundry services + // they appear as separate entries — doctor checks key on + // (ServiceName, Name) so per-service failures are surfaced + // individually. ServiceName string // Detail carries a kind-specific identifier: - // - models: ModelResource.Id (e.g., "azureml://...gpt-4o...") + // - models: / (e.g., "OpenAI/gpt-4.1-mini") // - connections: | // - toolboxes: empty (no identifier beyond Name today) // Doctor remediation messages render Detail verbatim, so changes @@ -203,16 +196,37 @@ type ResourceRef struct { Detail string } -// ServiceState mirrors one entry from the project's services map, plus a -// deployment marker derived from azd environment variables. IsDeployed is -// true when AGENT__VERSION is non-empty in the active environment, -// where is the service name upper-cased with hyphens replaced by -// underscores — the convention used by the deploy-time env-var writer in -// project/service_target_agent.go. +// ServiceState is the per-AGENT snapshot the resolvers operate on. Under +// the unified design each `host: microsoft.foundry` service declares N +// agents; one ServiceState is produced per agent (the unit the resolvers +// show and invoke), with ServiceName back-referencing the owning Foundry +// service entry. +// +// IsDeployed is true when AGENT__VERSION is non-empty in the active +// environment, where is the agent name upper-cased with spaces and +// hyphens replaced by underscores — the convention used by the +// deploy-time env-var writer in project/service_target_agent.go. type ServiceState struct { - Name string - Host string - Protocol string + // Name is the agent name (used in `azd ai agent show/invoke `). + Name string + // Kind is the agent kind: "hosted" or "prompt". + Kind string + // Host is the owning Foundry service host ("microsoft.foundry"). + Host string + // ServiceName is the owning `host: microsoft.foundry` service entry + // name in azure.yaml. + ServiceName string + // Protocol is the agent's preferred protocol ("responses" or + // "invocations"); best-effort, empty when undeclared. + Protocol string + // RelativePath is the agent's source/project directory relative to + // the project root (the agent's `project:` field), used for README + // lookups. Empty for prompt agents (no code). RelativePath string - IsDeployed bool + // IsDeployed reports whether AGENT__VERSION is set for this agent. + IsDeployed bool + // Env is the agent's declared environment map (`agents[].env` in + // azure.yaml). Values may contain ${VAR} / ${{...}} references + // verbatim; missing-var detection scans these. + Env map[string]string }