diff --git a/cli/azd/.vscode/cspell.yaml b/cli/azd/.vscode/cspell.yaml index 24b626dd6f8..3f6866c6267 100644 --- a/cli/azd/.vscode/cspell.yaml +++ b/cli/azd/.vscode/cspell.yaml @@ -82,6 +82,7 @@ words: - protoimpl - protojson - protoreflect + - projectconfig - SNAPPROCESS - structpb - subtest 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..0783a59a581 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,8 +37,7 @@ 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 +// C15). For each connection declared 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 manifest-declared connection has a corresponding entry; @@ -75,7 +74,7 @@ type foundryConnectionsProbeFn func( func newCheckConnections(deps Dependencies) Check { return Check{ ID: "remote.connections", - Name: "Manifest connections exist on Foundry project", + Name: "Configured connections exist on Foundry project", Remote: true, Fn: func(ctx context.Context, _ Options, prior []Result) Result { if deps.AzdClient == nil { @@ -132,10 +131,28 @@ func newCheckConnections(deps Dependencies) Check { Suggestion: "Re-run `azd ai agent doctor`; the state assembly returned nil unexpectedly.", } } + if len(state.ConnectionLoadErrors) > 0 { + return Result{ + Status: StatusFail, + Message: fmt.Sprintf( + "could not load connection configuration: %s", + strings.Join( + state.ConnectionLoadErrors, + "; ", + ), + ), + Suggestion: "Fix the connection services in " + + "azure.yaml or the agent manifest, then retry.", + Details: map[string]any{ + "loadErrors": state.ConnectionLoadErrors, + }, + } + } if !state.HasConnections { return Result{ - Status: StatusSkip, - Message: "skipped: no connection resources declared in any service's agent.manifest.yaml.", + Status: StatusSkip, + Message: "skipped: no connection resources " + + "declared in azure.yaml.", } } @@ -285,14 +302,16 @@ func classifyConnections( } } + suggestion := "Run `azd provision` to create the missing " + + "connection(s), or update their azure.yaml service names." + 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 " + - "match connections that already exist on the Foundry project.", + Suggestion: suggestion, Details: map[string]any{ "missingConnections": missing, "matchedCount": matched, diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections_test.go index 834ade78b00..1b5e77fb0bb 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_connections_test.go @@ -228,6 +228,53 @@ func TestCheckConnections_FailsWithMissing(t *testing.T) { require.EqualValues(t, 1, res.Details["matchedCount"]) } +func TestCheckConnections_SplitResourceUsesProvision(t *testing.T) { + t.Parallel() + state := &nextstep.State{ + HasConnections: true, + Connections: []nextstep.ResourceRef{ + { + Name: "search-conn", + ServiceName: "search-conn", + }, + }, + } + deps := Dependencies{ + assembleState: fixedAssembler(state), + readProjectResourceIDFn: fixedProjectIDReader(validProjectResourceID, nil), + probeFoundryConnections: fixedConnectionsProbe(nil, nil, nil), + } + + res := runConnectionsCheck( + t, + deps, + healthyConnectionsPrior(), + ) + + require.Equal(t, StatusFail, res.Status) + require.Contains(t, res.Suggestion, "azd provision") + require.NotContains(t, res.Suggestion, "azd deploy") +} + +func TestCheckConnections_FailsOnIncompleteState(t *testing.T) { + t.Parallel() + deps := Dependencies{ + assembleState: fixedAssembler(&nextstep.State{ + ConnectionLoadErrors: []string{"missing connection ref"}, + }), + } + + res := runConnectionsCheck( + t, + deps, + healthyConnectionsPrior(), + ) + + require.Equal(t, StatusFail, res.Status) + require.Contains(t, res.Message, "missing connection ref") + require.Contains(t, res.Suggestion, "azure.yaml") +} + func TestCheckConnections_FailsWhenAllMissing(t *testing.T) { t.Parallel() state := &nextstep.State{ 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 27e89140d7a..c39992246d0 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 @@ -155,13 +155,8 @@ type Dependencies struct { ) ([]string, error) } -// 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). +// NewLocalChecks returns local doctor checks in execution order. +// Resource configuration checks run before checks that consume them. func NewLocalChecks(deps Dependencies) []Check { return []Check{ newCheckGRPCAndVersion(deps), @@ -170,6 +165,7 @@ func NewLocalChecks(deps Dependencies) []Check { newCheckAgentServiceDetected(deps), newCheckProjectEndpointSet(deps), newCheckAgentYAMLValid(deps), + newCheckModels(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..1d23fd06fd0 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, 9) want := []struct { id string @@ -455,9 +455,10 @@ 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.agent-yaml-valid", "agent definition valid (per service)", false}, + {"local.models", "azure.yaml model configuration valid", false}, {"local.manual-env-vars", "manual env vars set", false}, - {"local.toolboxes", "Manifest toolboxes have endpoint env vars set", false}, + {"local.toolboxes", "azure.yaml 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..674ddf9f887 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 @@ -17,13 +17,13 @@ import ( // newCheckManualEnvVars produces Check `local.manual-env-vars` — the // "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 +// "Manual" env vars use `${...}` in agent configuration and 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}` +// configuration. Otherwise the agent sees the literal `${KEY}` // string and almost certainly fails on first use. // // The classification of "manual" vs "infra" lives in nextstep's @@ -42,7 +42,7 @@ 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 +// - `local.agent-yaml-valid` failed or was skipped. // 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 @@ -74,15 +74,20 @@ func newCheckManualEnvVars(deps Dependencies) Check { 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"} + return Result{ + Status: StatusSkip, + Message: "skipped: agent definition check " + + "failed or was skipped", + } } if priorBlocked(prior, "local.environment-selected") { // Without an azd env, AssembleState's detectMissingVars // block is skipped (state.go:258), so MissingManualVars // would be empty and the check would falsely Pass. return Result{ - Status: StatusSkip, - Message: "skipped: no azd environment selected (cannot resolve agent.yaml variables)", + Status: StatusSkip, + Message: "skipped: no azd environment selected " + + "(cannot resolve agent environment variables)", } } @@ -132,7 +137,8 @@ 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..bd1c9952a1d 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 @@ -62,7 +62,7 @@ func TestCheckManualEnvVars_PriorAgentYAMLFailed_Skips(t *testing.T) { 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, "agent definition check") } func TestCheckManualEnvVars_PriorAgentYAMLSkipped_AlsoSkips(t *testing.T) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_models.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_models.go new file mode 100644 index 00000000000..69fb078fa7f --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_models.go @@ -0,0 +1,92 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package doctor + +import ( + "context" + "fmt" + "strings" + + "azureaiagent/internal/cmd/nextstep" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" +) + +// newCheckModels validates local model resource configuration. +func newCheckModels(deps Dependencies) Check { + return Check{ + ID: "local.models", + Name: "azure.yaml model configuration valid", + 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.azure-yaml") || + priorBlocked(prior, "local.agent-service-detected") { + return Result{ + Status: StatusSkip, + Message: "skipped: project configuration " + + "checks did not succeed.", + } + } + + assembler := deps.assembleState + if assembler == nil { + assembler = func( + ctx context.Context, + client *azdext.AzdClient, + ) (*nextstep.State, []error) { + return nextstep.AssembleState(ctx, client) + } + } + state, errs := assembler(ctx, deps.AzdClient) + if state == nil { + cause := "unknown error" + if len(errs) > 0 { + cause = errs[0].Error() + } + return Result{ + Status: StatusFail, + Message: fmt.Sprintf( + "failed to assemble agent state: %s", + cause, + ), + Suggestion: "Fix azure.yaml and retry.", + } + } + if len(state.ModelLoadErrors) > 0 { + return Result{ + Status: StatusFail, + Message: fmt.Sprintf( + "could not load model configuration: %s", + strings.Join(state.ModelLoadErrors, "; "), + ), + Suggestion: "Fix the model configuration in " + + "azure.yaml or the agent manifest, then retry.", + Details: map[string]any{ + "loadErrors": state.ModelLoadErrors, + }, + } + } + if !state.HasModels { + return Result{ + Status: StatusSkip, + Message: "skipped: no model resources " + + "declared in azure.yaml.", + } + } + return Result{ + Status: StatusPass, + Message: "model resource configuration loaded.", + } + }, + } +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_models_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_models_test.go new file mode 100644 index 00000000000..c71b80b5072 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_models_test.go @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package doctor + +import ( + "testing" + + "azureaiagent/internal/cmd/nextstep" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/stretchr/testify/require" +) + +func TestCheckModels(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + deps Dependencies + wantStatus Status + wantText string + }{ + { + name: "missing client skips", + wantStatus: StatusSkip, + wantText: "not reachable", + }, + { + name: "load errors fail", + deps: Dependencies{ + AzdClient: &azdext.AzdClient{}, + assembleState: fixedAssembler(&nextstep.State{ + ModelLoadErrors: []string{"missing deployment ref"}, + }), + }, + wantStatus: StatusFail, + wantText: "missing deployment ref", + }, + { + name: "no models skips", + deps: Dependencies{ + AzdClient: &azdext.AzdClient{}, + assembleState: fixedAssembler(&nextstep.State{}), + }, + wantStatus: StatusSkip, + wantText: "no model resources", + }, + { + name: "valid models pass", + deps: Dependencies{ + AzdClient: &azdext.AzdClient{}, + assembleState: fixedAssembler(&nextstep.State{ + HasModels: true, + }), + }, + wantStatus: StatusPass, + wantText: "configuration loaded", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + result := newCheckModels(test.deps).Fn( + t.Context(), + Options{}, + nil, + ) + require.Equal(t, test.wantStatus, result.Status) + require.Contains(t, result.Message, test.wantText) + }) + } +} 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..ff60f81cc6e 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 @@ -4,14 +4,13 @@ package doctor import ( + "cmp" "context" "fmt" - "os" - "sort" + "slices" "strings" - "azureaiagent/internal/pkg/agents/agent_yaml" - "azureaiagent/internal/pkg/paths" + projectpkg "azureaiagent/internal/project" "github.com/azure/azure-dev/cli/azd/pkg/azdext" ) @@ -74,7 +73,7 @@ func newCheckAgentServiceDetected(deps Dependencies) Check { } // Sort for deterministic display: protobuf Services is a map, // so iteration order is unstable across runs. - sort.Strings(agentServices) + slices.Sort(agentServices) if len(agentServices) == 0 { return Result{ Status: StatusFail, @@ -154,20 +153,11 @@ 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. +// newCheckAgentYAMLValid validates every agent definition. func newCheckAgentYAMLValid(deps Dependencies) Check { return Check{ ID: "local.agent-yaml-valid", - Name: "agent.yaml valid (per service)", + Name: "agent definition 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"} @@ -197,81 +187,79 @@ func newCheckAgentYAMLValid(deps Dependencies) Check { } 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 + var agents []*azdext.ServiceConfig for _, s := range resp.Project.Services { if s == nil || s.Host != agentHost { continue } - agents = append(agents, agentSvc{name: s.Name, rel: s.RelativePath}) + agents = append(agents, s) } - sort.Slice(agents, func(i, j int) bool { return agents[i].name < agents[j].name }) + slices.SortFunc(agents, func(a, b *azdext.ServiceConfig) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) - var validatedPaths []string + var validatedServices []string + var legacyServices []string var failures []string - for _, a := range agents { - yamlPath, err := paths.JoinAllowRoot(projectPath, a.rel, "agent.yaml") + for _, svc := range agents { + // A no-error load means the definition is valid. + // isHosted only distinguishes container agents from + // other valid kinds (e.g. workflow); the hosted-only + // check belongs to container operations, not here. + _, _, source, err := projectpkg.LoadAgentDefinition( + svc, + projectPath, + ) if err != nil { - failures = append(failures, fmt.Sprintf("%s: %v", a.name, err)) + failures = append( + failures, + fmt.Sprintf("%s: %v", svc.GetName(), err), + ) continue } - if pathErr := validateAgentYAML(yamlPath); pathErr != nil { - failures = append(failures, fmt.Sprintf("%s: %v", a.name, pathErr)) - continue + validatedServices = append( + validatedServices, + svc.GetName(), + ) + if source.IsLegacy() { + legacyServices = append( + legacyServices, + svc.GetName(), + ) } - validatedPaths = append(validatedPaths, yamlPath) } if len(failures) > 0 { return Result{ Status: StatusFail, Message: fmt.Sprintf( - "agent.yaml validation failed for %d service(s): %s", + "agent definition 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.", + Suggestion: "Fix the agent service definition in azure.yaml " + + "or re-run `azd ai agent init`.", Details: map[string]any{ - "failures": failures, - "validatedPaths": validatedPaths, + "failures": failures, + "validatedServices": validatedServices, + "legacyServices": legacyServices, }, } } return Result{ - Status: StatusPass, - Message: fmt.Sprintf("agent.yaml valid for %d service(s)", len(validatedPaths)), + Status: StatusPass, + Message: fmt.Sprintf( + "agent definitions valid for %d service(s)", + len(validatedServices), + ), Details: map[string]any{ - "validatedPaths": validatedPaths, + "validatedServices": validatedServices, + "legacyServices": legacyServices, }, } }, } } -// 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..a8f6c03b8cb 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 @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/structpb" ) // ---- Check `local.agent-service-detected` ---- @@ -387,7 +388,53 @@ protocols: 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)") + require.Contains( + t, + got.Message, + "agent definitions valid for 1 service(s)", + ) +} + +func TestCheckAgentYAMLValid_InlineDefinitionPasses(t *testing.T) { + t.Parallel() + + props, err := structpb.NewStruct(map[string]any{ + "kind": "hosted", + "name": "inline-agent", + "protocols": []any{ + map[string]any{ + "protocol": "responses", + "version": "1.0.0", + }, + }, + }) + require.NoError(t, err) + client := newTestAzdClient(t, + &fakeProjectServer{resp: &azdext.GetProjectResponse{ + Project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{ + "inline-agent": { + Name: "inline-agent", + Host: agentHost, + AdditionalProperties: props, + }, + }, + }, + }}, + &fakeEnvironmentServer{}) + + got := newCheckAgentYAMLValid( + Dependencies{AzdClient: client}, + ).Fn(t.Context(), Options{}, nil) + + require.Equal(t, StatusPass, got.Status) + require.Equal( + t, + []string{"inline-agent"}, + got.Details["validatedServices"], + ) + require.Empty(t, got.Details["legacyServices"]) } func TestCheckAgentYAMLValid_NonAgentServicesIgnored(t *testing.T) { @@ -413,10 +460,9 @@ func TestCheckAgentYAMLValid_NonAgentServicesIgnored(t *testing.T) { 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) + services, ok := got.Details["validatedServices"].([]string) require.True(t, ok) - require.Len(t, paths, 1) - require.Contains(t, paths[0], "src"+string(filepath.Separator)+"agent") + require.Equal(t, []string{"echo-agent"}, services) } func TestCheckAgentYAMLValid_MissingFile_Fails(t *testing.T) { @@ -441,7 +487,7 @@ func TestCheckAgentYAMLValid_MissingFile_Fails(t *testing.T) { require.Equal(t, StatusFail, got.Status) require.Contains(t, got.Message, "echo-agent") - require.Contains(t, got.Suggestion, "Fix the YAML") + require.Contains(t, got.Suggestion, "azure.yaml") failures, ok := got.Details["failures"].([]string) require.True(t, ok) require.Len(t, failures, 1) @@ -509,9 +555,9 @@ func TestCheckAgentYAMLValid_MixedValidAndInvalid_Fails(t *testing.T) { require.True(t, ok) require.Len(t, failures, 1) - validated, ok := got.Details["validatedPaths"].([]string) + validated, ok := got.Details["validatedServices"].([]string) require.True(t, ok) - require.Len(t, validated, 1) + require.Equal(t, []string{"ok-agent"}, validated) } // ---- helper: priorBlocked ---- @@ -643,6 +689,35 @@ func TestCheckAgentYAMLValid_InvalidName_Fails(t *testing.T) { require.Contains(t, failures[0], "name") } +func TestCheckAgentYAMLValid_Workflow_Passes(t *testing.T) { + // `kind: workflow` is a valid agent kind (ValidAgentKinds includes + // it) even though it is not a hosted/container agent. Doctor must + // accept it rather than rejecting every non-hosted definition. + t.Parallel() + + projectPath := t.TempDir() + writeYAML(t, projectPath, "src/wf/agent.yaml", "kind: workflow\nname: my-workflow\n") + + client := newTestAzdClient(t, + &fakeProjectServer{resp: &azdext.GetProjectResponse{ + Project: &azdext.ProjectConfig{ + Path: projectPath, + Services: map[string]*azdext.ServiceConfig{ + "my-workflow": {Name: "my-workflow", Host: agentHost, RelativePath: "src/wf"}, + }, + }, + }}, + &fakeEnvironmentServer{}) + check := newCheckAgentYAMLValid(Dependencies{AzdClient: client}) + + got := check.Fn(t.Context(), Options{}, nil) + + require.Equal(t, StatusPass, got.Status) + validated, ok := got.Details["validatedServices"].([]string) + require.True(t, ok) + require.Equal(t, []string{"my-workflow"}, validated) +} + // 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) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_remote_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_remote_test.go index 2f877a81cd4..53c1c19e349 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_remote_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_remote_test.go @@ -58,7 +58,7 @@ func TestNewRemoteChecks_HasAuthFoundryEndpointRBACAgentStatusConnections(t *tes require.True(t, got[3].Remote, "remote.agent-status must declare Remote=true") require.NotNil(t, got[3].Fn, "remote.agent-status must have a non-nil Fn") require.Equal(t, "remote.connections", got[4].ID) - require.Equal(t, "Manifest connections exist on Foundry project", got[4].Name) + require.Equal(t, "Configured connections exist on Foundry project", got[4].Name) require.True(t, got[4].Remote, "remote.connections must declare Remote=true") require.NotNil(t, got[4].Fn, "remote.connections must have a non-nil Fn") } 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..3e4714cb8a6 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,8 +26,7 @@ import ( type toolboxEnvLookupFn func(ctx context.Context, key string) (value string, err error) // newCheckToolboxes produces Check `local.toolboxes` (P5.1 C14). -// For each `ToolboxResource` declared in any service's -// `agent.manifest.yaml` (collected by the C2 manifest walker), the +// For each toolbox declared 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. @@ -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: "azure.yaml toolboxes have endpoint env vars set", Remote: false, Fn: func(ctx context.Context, _ Options, prior []Result) Result { if deps.AzdClient == nil { @@ -116,10 +115,28 @@ func newCheckToolboxes(deps Dependencies) Check { Suggestion: "Re-run `azd ai agent doctor`; the state assembly returned nil unexpectedly.", } } + if len(state.ToolboxLoadErrors) > 0 { + return Result{ + Status: StatusFail, + Message: fmt.Sprintf( + "could not load toolbox configuration: %s", + strings.Join( + state.ToolboxLoadErrors, + "; ", + ), + ), + Suggestion: "Fix the toolbox services in " + + "azure.yaml or the agent manifest, then retry.", + Details: map[string]any{ + "loadErrors": state.ToolboxLoadErrors, + }, + } + } if !state.HasToolboxes { return Result{ - Status: StatusSkip, - Message: "skipped: no toolbox resources declared in any service's agent.manifest.yaml.", + Status: StatusSkip, + Message: "skipped: no toolbox resources " + + "declared in azure.yaml.", } } @@ -153,9 +170,10 @@ func classifyToolboxEndpoints( lookup toolboxEnvLookupFn, ) Result { type toolboxLookup struct { - Name string `json:"name"` - ServiceName string `json:"service"` - EnvVar string `json:"envVar"` + Name string `json:"name"` + ServiceName string `json:"service"` + EnvVar string `json:"envVar"` + ManagedByDeploy bool `json:"-"` } seen := make(map[string]struct{}, len(toolboxes)) @@ -182,7 +200,10 @@ func classifyToolboxEndpoints( } if strings.TrimSpace(value) == "" { missing = append(missing, toolboxLookup{ - Name: t.Name, ServiceName: t.ServiceName, EnvVar: key, + Name: t.Name, + ServiceName: t.ServiceName, + EnvVar: key, + ManagedByDeploy: t.ManagedByDeploy, }) continue } @@ -214,13 +235,31 @@ func classifyToolboxEndpoints( sb.WriteString(fmt.Sprintf("%s (env %s, service %s)", m.Name, m.EnvVar, m.ServiceName)) } + needsDeploy := slices.ContainsFunc(missing, func(entry toolboxLookup) bool { + return entry.ManagedByDeploy + }) + needsProvision := slices.ContainsFunc(missing, func(entry toolboxLookup) bool { + return !entry.ManagedByDeploy + }) + suggestion := "Run `azd provision` to materialize legacy " + + "toolbox infrastructure." + switch { + case needsDeploy && needsProvision: + suggestion = "Run `azd deploy` for toolbox services and " + + "`azd provision` for legacy toolbox resources." + case needsDeploy: + suggestion = "Run `azd deploy` to deploy toolbox services." + } + suggestion += " Alternatively, use `azd env set " + + "` to point at an existing toolbox." + 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.", + Suggestion: suggestion, Details: map[string]any{ "missingToolboxes": missing, "matchedCount": matched, diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes_test.go index 14f7fa91b2a..a0cc80bb80b 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_toolboxes_test.go @@ -189,6 +189,43 @@ func TestCheckToolboxes_FailsWhenSomeEndpointsMissing(t *testing.T) { require.Equal(t, 1, res.Details["matchedCount"]) } +func TestCheckToolboxes_SplitResourceUsesDeploy(t *testing.T) { + t.Parallel() + deps := Dependencies{ + AzdClient: &azdext.AzdClient{}, + assembleState: fixedAssembler(stateWithToolboxes( + nextstep.ResourceRef{ + Name: "web-search-tools", + ServiceName: "web-search-tools", + ManagedByDeploy: true, + }, + )), + lookupToolboxEnv: fixedToolboxLookup(map[string]string{}), + } + + res := runToolboxesCheck(t, deps, nil) + + require.Equal(t, StatusFail, res.Status) + require.Contains(t, res.Suggestion, "azd deploy") + require.NotContains(t, res.Suggestion, "azd provision") +} + +func TestCheckToolboxes_FailsOnIncompleteState(t *testing.T) { + t.Parallel() + deps := Dependencies{ + AzdClient: &azdext.AzdClient{}, + assembleState: fixedAssembler(&nextstep.State{ + ToolboxLoadErrors: []string{"missing toolbox ref"}, + }), + } + + res := runToolboxesCheck(t, deps, nil) + + require.Equal(t, StatusFail, res.Status) + require.Contains(t, res.Message, "missing toolbox ref") + require.Contains(t, res.Suggestion, "azure.yaml") +} + func TestCheckToolboxes_FailsWhenAllEndpointsMissing(t *testing.T) { t.Parallel() deps := Dependencies{ diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index 496df5a09ad..8c029569025 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "log" + "maps" "net/http" "net/url" "os" @@ -22,6 +23,7 @@ import ( "azureaiagent/internal/pkg/agents/agent_api" "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/paths" + "azureaiagent/internal/pkg/projectconfig" projectpkg "azureaiagent/internal/project" "github.com/azure/azure-dev/cli/azd/pkg/azdext" @@ -706,6 +708,7 @@ type ServiceRunContext struct { ServiceName string // the resolved service name (from azure.yaml) ProjectDir string // absolute path to the service source directory StartupCommand string // startupCommand from AdditionalProperties (may be empty) + Environment map[string]string // Definition is the resolved agent definition (from the inline azure.yaml // entry or a legacy agent.yaml). It is nil when no definition can be resolved. Definition *agent_yaml.ContainerAgent @@ -718,6 +721,20 @@ func resolveServiceRunContext(ctx context.Context, azdClient *azdext.AzdClient, if err != nil { return nil, err } + if err := projectpkg.ResolveServiceConfigInPlace( + svc, + project.Path, + ); err != nil { + return nil, exterrors.Validation( + exterrors.CodeInvalidServiceConfig, + fmt.Sprintf( + "failed to resolve agent service %s: %s", + svc.Name, + err, + ), + "fix the agent service configuration in azure.yaml", + ) + } projectDir, err := paths.JoinAllowRoot(project.Path, svc.RelativePath) if err != nil { @@ -729,8 +746,28 @@ func resolveServiceRunContext(ctx context.Context, azdClient *azdext.AzdClient, } var startupCmd string - if agentConfig, cfgErr := projectpkg.LoadServiceTargetAgentConfig(svc); cfgErr == nil { + serviceEnv := map[string]string{} + if agentConfig, cfgErr := projectpkg.LoadServiceTargetAgentConfig( + svc, + ); cfgErr == nil { startupCmd = agentConfig.StartupCommand + maps.Copy(serviceEnv, agentConfig.Environment) + } + serviceEnv, err = loadServiceRunEnvironment( + project.Path, + svc, + serviceEnv, + ) + if err != nil { + return nil, exterrors.Validation( + exterrors.CodeInvalidServiceConfig, + fmt.Sprintf( + "failed to load environment for %s: %s", + svc.Name, + err, + ), + "fix the service env configuration in azure.yaml", + ) } var definition *agent_yaml.ContainerAgent @@ -745,10 +782,35 @@ func resolveServiceRunContext(ctx context.Context, azdClient *azdext.AzdClient, ServiceName: svc.Name, ProjectDir: projectDir, StartupCommand: startupCmd, + Environment: serviceEnv, Definition: definition, }, nil } +func loadServiceRunEnvironment( + projectRoot string, + svc *azdext.ServiceConfig, + base map[string]string, +) (map[string]string, error) { + env := maps.Clone(base) + if env == nil { + env = map[string]string{} + } + raw, err := projectconfig.LoadServiceEnvironment( + projectRoot, + svc.GetName(), + ) + if err != nil { + return nil, err + } + if raw == nil { + maps.Copy(env, svc.GetEnvironment()) + } else { + maps.Copy(env, raw) + } + return env, nil +} + // toServiceKey converts a service name into the env var key format (uppercase, underscores). func toServiceKey(serviceName string) string { key := strings.ReplaceAll(serviceName, " ", "_") diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go index dbad549aea1..6b2067f4c6e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go @@ -517,3 +517,41 @@ func TestResolveAgentProtocol_MultipleServicesPromptsOnce(t *testing.T) { require.Equal(t, int32(1), promptServer.selectCalls.Load(), "resolveAgentProtocol should trigger exactly one prompt") } + +func TestLoadServiceRunEnvironmentUsesRawValues(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "azure.yaml"), + []byte(`services: + agent: + host: azure.ai.agent + env: + PROJECT: ${{project.endpoint}} + ENABLED: true + SHARED: direct +`), + 0o600, + )) + svc := &azdext.ServiceConfig{ + Name: "agent", + Environment: map[string]string{ + "PROJECT": "", + "ENABLED": "", + "SHARED": "expanded", + }, + } + + env, err := loadServiceRunEnvironment( + root, + svc, + map[string]string{"CONFIG_ONLY": "config", "SHARED": "config"}, + ) + + require.NoError(t, err) + require.Equal(t, "${{project.endpoint}}", env["PROJECT"]) + require.Equal(t, "true", env["ENABLED"]) + require.Equal(t, "direct", env["SHARED"]) + require.Equal(t, "config", env["CONFIG_ONLY"]) +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go index 9684774eddd..b1f7bfe853d 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go @@ -15,7 +15,6 @@ import ( "sync" "azureaiagent/internal/exterrors" - "azureaiagent/internal/pkg/agents/agent_api" "azureaiagent/internal/pkg/agents/optimize_api" "azureaiagent/internal/project" @@ -68,11 +67,17 @@ func configureExtensionHost(host *azdext.ExtensionHost) { } func preprovisionHandler(ctx context.Context, azdClient *azdext.AzdClient, args *azdext.ProjectEventArgs) error { - deployments, err := collectProjectDeployments(args.Project.Services) + deployments, err := collectProjectDeploymentsAtRoot( + args.Project.Services, + args.Project.Path, + ) if err != nil { return err } - connections, err := collectConnections(args.Project.Services) + connections, err := collectConnectionsAtRoot( + args.Project.Services, + args.Project.Path, + ) if err != nil { return err } @@ -80,7 +85,12 @@ func preprovisionHandler(ctx context.Context, azdClient *azdext.AzdClient, args for _, svc := range args.Project.Services { switch svc.Host { case AiAgentHost: - if err := populateContainerSettings(ctx, azdClient, svc); err != nil { + if err := populateContainerSettings( + ctx, + azdClient, + svc, + args.Project.Path, + ); err != nil { return fmt.Errorf("failed to populate container settings for service %q: %w", svc.Name, err) } if err := envUpdate(ctx, azdClient, args.Project, svc, deployments, connections); err != nil { @@ -194,29 +204,33 @@ func predeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *az warnDuplicateAgentNames(args.Project) }) - deployments, err := collectProjectDeployments(args.Project.Services) + deployments, err := collectProjectDeploymentsAtRoot( + args.Project.Services, + args.Project.Path, + ) if err != nil { return err } - connections, err := collectConnections(args.Project.Services) + connections, err := collectConnectionsAtRoot( + args.Project.Services, + args.Project.Path, + ) if err != nil { return err } - if err := populateContainerSettings(ctx, azdClient, svc); err != nil { + if err := populateContainerSettings( + ctx, + azdClient, + svc, + args.Project.Path, + ); err != nil { return fmt.Errorf("failed to populate container settings for service %q: %w", svc.Name, err) } if err := envUpdate(ctx, azdClient, args.Project, svc, deployments, connections); err != nil { return fmt.Errorf("failed to update environment for service %q: %w", svc.Name, err) } - // Capture the current session so it can be resumed on the newly deployed - // version after deploy (see session_carryover.go). Best-effort; hosted - // agents only. - if isHostedAgentService(svc, args.Project) { - captureSessionForCarryover(ctx, azdClient, svc) - } - // Run developer RBAC pre-flight checks only for hosted agent deployments. // Guarded by sync.Once since this handler fires per-service but the check // is project-scoped. @@ -406,12 +420,6 @@ func postdeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *a ) }() - // Resume the pre-deploy session on the newly deployed version so the next - // invoke continues on the new code with the session's persisted volume - // intact (see session_carryover.go). Best-effort; never blocks deploy. - agentClient := agent_api.NewAgentClient(endpoint, cred) - carryOverSessionAfterDeploy(ctx, azdClient, agentClient, svc, envName) - return nil } @@ -716,10 +724,56 @@ func setEnvVar(ctx context.Context, azdClient *azdext.AzdClient, envName string, return nil } -func populateContainerSettings(ctx context.Context, azdClient *azdext.AzdClient, svc *azdext.ServiceConfig) error { +func populateContainerSettings( + ctx context.Context, + azdClient *azdext.AzdClient, + svc *azdext.ServiceConfig, + projectRoot string, +) error { + persisted, err := prepareContainerSettings(svc, projectRoot) + if err != nil { + return err + } + if persisted == nil { + return nil + } + + if err := project.AddServiceSerialized( + ctx, + azdClient, + persisted, + ); err != nil { + return fmt.Errorf("adding agent service to project: %w", err) + } + + return nil +} + +func prepareContainerSettings( + svc *azdext.ServiceConfig, + projectRoot string, +) (*azdext.ServiceConfig, error) { + rawAdditional := svc.GetAdditionalProperties() + rawConfig := svc.GetConfig() + hasFileRef := project.ConfigContainsFileRef(rawAdditional) || + project.ConfigContainsFileRef(rawConfig) + if hasFileRef { + if err := project.ResolveServiceConfigInPlace( + svc, + projectRoot, + ); err != nil { + return nil, fmt.Errorf( + "failed to resolve agent config: %w", + err, + ) + } + } foundryAgentConfig, err := project.LoadServiceTargetAgentConfig(svc) if err != nil { - return fmt.Errorf("failed to parse foundry agent config: %w", err) + return nil, fmt.Errorf( + "failed to parse foundry agent config: %w", + err, + ) } // Resolve the container resources, applying defaults when unset. @@ -738,20 +792,19 @@ func populateContainerSettings(ctx context.Context, azdClient *azdext.AzdClient, result.Cpu = project.DefaultCpu } - // Persist the resolved container settings back onto the service's inline - // properties, preserving the agent definition and other config keys. - if err := project.SetAgentContainerSettings(svc, &project.ContainerSettings{Resources: result}); err != nil { - return fmt.Errorf("failed to update agent container settings: %w", err) + if err := project.SetAgentContainerSettings( + svc, + &project.ContainerSettings{Resources: result}, + ); err != nil { + return nil, fmt.Errorf( + "failed to update agent container settings: %w", + err, + ) } - - // Need to add the service config back to the project for use further down the pipeline - req := &azdext.AddServiceRequest{Service: svc} - - if _, err := azdClient.Project().AddService(ctx, req); err != nil { - return fmt.Errorf("adding agent service to project: %w", err) + if hasFileRef { + return nil, nil } - - return nil + return svc, nil } // resolveToolboxEnvVars resolves ${VAR} references in toolbox name, description, diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go index 81d63020a88..bc072d0e224 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go @@ -17,6 +17,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" ) // TestPostdeployHandler_NonHostedAgent_NoOp verifies postdeployHandler returns nil @@ -130,6 +131,47 @@ func TestIsHostedAgentServiceRejectsTraversal(t *testing.T) { } } +func TestPopulateContainerSettings_DoesNotPersistResolvedFileRef( + t *testing.T, +) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "agent.yaml"), + []byte( + "kind: hosted\n"+ + "name: echo\n"+ + "project: src/echo\n"+ + "container:\n"+ + " resources:\n"+ + " cpu: \"2\"\n"+ + " memory: 4Gi\n", + ), + 0o600, + )) + props, err := structpb.NewStruct(map[string]any{ + "$ref": "./agent.yaml", + }) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "echo", + Host: AiAgentHost, + AdditionalProperties: props, + } + + err = populateContainerSettings(t.Context(), nil, svc, root) + + require.NoError(t, err) + require.Equal(t, "src/echo", svc.GetRelativePath()) + cfg, err := project.LoadServiceTargetAgentConfig(svc) + require.NoError(t, err) + require.NotNil(t, cfg.Container) + require.NotNil(t, cfg.Container.Resources) + require.Equal(t, "2", cfg.Container.Resources.Cpu) + require.Equal(t, "4Gi", cfg.Container.Resources.Memory) +} + func TestKindEnvUpdateRejectsTraversal(t *testing.T) { t.Parallel() diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/config.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/config.go new file mode 100644 index 00000000000..5d3d8f69290 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/config.go @@ -0,0 +1,263 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package nextstep + +import ( + "encoding/json" + "fmt" + "maps" + "os" + + "azureaiagent/internal/pkg/agents/agent_yaml" + "azureaiagent/internal/pkg/paths" + "azureaiagent/internal/pkg/projectconfig" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/foundry" + "go.yaml.in/yaml/v3" + "google.golang.org/protobuf/types/known/structpb" +) + +type guidanceAgentDefinition struct { + agent_yaml.AgentDefinition `json:",inline"` + Protocols []agent_yaml.ProtocolVersionRecord `json:"protocols,omitempty"` + EnvironmentVariables *[]agent_yaml.EnvironmentVariable `json:"environmentVariables,omitempty"` +} + +type guidanceServiceConfig struct { + Environment map[string]string `json:"env,omitempty"` + Deployments []guidanceDeployment `json:"deployments,omitempty"` + Toolboxes []guidanceToolbox `json:"toolboxes,omitempty"` + Connections []guidanceConnection `json:"connections,omitempty"` +} + +type guidanceDeployment struct { + Name string `json:"name"` + Model guidanceDeploymentModel `json:"model"` +} + +type guidanceDeploymentModel struct { + Name string `json:"name"` +} + +type guidanceToolbox struct { + Name string `json:"name"` +} + +func (t *guidanceToolbox) UnmarshalJSON(data []byte) error { + var name string + if err := json.Unmarshal(data, &name); err == nil { + t.Name = name + return nil + } + type toolbox guidanceToolbox + var value toolbox + if err := json.Unmarshal(data, &value); err != nil { + return err + } + *t = guidanceToolbox(value) + return nil +} + +type guidanceConnection struct { + Name string `json:"name"` + Category string `json:"category"` + Target string `json:"target"` + AuthType string `json:"authType"` +} + +func loadGuidanceServiceConfig( + svc *azdext.ServiceConfig, + projectRoot string, +) ( + guidanceAgentDefinition, + guidanceServiceConfig, + *structpb.Struct, + error, +) { + var agentDef guidanceAgentDefinition + var cfg guidanceServiceConfig + var fallbackProps *structpb.Struct + + for _, candidate := range []*structpb.Struct{ + svc.GetAdditionalProperties(), + svc.GetConfig(), + } { + if candidate == nil || len(candidate.GetFields()) == 0 { + continue + } + props, err := resolveServiceProps( + candidate, + svc.GetName(), + projectRoot, + ) + if err != nil { + return agentDef, cfg, nil, err + } + var candidateCfg guidanceServiceConfig + if err := unmarshalServiceProps( + props, + &candidateCfg, + ); err != nil { + return agentDef, cfg, props, err + } + if fallbackProps == nil { + fallbackProps = props + cfg = candidateCfg + } + if kind := props.GetFields()["kind"]; kind != nil && + kind.GetStringValue() != "" { + if err := unmarshalServiceProps( + props, + &agentDef, + ); err != nil { + return agentDef, candidateCfg, props, err + } + if err := mergeServiceEnvironment( + svc, + projectRoot, + &candidateCfg, + ); err != nil { + return agentDef, candidateCfg, props, err + } + return agentDef, candidateCfg, props, nil + } + } + + legacy, found, err := loadLegacyAgentDefinition( + svc, + projectRoot, + ) + if err != nil { + return agentDef, cfg, fallbackProps, err + } + if found { + agentDef = guidanceAgentDefinition{ + AgentDefinition: legacy.AgentDefinition, + Protocols: legacy.Protocols, + EnvironmentVariables: legacy.EnvironmentVariables, + } + } + if err := mergeServiceEnvironment( + svc, + projectRoot, + &cfg, + ); err != nil { + return agentDef, cfg, fallbackProps, err + } + return agentDef, cfg, fallbackProps, nil +} + +func resolveServiceConfigProps( + svc *azdext.ServiceConfig, + projectRoot string, +) (*structpb.Struct, error) { + props := svc.GetAdditionalProperties() + if props == nil || len(props.GetFields()) == 0 { + props = svc.GetConfig() + } + if props == nil { + return nil, nil + } + return resolveServiceProps(props, svc.GetName(), projectRoot) +} + +func resolveServiceProps( + props *structpb.Struct, + serviceName string, + projectRoot string, +) (*structpb.Struct, error) { + resolved, err := foundry.ResolveFileRefs( + props.AsMap(), + projectRoot, + ) + if err != nil { + return nil, fmt.Errorf( + "resolving service %q config: %w", + serviceName, + err, + ) + } + if err := projectconfig.NormalizeEnvironment(resolved); err != nil { + return nil, fmt.Errorf( + "normalizing service %q environment: %w", + serviceName, + err, + ) + } + out, err := structpb.NewStruct(resolved) + if err != nil { + return nil, fmt.Errorf( + "encoding service %q config: %w", + serviceName, + err, + ) + } + return out, nil +} + +func unmarshalServiceProps( + props *structpb.Struct, + out any, +) error { + data, err := json.Marshal(props.AsMap()) + if err != nil { + return err + } + return json.Unmarshal(data, out) +} + +func mergeServiceEnvironment( + svc *azdext.ServiceConfig, + projectRoot string, + cfg *guidanceServiceConfig, +) error { + if cfg.Environment == nil { + cfg.Environment = map[string]string{} + } + for key, value := range svc.GetEnvironment() { + if _, exists := cfg.Environment[key]; !exists { + cfg.Environment[key] = value + } + } + + raw, err := projectconfig.LoadServiceEnvironment( + projectRoot, + svc.GetName(), + ) + if err != nil { + return err + } + maps.Copy(cfg.Environment, raw) + return nil +} + +func loadLegacyAgentDefinition( + svc *azdext.ServiceConfig, + projectRoot string, +) (agent_yaml.ContainerAgent, bool, error) { + if projectRoot == "" { + return agent_yaml.ContainerAgent{}, false, nil + } + for _, name := range []string{"agent.yaml", "agent.yml"} { + path, err := paths.JoinAllowRoot( + projectRoot, + svc.GetRelativePath(), + name, + ) + if err != nil { + return agent_yaml.ContainerAgent{}, false, err + } + data, err := os.ReadFile(path) //nolint:gosec + if err != nil { + continue + } + var agentDef agent_yaml.ContainerAgent + if err := yaml.Unmarshal(data, &agentDef); err != nil { + return agent_yaml.ContainerAgent{}, false, err + } + return agentDef, true, nil + } + return agent_yaml.ContainerAgent{}, false, nil +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/format.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/format.go index aaf1e48e7ac..dd773ef39e9 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/format.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/format.go @@ -193,7 +193,7 @@ func renderRows(suggestions []Suggestion, limit int) string { // highlightCommand returns cmd wrapped in the highlight (blue) color when // it is a runnable azd command (prefix "azd "), and cmd unchanged // otherwise. Non-command suggestions — "see /README.md" pointers and -// "edit agent.yaml: ..." instructions — stay plain. +// "edit azure.yaml: ..." instructions stay plain. // // output.WithHighLightFormat gates on color.NoColor. In this extension that // flag is driven by the FORCE_COLOR env var azd core sets when core itself 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 index 61128166ce5..44aec833fee 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/manifest.go @@ -5,11 +5,14 @@ package nextstep import ( "cmp" + "fmt" "os" "slices" "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/paths" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" ) // manifestFileNames are the candidate manifest filenames the walker @@ -25,15 +28,16 @@ var manifestFileNames = []string{ "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. +const ( + aiProjectHost = "azure.ai.project" + aiConnectionHost = "azure.ai.connection" + aiToolboxHost = "azure.ai.toolbox" +) + +// populateResources reads unified resources with legacy fallbacks. // -// Aggregation rules: +// Unified split services take precedence over bundled service config. +// Bundled config takes precedence over legacy manifest files. // // - Has* flags are true when at least one resource of the matching // kind is found across all services. @@ -52,74 +56,403 @@ var manifestFileNames = []string{ // 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 { +func populateResources( + projectConfig *azdext.ProjectConfig, + state *State, + errs *[]error, +) { + if projectConfig == nil || state == nil { return } - models := map[resourceKey]ResourceRef{} - toolboxes := map[resourceKey]ResourceRef{} - connections := map[resourceKey]ResourceRef{} + resources := newResourceSets() + collectSplitResources( + projectConfig, + &resources, + errs, + ) + collectBundledResources( + projectConfig, + &resources, + errs, + ) + collectManifestResources( + projectConfig.Path, + state.Services, + &resources, + errs, + ) + applyResourceSets(state, resources) +} + +type resourceSets struct { + models map[resourceKey]ResourceRef + toolboxes map[resourceKey]ResourceRef + connections map[resourceKey]ResourceRef + modelErrors []string + toolboxErrors []string + connectionErrors []string +} + +func newResourceSets() resourceSets { + return resourceSets{ + models: map[resourceKey]ResourceRef{}, + toolboxes: map[resourceKey]ResourceRef{}, + connections: map[resourceKey]ResourceRef{}, + } +} + +func collectSplitResources( + projectConfig *azdext.ProjectConfig, + resources *resourceSets, + errs *[]error, +) { + for _, svc := range sortedProjectServices(projectConfig.Services) { + switch svc.GetHost() { + case aiProjectHost: + props, err := resolveServiceConfigProps( + svc, + projectConfig.Path, + ) + if err != nil { + appendResourceError( + errs, + &resources.modelErrors, + svc, + err, + ) + continue + } + var cfg guidanceServiceConfig + if props != nil { + if err := unmarshalServiceProps( + props, + &cfg, + ); err != nil { + appendResourceError( + errs, + &resources.modelErrors, + svc, + err, + ) + continue + } + } + for _, deployment := range cfg.Deployments { + addResource( + resources.models, + ResourceRef{ + Name: deployment.Name, + ServiceName: svc.GetName(), + Detail: deployment.Model.Name, + }, + ) + } + case aiConnectionHost: + props, err := resolveServiceConfigProps( + svc, + projectConfig.Path, + ) + if err != nil { + appendResourceError( + errs, + &resources.connectionErrors, + svc, + err, + ) + continue + } + var connection guidanceConnection + if props != nil { + if err := unmarshalServiceProps( + props, + &connection, + ); err != nil { + appendResourceError( + errs, + &resources.connectionErrors, + svc, + err, + ) + continue + } + } + connection.Name = svc.GetName() + addResource( + resources.connections, + ResourceRef{ + Name: connection.Name, + ServiceName: svc.GetName(), + Detail: connectionDetail( + connection.Category, + connection.Target, + ), + }, + ) + case aiToolboxHost: + if _, err := resolveServiceConfigProps( + svc, + projectConfig.Path, + ); err != nil { + appendResourceError( + errs, + &resources.toolboxErrors, + svc, + err, + ) + continue + } + addResource( + resources.toolboxes, + ResourceRef{ + Name: svc.GetName(), + ServiceName: svc.GetName(), + ManagedByDeploy: true, + }, + ) + } + } +} + +func collectBundledResources( + projectConfig *azdext.ProjectConfig, + resources *resourceSets, + errs *[]error, +) { + needsModels := len(resources.models) == 0 + needsToolboxes := len(resources.toolboxes) == 0 + needsConnections := len(resources.connections) == 0 + if !needsModels && !needsToolboxes && !needsConnections { + return + } - for _, svc := range state.Services { + for _, svc := range sortedProjectServices(projectConfig.Services) { + if svc.GetHost() != agentHost { + continue + } + _, cfg, _, err := loadGuidanceServiceConfig( + svc, + projectConfig.Path, + ) + if err != nil { + appendResourceLoadError( + errs, + resources, + svc.GetName(), + err, + needsModels, + needsToolboxes, + needsConnections, + ) + continue + } + if needsModels { + for _, deployment := range cfg.Deployments { + addResource( + resources.models, + ResourceRef{ + Name: deployment.Name, + ServiceName: svc.GetName(), + Detail: deployment.Model.Name, + }, + ) + } + } + if needsToolboxes { + for _, toolbox := range cfg.Toolboxes { + addResource( + resources.toolboxes, + ResourceRef{ + Name: toolbox.Name, + ServiceName: svc.GetName(), + }, + ) + } + } + if needsConnections { + for _, connection := range cfg.Connections { + addResource( + resources.connections, + ResourceRef{ + Name: connection.Name, + ServiceName: svc.GetName(), + Detail: connectionDetail( + connection.Category, + connection.Target, + ), + }, + ) + } + } + } +} + +func collectManifestResources( + projectPath string, + services []ServiceState, + resources *resourceSets, + errs *[]error, +) { + needsModels := len(resources.models) == 0 + needsToolboxes := len(resources.toolboxes) == 0 + needsConnections := len(resources.connections) == 0 + if projectPath == "" || + (!needsModels && !needsToolboxes && !needsConnections) { + return + } + + for _, svc := range services { data := readManifestBytes(projectPath, svc.RelativePath) if data == nil { continue } - resources, err := agent_yaml.ExtractResourceDefinitions(data) + definitions, err := agent_yaml.ExtractResourceDefinitions(data) if err != nil { + appendResourceLoadError( + errs, + resources, + svc.Name, + err, + needsModels, + needsToolboxes, + needsConnections, + ) continue } - for _, resource := range resources { - switch r := resource.(type) { + for _, definition := range definitions { + switch r := definition.(type) { case agent_yaml.ModelResource: - if r.Name == "" { + if !needsModels || r.Name == "" { continue } - k := resourceKey{service: svc.Name, name: r.Name} - if _, dup := models[k]; dup { - continue - } - models[k] = ResourceRef{ + addResource(resources.models, 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 { + if !needsToolboxes || r.Name == "" { continue } - toolboxes[k] = ResourceRef{ + addResource(resources.toolboxes, 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 { + if !needsConnections || r.Name == "" { continue } - connections[k] = ResourceRef{ + addResource(resources.connections, ResourceRef{ Name: r.Name, ServiceName: svc.Name, - Detail: connectionDetail(r), - } + Detail: connectionDetail( + string(r.Category), + r.Target, + ), + }) } } } +} - state.ModelRefs = sortedResourceRefs(models) - state.Toolboxes = sortedResourceRefs(toolboxes) - state.Connections = sortedResourceRefs(connections) +func applyResourceSets(state *State, resources resourceSets) { + state.ModelRefs = sortedResourceRefs(resources.models) + state.Toolboxes = sortedResourceRefs(resources.toolboxes) + state.Connections = sortedResourceRefs(resources.connections) state.HasModels = len(state.ModelRefs) > 0 state.HasToolboxes = len(state.Toolboxes) > 0 state.HasConnections = len(state.Connections) > 0 + state.ModelLoadErrors = slices.Clone(resources.modelErrors) + state.ToolboxLoadErrors = slices.Clone(resources.toolboxErrors) + state.ConnectionLoadErrors = slices.Clone( + resources.connectionErrors, + ) +} + +func addResource( + resources map[resourceKey]ResourceRef, + resource ResourceRef, +) { + if resource.Name == "" { + return + } + key := resourceKey{ + service: resource.ServiceName, + name: resource.Name, + } + if _, exists := resources[key]; exists { + return + } + resources[key] = resource +} + +func appendResourceError( + errs *[]error, + loadErrors *[]string, + svc *azdext.ServiceConfig, + err error, +) { + wrapped := fmt.Errorf( + "load resources for %s: %w", + svc.GetName(), + err, + ) + *loadErrors = append(*loadErrors, wrapped.Error()) + if errs != nil { + *errs = append(*errs, wrapped) + } +} + +func appendResourceLoadError( + errs *[]error, + resources *resourceSets, + serviceName string, + err error, + models bool, + toolboxes bool, + connections bool, +) { + wrapped := fmt.Errorf( + "load resources for %s: %w", + serviceName, + err, + ) + if errs != nil { + *errs = append(*errs, wrapped) + } + if models { + resources.modelErrors = append( + resources.modelErrors, + wrapped.Error(), + ) + } + if toolboxes { + resources.toolboxErrors = append( + resources.toolboxErrors, + wrapped.Error(), + ) + } + if connections { + resources.connectionErrors = append( + resources.connectionErrors, + wrapped.Error(), + ) + } +} + +func sortedProjectServices( + services map[string]*azdext.ServiceConfig, +) []*azdext.ServiceConfig { + out := make([]*azdext.ServiceConfig, 0, len(services)) + for _, svc := range services { + if svc != nil { + out = append(out, svc) + } + } + slices.SortFunc(out, func(a, b *azdext.ServiceConfig) int { + return cmp.Compare(a.GetName(), b.GetName()) + }) + return out } // readManifestBytes returns the first manifest file's contents under @@ -151,9 +484,7 @@ func readManifestBytes(projectPath, relativePath string) []byte { // 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 +func connectionDetail(category, target string) string { switch { case category != "" && target != "": return category + " | " + target 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 index 2a9691c4da9..68df9f98bf3 100644 --- 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 @@ -10,8 +10,6 @@ import ( "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" @@ -97,6 +95,137 @@ func TestAssembleState_ManifestWalker_AllThreeKinds(t *testing.T) { assert.Equal(t, "BingLLMSearch | https://api.bing.microsoft.com/", state.Connections[0].Detail) } +func TestAssembleState_UnifiedSplitResources(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{ + "ai-project": { + Name: "ai-project", + Host: aiProjectHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "deployments": []any{ + map[string]any{ + "name": "gpt-4o", + "model": map[string]any{ + "name": "gpt-4o", + }, + }, + }, + }), + }, + "search-conn": { + Name: "search-conn", + Host: aiConnectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "CognitiveSearch", + "target": "https://search.example", + "authType": "ApiKey", + }), + }, + "research-tools": { + Name: "research-tools", + Host: aiToolboxHost, + }, + "echo": { + Name: "echo", + Host: agentHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "kind": "hosted", + "name": "echo-agent", + "toolboxes": []any{"research-tools"}, + "env": map[string]any{ + "TOOLBOX_ENDPOINT": "${TOOLBOX_RESEARCH_TOOLS_MCP_ENDPOINT}", + }, + }), + }, + }, + }, + values: map[string]string{ + "dev/" + projectEndpointVar: "https://example", + }, + } + + state, errs := assembleState(t.Context(), src) + + require.Empty(t, errs) + require.Len(t, state.ModelRefs, 1) + assert.Equal(t, "gpt-4o", state.ModelRefs[0].Name) + assert.Equal(t, "ai-project", state.ModelRefs[0].ServiceName) + + require.Len(t, state.Connections, 1) + assert.Equal(t, "search-conn", state.Connections[0].Name) + assert.False(t, state.Connections[0].ManagedByDeploy) + assert.Equal( + t, + "CognitiveSearch | https://search.example", + state.Connections[0].Detail, + ) + + require.Len(t, state.Toolboxes, 1) + assert.Equal(t, "research-tools", state.Toolboxes[0].Name) + assert.True(t, state.Toolboxes[0].ManagedByDeploy) + + require.Len(t, state.MissingToolboxEndpoints, 1) + assert.Equal( + t, + "research-tools", + state.MissingToolboxEndpoints[0].Name, + ) + assert.True( + t, + state.MissingToolboxEndpoints[0].ManagedByDeploy, + ) + assert.Empty(t, state.MissingManualVars) +} + +func TestAssembleState_RecordsSplitResourceErrors(t *testing.T) { + t.Parallel() + + projectRoot := t.TempDir() + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{ + "broken-conn": { + Name: "broken-conn", + Host: aiConnectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "$ref": "./missing-connection.yaml", + }), + }, + "echo": { + Name: "echo", + Host: agentHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "kind": "hosted", + "name": "echo-agent", + }), + }, + }, + }, + values: map[string]string{ + "dev/" + projectEndpointVar: "https://example", + }, + } + + state, errs := assembleState(t.Context(), src) + + require.NotEmpty(t, errs) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains( + t, + state.ConnectionLoadErrors[0], + "missing-connection.yaml", + ) + assert.False(t, state.HasConnections) +} + func TestAssembleState_ManifestWalker_RootRelativePath(t *testing.T) { t.Parallel() @@ -180,13 +309,16 @@ func TestAssembleState_ManifestWalker_RejectsTraversal(t *testing.T) { state, errs := assembleState(context.Background(), src) - require.Empty(t, errs) + require.NotEmpty(t, errs) + assert.Contains(t, errs[0].Error(), "must not contain '..'") assert.False(t, state.HasModels) assert.False(t, state.HasToolboxes) assert.False(t, state.HasConnections) } -func TestAssembleState_ManifestWalker_MalformedManifestNoError(t *testing.T) { +func TestAssembleState_ManifestWalker_ReportsMalformedManifest( + t *testing.T, +) { t.Parallel() projectRoot := t.TempDir() @@ -202,7 +334,13 @@ func TestAssembleState_ManifestWalker_MalformedManifestNoError(t *testing.T) { }, } - state, _ := assembleState(context.Background(), src) + state, errs := assembleState(context.Background(), src) + + require.NotEmpty(t, errs) + require.Len(t, state.ModelLoadErrors, 1) + require.Len(t, state.ToolboxLoadErrors, 1) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ModelLoadErrors[0], "load resources for echo") assert.False(t, state.HasModels) assert.False(t, state.HasToolboxes) assert.False(t, state.HasConnections) @@ -375,11 +513,11 @@ func TestConnectionDetail(t *testing.T) { 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)) + assert.Equal( + t, + tc.want, + connectionDetail(tc.category, tc.target), + ) }) } } 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 4cb658aaea9..a27caf330f7 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 @@ -11,11 +11,10 @@ import ( ) const ( - // ProtocolInvocations is the value of `agent.yaml#protocol` for + // ProtocolInvocations identifies JSON-body invocation agents. // JSON-body /invocations agents. ProtocolInvocations = "invocations" - // ProtocolResponses is the value of `agent.yaml#protocol` for plain - // text /responses agents. + // ProtocolResponses identifies plain-text response agents. ProtocolResponses = "responses" // placeholderPayload is the single-quoted literal the resolver @@ -32,7 +31,7 @@ const ( // so the user has somewhere concrete to look for the real shape. placeholderPayload = `''` - // maxFixupLines caps the number of `azd env set` / `edit agent.yaml` + // maxFixupLines caps the number of configuration fix-up hints. // hints emitted by ResolveAfterInit per missing-input category so the // block stays scannable even when an agent declares many manual // variables or unresolved placeholders. @@ -45,11 +44,11 @@ const ( // Decision tree: // // - UnresolvedPlaceholders (always shown first when present, regardless -// of other branches) → one "edit agent.yaml: replace {{NAME}}" line +// of other branches) → one "edit azure.yaml" line // per unresolved Mustache placeholder (up to maxFixupLines). These // are deploy-time landmines: the literal `{{NAME}}` would otherwise // land in the container. They never reach `azd env set` because the -// value lives in agent.yaml itself, not the azd environment. +// value lives in agent configuration, not the azd environment. // // - len(PendingProvisionReasons) > 0 OR !HasProjectEndpoint OR // MissingInfraVars → required Azure-context `azd env set` fixups, @@ -60,11 +59,11 @@ const ( // project. When the endpoint is empty, provision has not yet // populated the infra outputs (typical path: user selected // "Deploy new models from the catalog" in init), so `azd -// provision` is the next step regardless of whether agent.yaml +// provision` is the next step regardless of whether agent config // directly references any Bicep-output variables. // MissingInfraVars is still consulted to cover the // post-provision re-provision case (a new `${VAR}` reference -// mapping to a Bicep output was added to agent.yaml after the +// mapping to a Bicep output was added to azure.yaml after the // last provision run). PendingProvisionReasons is the explicit // "init configured something provision still has to materialize" // signal — every reason tag (project, model_deployment, acr, @@ -76,25 +75,15 @@ const ( // // - MissingToolboxEndpoints OR MissingManualVars (or BOTH) → a // combined "fix things before you can run locally" branch. The two -// sub-branches are additive (not mutually exclusive) so a manifest +// sub-branches are additive (not mutually exclusive) so a project // that declares a toolbox AND references an unrelated manual var // (e.g. an API key) surfaces guidance for both — otherwise the // toolbox sub-branch would silently swallow the `azd env set` lines // and still emit `azd ai agent run`, leaving the user to discover // the unset manual var when the agent crashes. // -// Toolbox sub-branch: emits `azd provision` (with an -// `azd ai agent doctor` follow-up so the user can verify whether -// the toolbox already exists in their Foundry project before -// publishing a fresh version). Toolbox-derived endpoint vars -// (`TOOLBOX__MCP_ENDPOINT`) are azd-managed outputs of -// provision (see listen.go::registerToolboxEnvVars), not operator- -// supplied; suggesting `azd env set` for them would be misleading -// because the value is the URL of a Foundry resource that doesn't -// yet exist at this point in the lifecycle. The actual live -// existence check requires a Foundry API call and lives in -// `azd ai agent doctor`'s local.toolboxes check, not here — -// ResolveAfterInit is offline by contract. +// Toolbox sub-branch: emits `azd deploy` for split toolbox +// services or `azd provision` for a legacy manifest. // // Manual-vars sub-branch: one `azd env set ` per // missing operator-supplied var (up to maxFixupLines). Matches @@ -150,10 +139,9 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) }) } - // Placeholder fix-ups always come first when present: they are broken - // state in azure.yaml itself and block both `run` and `deploy`. The - // user has to edit azure.yaml (or define a matching parameter in - // agent.manifest.yaml) — `azd env set` cannot reach them. + // Placeholder fix-ups always come first when present: they block both + // `run` and `deploy`. The user has to edit the agent configuration; + // `azd env set` cannot reach them. hasPlaceholders := len(state.UnresolvedPlaceholders) > 0 if hasPlaceholders { placeholders := slices.Clone(state.UnresolvedPlaceholders) @@ -161,8 +149,8 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) limit := min(len(placeholders), maxFixupLines) for _, name := range placeholders[:limit] { out = append(out, Suggestion{ - Command: fmt.Sprintf("edit azure.yaml: replace {{%s}} with the actual value", name), - Description: "azure.yaml has unresolved manifest placeholders", + Command: fmt.Sprintf("edit the agent configuration: replace {{%s}} with the actual value", name), + Description: "agent configuration has unresolved manifest placeholders", Priority: priority, }) priority++ @@ -171,6 +159,12 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) hasToolboxEndpoints := len(state.MissingToolboxEndpoints) > 0 hasManualVars := len(state.MissingManualVars) > 0 + toolboxCommand := "" + toolboxDescription := "" + if hasToolboxEndpoints { + toolboxCommand, toolboxDescription = + toolboxSetupSuggestion(state.MissingToolboxEndpoints) + } needsProvision := len(state.PendingProvisionReasons) > 0 || !state.HasProjectEndpoint || @@ -194,30 +188,19 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) case hasToolboxEndpoints || hasManualVars: // Combined branch for the two "things the user has to fix before // running locally" categories. They are intentionally additive - // (not mutually exclusive) so a manifest that declares a + // (not mutually exclusive) so a project that declares a // toolbox AND references an unrelated manual var (e.g. an API // key) surfaces guidance for BOTH — otherwise the toolbox // branch would silently swallow the `azd env set` lines and // still emit `azd ai agent run`, leaving the user to discover // the unset manual var when the agent crashes. // - // Toolbox sub-branch: manifest declares one or more toolboxes - // whose azd-injected TOOLBOX__MCP_ENDPOINT variable is - // not yet present in the azd environment. The variable is - // written by `azd provision` (listen.go::registerToolboxEnvVars) - // after the FoundryToolboxClient publishes the toolbox version, - // so the canonical fix is provision — NOT `azd env set`, which - // the generic manual-vars sub-branch below would otherwise - // suggest. We also surface `azd ai agent doctor` as a follow-up - // so the user can check whether the toolbox already exists in - // their Foundry project. The actual live existence check - // belongs in doctor's local.toolboxes (one HTTP GET per - // toolbox); ResolveAfterInit is offline by contract and must - // not initiate Foundry API calls. + // Toolbox endpoints are azd-managed values. Split services use + // deploy; legacy manifest resources continue to use provision. if hasToolboxEndpoints { out = append(out, Suggestion{ - Command: "azd provision", - Description: "create your toolbox(es) in Foundry", + Command: toolboxCommand, + Description: toolboxDescription, Priority: priority, }) priority++ @@ -245,18 +228,20 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) priority++ } } - // Follow-up: once the user finishes the steps above (provision - // for toolboxes, env-set for manual vars), the next productive + // Once the setup command and env fixes complete, suggest run. // 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 + // literal `{{NAME}}` values in azure.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, + Command: "azd ai agent run", + Description: runFollowUpDescription( + toolboxCommand, + hasManualVars, + ), + Priority: priority, }) priority++ out, _ = appendInvokeLocalSecondary(out, state, readmeExists, priority) @@ -294,11 +279,27 @@ func ResolveAfterInit(state *State, readmeExists func(relativePath string) bool) // `azd ai agent run` follow-up emitted after the toolbox / manual-vars // branch, so the suffix reflects which categories of work the user // still has to complete first. -func runFollowUpDescription(hasToolboxEndpoints, hasManualVars bool) string { +func toolboxSetupSuggestion( + toolboxes []ResourceRef, +) (string, string) { + if slices.ContainsFunc(toolboxes, func(ref ResourceRef) bool { + return ref.ManagedByDeploy + }) { + return "azd deploy", "deploy your toolbox service(s) in Foundry" + } + return "azd provision", "create your toolbox(es) in Foundry" +} + +func runFollowUpDescription( + toolboxCommand string, + hasManualVars bool, +) string { switch { - case hasToolboxEndpoints && hasManualVars: + case toolboxCommand != "" && hasManualVars: return "start the agent locally once the steps above are complete" - case hasToolboxEndpoints: + case toolboxCommand == "azd deploy": + return "start the agent locally once deployment completes" + case toolboxCommand != "": return "start the agent locally once provision completes" case hasManualVars: return "start the agent locally once the values above are set" 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 231a60eb8cd..16c02101804 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 @@ -412,7 +412,7 @@ func TestResolveAfterInit_ToolboxReproRendersAllCategories(t *testing.T) { rendered := buf.String() assert.Contains(t, rendered, - "edit azure.yaml: replace {{TOOLBOX_ENDPOINT}} with the actual value", + "edit the agent configuration: 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") @@ -478,6 +478,31 @@ func TestResolveAfterInit_ToolboxEndpointsEmitsRunAndInvokeLocal(t *testing.T) { assert.Contains(t, rendered, "azd deploy", "trailing deploy reminder missing") } +func TestResolveAfterInit_SplitToolboxUsesDeploy(t *testing.T) { + t.Parallel() + + state := &State{ + HasProjectEndpoint: true, + MissingToolboxEndpoints: []ResourceRef{ + { + Name: "web-search-tools", + ServiceName: "web-search-tools", + ManagedByDeploy: true, + }, + }, + } + + suggestions := ResolveAfterInit(state, nil) + require.NotEmpty(t, suggestions) + assert.Equal(t, "azd deploy", suggestions[0].Command) + assert.Contains(t, suggestions[0].Description, "toolbox service") + assert.Contains( + t, + suggestions[2].Description, + "once deployment completes", + ) +} + // TestResolveAfterInit_ToolboxAndManualVarsCoexist locks the bug both // reviewers caught: when MissingToolboxEndpoints AND MissingManualVars // are populated, the previously-exclusive switch hid the manual @@ -552,7 +577,7 @@ func TestResolveAfterInit_ToolboxAndManualVarsCoexistWithPlaceholders(t *testing rendered := buf.String() assert.Contains(t, rendered, - "edit azure.yaml: replace {{AGENT_NAME}} with the actual value", + "edit the agent configuration: 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") @@ -575,21 +600,26 @@ func TestRunFollowUpDescription(t *testing.T) { t.Parallel() tests := []struct { - name string - hasToolboxEndpoint bool - hasManualVars bool - want string + name string + toolboxCommand string + hasManualVars bool + want string }{ { - name: "both", - hasToolboxEndpoint: true, - hasManualVars: true, - want: "start the agent locally once the steps above are complete", + name: "both", + toolboxCommand: "azd provision", + hasManualVars: true, + want: "start the agent locally once the steps above are complete", + }, + { + name: "legacy toolbox only", + toolboxCommand: "azd provision", + want: "start the agent locally once provision completes", }, { - name: "toolbox only", - hasToolboxEndpoint: true, - want: "start the agent locally once provision completes", + name: "split toolbox only", + toolboxCommand: "azd deploy", + want: "start the agent locally once deployment completes", }, { name: "manual only", @@ -607,7 +637,10 @@ func TestRunFollowUpDescription(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got := runFollowUpDescription(tc.hasToolboxEndpoint, tc.hasManualVars) + got := runFollowUpDescription( + tc.toolboxCommand, + tc.hasManualVars, + ) assert.Equal(t, tc.want, got) }) } @@ -693,7 +726,7 @@ func TestResolveAfterInit_UnresolvedPlaceholders(t *testing.T) { for i, name := range tt.wantPlaceholders { require.Less(t, i, len(out)) assert.Equal(t, - "edit azure.yaml: replace {{"+name+"}} with the actual value", + "edit the agent configuration: replace {{"+name+"}} with the actual value", out[i].Command, ) } 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 bf8bc5078c0..afc74504423 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,7 +8,6 @@ import ( "errors" "fmt" "maps" - "os" "path/filepath" "regexp" "slices" @@ -16,10 +15,9 @@ import ( "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" + "google.golang.org/protobuf/types/known/structpb" ) const ( @@ -64,7 +62,7 @@ const ( // envVarRefPattern captures ${VAR} references inside YAML 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 +// tail `:-fallback`; when group 2 is non-empty the configuration // 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 @@ -289,7 +287,7 @@ func assembleState(ctx context.Context, src Source, opts ...Option) (*State, []e } if project != nil && len(state.Services) > 0 { - populateManifestResources(project.Path, state) + populateResources(project, state, &errs) } // Partition toolbox-derived endpoint vars out of MissingManualVars @@ -426,25 +424,47 @@ func collectServices( ctx context.Context, src Source, envName string, - project *azdext.ProjectConfig, + projectConfig *azdext.ProjectConfig, errs *[]error, ) []ServiceState { - if project == nil || len(project.Services) == 0 { + if projectConfig == nil || len(projectConfig.Services) == 0 { return nil } - services := make([]ServiceState, 0, len(project.Services)) - for _, svc := range project.Services { + services := make([]ServiceState, 0, len(projectConfig.Services)) + for _, svc := range projectConfig.Services { if svc == nil || svc.Host != agentHost { continue } - services = append(services, ServiceState{ + + serviceState := ServiceState{ Name: svc.Name, Host: svc.Host, RelativePath: svc.RelativePath, - Protocol: loadServiceProtocol(project.Path, svc.RelativePath), IsDeployed: isDeployed(ctx, src, envName, svc.Name, errs), - }) + } + + agentDef, cfg, resolved, err := loadGuidanceServiceConfig( + svc, + projectConfig.Path, + ) + if err != nil { + *errs = append( + *errs, + fmt.Errorf("resolve service %s: %w", svc.Name, err), + ) + } else { + if serviceState.RelativePath == "" && resolved != nil { + serviceState.RelativePath = resolvedProjectPath(resolved) + } + serviceState.Protocol = preferredProtocol( + agentDef.Protocols, + ) + serviceState.EnvironmentValues = + agentEnvironmentValues(agentDef, &cfg) + } + + services = append(services, serviceState) } slices.SortFunc(services, func(a, b ServiceState) int { @@ -453,32 +473,11 @@ func collectServices( return services } -// 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 "" - } - var hosted agent_yaml.ContainerAgent - if err := yaml.Unmarshal(data, &hosted); err != nil { - return "" - } - +func preferredProtocol( + protocols []agent_yaml.ProtocolVersionRecord, +) string { sawInvocations := false - for _, p := range hosted.Protocols { + for _, p := range protocols { switch strings.TrimSpace(p.Protocol) { case ProtocolResponses: return ProtocolResponses @@ -492,20 +491,46 @@ 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: +func agentEnvironmentValues( + agentDef guidanceAgentDefinition, + cfg *guidanceServiceConfig, +) []string { + valuesByName := map[string]string{} + if agentDef.EnvironmentVariables != nil { + for _, envVar := range *agentDef.EnvironmentVariables { + valuesByName[envVar.Name] = envVar.Value + } + } + if cfg != nil { + maps.Copy(valuesByName, cfg.Environment) + } + values := make([]string, 0, len(valuesByName)) + for _, key := range slices.Sorted(maps.Keys(valuesByName)) { + values = append(values, valuesByName[key]) + } + return values +} + +func resolvedProjectPath(props *structpb.Struct) string { + value, ok := props.GetFields()["project"] + if !ok { + return "" + } + return value.GetStringValue() +} + +// detectMissingVars inspects each agent's environment values. // // 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) +// 3. placeholders: surviving {{NAME}} Mustache placeholders // // Only bare-form ${VAR} refs participate in (1) and (2): when the -// agent.yaml author supplies an explicit fallback via `${VAR:-default}`, +// a value 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. +// not required. `extractEnvironmentRefs` filters defaulted refs out. // // Classification rule for ${VAR}: a variable is an infra var iff its // name is declared as a top-level `output` in `/infra/ @@ -546,17 +571,22 @@ func detectMissingVars( services []ServiceState, errs *[]error, ) (infra, manual, placeholders []string) { - if envName == "" || projectPath == "" || len(services) == 0 { + if envName == "" || len(services) == 0 { return nil, nil, nil } - bicepOutputs := bicepOutputSet(projectPath) + bicepOutputs := map[string]struct{}{} + if projectPath != "" { + 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) + refs, phs := extractEnvironmentRefs( + svc.EnvironmentValues, + ) for _, name := range refs { if _, ok := seenInfra[name]; ok { continue @@ -603,49 +633,23 @@ func bicepOutputSet(projectPath string) map[string]struct{} { return set } -// extractAgentYamlEnvRefs returns two lists from the service's -// agent.yaml environment_variables block: +// extractEnvironmentRefs returns refs and placeholders from values. // // 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. +// 2. placeholders: unique {{NAME}} Mustache-style placeholders. // -// 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 - } - +// Order matches first appearance in the configured values. +func extractEnvironmentRefs( + values []string, +) (refs, placeholders []string) { seenRef := make(map[string]struct{}) seenPh := make(map[string]struct{}) - for _, ev := range *hosted.EnvironmentVariables { - for _, m := range envVarRefPattern.FindAllStringSubmatch(ev.Value, -1) { + for _, value := range values { + 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 - // to set the var. Skipping here keeps the next-step hint - // honest: only bare-form refs become missing-var prompts. continue } name := m[1] @@ -655,8 +659,14 @@ func extractAgentYamlEnvRefs(projectPath, relativePath string) (refs, placeholde seenRef[name] = struct{}{} refs = append(refs, name) } - for _, m := range placeholderPattern.FindAllStringSubmatch(ev.Value, -1) { - name := m[1] + for _, match := range placeholderPattern.FindAllStringSubmatchIndex( + value, + -1, + ) { + if match[0] > 0 && value[match[0]-1] == '$' { + continue + } + name := value[match[2]:match[3]] if _, ok := seenPh[name]; ok { continue } 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..bb452dd0acc 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 @@ -10,9 +10,13 @@ import ( "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" + "go.yaml.in/yaml/v3" + "google.golang.org/protobuf/types/known/structpb" ) // fakeSource is a hand-rolled Source for table-driven tests. @@ -25,6 +29,16 @@ type fakeSource struct { valueErr error } +func mustStruct( + t *testing.T, + values map[string]any, +) *structpb.Struct { + t.Helper() + result, err := structpb.NewStruct(values) + require.NoError(t, err) + return result +} + func (f *fakeSource) CurrentEnvName(_ context.Context) (string, error) { return f.envName, f.envNameErr } @@ -569,156 +583,263 @@ 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 []agent_yaml.ProtocolVersionRecord + want string }{ { name: "single responses protocol", - manifest: `kind: hostedAgent -protocols: - - protocol: responses - version: "1.0.0" -`, + protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: ProtocolResponses}, + }, want: ProtocolResponses, }, { name: "single invocations protocol", - manifest: `kind: hostedAgent -protocols: - - protocol: invocations - version: "1.0.0" -`, + protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: ProtocolInvocations}, + }, want: ProtocolInvocations, }, { name: "responses wins when both declared", - manifest: `kind: hostedAgent -protocols: - - protocol: invocations - version: "1.0.0" - - protocol: responses - version: "1.0.0" -`, + protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: ProtocolInvocations}, + {Protocol: ProtocolResponses}, + }, want: ProtocolResponses, }, { - name: "empty protocols section", - manifest: `kind: hostedAgent -protocols: [] -`, - want: "", + name: "empty protocols section", + protocols: nil, + want: "", }, { name: "unknown protocol value silently ignored", - manifest: `kind: hostedAgent -protocols: - - protocol: pigeon-mail - version: "1.0.0" -`, + protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: "pigeon-mail"}, + }, 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: "", - }, } 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) + got := preferredProtocol(tt.protocols) assert.Equal(t, tt.want, got) }) } } -func TestLoadServiceProtocol_EmptyArgs(t *testing.T) { - t.Parallel() - - assert.Equal(t, "", loadServiceProtocol("", "echo")) -} - -func TestLoadServiceProtocol_RootRelativePath(t *testing.T) { +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, "agent.yaml"), + filepath.Join(projectRoot, "echo", "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, ".")) + 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) + require.Len(t, state.Services, 1) + assert.Equal(t, ProtocolInvocations, state.Services[0].Protocol) } -func TestLoadServiceProtocol_RejectsTraversal(t *testing.T) { +func TestAssembleState_PopulatesInlineAgentConfig(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)) + projectRoot := t.TempDir() require.NoError(t, os.WriteFile( - filepath.Join(outside, "agent.yaml"), - []byte("kind: hostedAgent\nprotocols:\n - protocol: invocations\n version: \"1.0.0\"\n"), + filepath.Join(projectRoot, "azure.yaml"), + []byte(`name: inline-project +services: + echo: + host: azure.ai.agent + project: src/echo + kind: hosted + name: echo-agent + env: + API_KEY: ${API_KEY} + DEFAULT: ${OPTIONAL_KEY:-fallback} + PROJECT: ${{project.endpoint}} + COMPOSITE: prefix-${SECOND_KEY} + ENABLED: true + RETRIES: 3 +`), 0o600, )) + props := mustStruct(t, map[string]any{ + "kind": "hosted", + "name": "echo-agent", + "protocols": []any{ + map[string]any{ + "protocol": "responses", + "version": "1.0.0", + }, + }, + }) + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: projectRoot, + Services: map[string]*azdext.ServiceConfig{ + "echo": { + Name: "echo", + Host: agentHost, + RelativePath: "src/echo", + Environment: map[string]string{ + "API_KEY": "", + "DEFAULT": "fallback", + "PROJECT": "https://example", + "COMPOSITE": "prefix-set", + "ENABLED": "true", + "RETRIES": "3", + }, + AdditionalProperties: props, + }, + }, + }, + values: map[string]string{ + "dev/" + projectEndpointVar: "https://example", + "dev/SECOND_KEY": "set", + }, + } + + state, errs := assembleState(t.Context(), src) - assert.Equal(t, "", loadServiceProtocol(projectRoot, "../outside")) + require.Empty(t, errs) + require.Len(t, state.Services, 1) + assert.Equal(t, ProtocolResponses, state.Services[0].Protocol) + assert.Equal(t, "src/echo", state.Services[0].RelativePath) + assert.Equal(t, []string{"API_KEY"}, state.MissingManualVars) + assert.Empty(t, state.UnresolvedPlaceholders) } -func TestAssembleState_PopulatesProtocolFromAgentYaml(t *testing.T) { +func TestAssembleState_UnrelatedInlineFallsBackToConfig( + t *testing.T, +) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{ + "echo": { + Name: "echo", + Host: agentHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "resumeSessionOnDeploy": true, + }), + Config: mustStruct(t, map[string]any{ + "kind": "hosted", + "name": "echo-agent", + "protocols": []any{ + map[string]any{ + "protocol": "responses", + }, + }, + "deployments": []any{ + map[string]any{ + "name": "gpt-4o", + "model": map[string]any{ + "name": "gpt-4o", + }, + }, + }, + "connections": []any{ + map[string]any{ + "name": "legacy-connection", + }, + }, + "toolboxes": []any{ + map[string]any{ + "name": "legacy-toolbox", + }, + }, + }), + }, + }, + }, + values: map[string]string{ + "dev/" + projectEndpointVar: "https://example", + }, + } + + state, errs := assembleState(t.Context(), src) + + require.Empty(t, errs) + require.Len(t, state.Services, 1) + assert.Equal(t, ProtocolResponses, state.Services[0].Protocol) + require.Len(t, state.ModelRefs, 1) + require.Len(t, state.Connections, 1) + require.Len(t, state.Toolboxes, 1) +} + +func TestAssembleState_PopulatesReferencedAgentConfig(t *testing.T) { t.Parallel() projectRoot := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(projectRoot, "echo"), 0o750)) + definitionsDir := filepath.Join(projectRoot, "definitions") + require.NoError(t, os.MkdirAll(definitionsDir, 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"), + filepath.Join(definitionsDir, "echo.yaml"), + []byte( + "kind: hosted\n"+ + "name: echo-agent\n"+ + "project: ../src/echo\n"+ + "protocols:\n"+ + " - protocol: invocations\n"+ + " version: \"1.0.0\"\n"+ + "env:\n"+ + " API_KEY: ${API_KEY}\n", + ), 0o600, )) - src := &fakeSource{ envName: "dev", project: &azdext.ProjectConfig{ Path: projectRoot, Services: map[string]*azdext.ServiceConfig{ - "echo": {Name: "echo", Host: agentHost, RelativePath: "echo"}, + "echo": { + Name: "echo", + Host: agentHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "$ref": "./definitions/echo.yaml", + }), + }, }, }, + values: map[string]string{ + "dev/" + projectEndpointVar: "https://example", + }, } - state, errs := assembleState(context.Background(), src) + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) require.Len(t, state.Services, 1) assert.Equal(t, ProtocolInvocations, state.Services[0].Protocol) + assert.Equal(t, "src/echo", state.Services[0].RelativePath) + assert.Equal(t, []string{"API_KEY"}, state.MissingManualVars) } func TestExtractAgentYamlEnvRefs(t *testing.T) { @@ -797,6 +918,18 @@ environment_variables: `, wantRefs: nil, }, + { + name: "Foundry expressions are not placeholders", + manifest: `kind: hostedAgent +environment_variables: + - name: PROJECT + value: '${{project.endpoint}}' + - name: CONNECTION + value: '${{connections.search.credentials.key}}' +`, + wantRefs: nil, + wantPlaceholders: nil, + }, { name: "malformed yaml returns nil", manifest: "this: is: not: valid: yaml: at: all: [", @@ -894,73 +1027,38 @@ environment_variables: 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"), + var agentDef agent_yaml.ContainerAgent + if err := yaml.Unmarshal( []byte(tt.manifest), - 0o600, - )) - gotRefs, gotPlaceholders := extractAgentYamlEnvRefs(projectRoot, "echo") + &agentDef, + ); err != nil { + assert.Nil(t, tt.wantRefs) + assert.Nil(t, tt.wantPlaceholders) + return + } + var values []string + if agentDef.EnvironmentVariables != nil { + for _, envVar := range *agentDef.EnvironmentVariables { + values = append(values, envVar.Value) + } + } + gotRefs, gotPlaceholders := extractEnvironmentRefs(values) assert.Equal(t, tt.wantRefs, gotRefs, "refs") assert.Equal(t, tt.wantPlaceholders, gotPlaceholders, "placeholders") }) } } -func TestExtractAgentYamlEnvRefs_MissingFileOrArgs(t *testing.T) { +func TestExtractEnvironmentRefs_Empty(t *testing.T) { t.Parallel() - for _, args := range [][2]string{ - {"", "echo"}, - {t.TempDir(), ""}, - {t.TempDir(), "missing"}, - } { - refs, placeholders := extractAgentYamlEnvRefs(args[0], args[1]) + for _, values := range [][]string{nil, {}} { + refs, placeholders := extractEnvironmentRefs(values) 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() @@ -1524,3 +1622,20 @@ environment_variables: assert.Empty(t, state.MissingInfraVars) assert.Equal(t, []string{"MY_API_KEY"}, state.MissingManualVars) } + +func TestAgentEnvironmentValues_ConfigOverridesLegacyValue(t *testing.T) { + t.Parallel() + + values := agentEnvironmentValues( + guidanceAgentDefinition{ + EnvironmentVariables: &[]agent_yaml.EnvironmentVariable{ + {Name: "SHARED", Value: "${MISSING}"}, + }, + }, + &guidanceServiceConfig{ + Environment: map[string]string{"SHARED": "literal"}, + }, + ) + + assert.Equal(t, []string{"literal"}, values) +} 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..e0c928602d9 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 @@ -70,10 +70,8 @@ type State struct { // reason-tag taxonomy. PendingProvisionReasons []string - // MissingInfraVars names ${...} references in agent.yaml that map to - // Bicep outputs not yet present in the azd environment (i.e., - // provision is needed or has been skipped). Named so the resolver can - // surface an actionable hint. + // MissingInfraVars lists unresolved agent configuration values + // that map to infrastructure outputs. MissingInfraVars []string // MissingAzureContextVars names Azure environment values that must be @@ -85,38 +83,18 @@ type State struct { // MissingManualVars names ${...} references that map to user-supplied // variables which are not set in the azd environment. // - // Toolbox-derived endpoint variables (`TOOLBOX__MCP_ENDPOINT` - // keys that correspond to a manifest-declared toolbox) are - // partitioned out into MissingToolboxEndpoints — they are - // azd-managed outputs of `azd provision`, not operator-supplied, - // and routing them to `azd env set` is misleading. + // Toolbox endpoint variables are moved to + // MissingToolboxEndpoints because azd manages them. MissingManualVars []string - // MissingToolboxEndpoints lists manifest-declared toolboxes whose - // azd-injected TOOLBOX__MCP_ENDPOINT variable is unset in the - // active azd environment. AssembleState partitions these out of - // MissingManualVars because they are produced by - // `azd provision` (listen.go::registerToolboxEnvVars), not by the - // user — the right remediation is `azd provision` (which creates - // the toolbox in the Foundry project on first run and sets the - // derived env var), not `azd env set`. + // MissingToolboxEndpoints lists declared toolboxes whose endpoint + // variable is unset in the active azd environment. // - // Each entry carries the manifest's resource Name and the owning - // ServiceName so the resolver and doctor checks can render - // per-service guidance. The Detail field is unused (toolbox - // endpoints have no kind-specific identifier beyond Name) but the - // shared ResourceRef shape keeps the renderer code uniform with - // state.Toolboxes / state.ModelRefs / state.Connections. + // ResourceRef records whether deploy or provision owns setup. 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 lists literal {{NAME}} values in agent + // configuration. They require an azure.yaml edit. UnresolvedPlaceholders []string // Services is the per-service snapshot derived from azure.yaml plus @@ -144,54 +122,29 @@ 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. - // - // 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. + // HasModels, HasToolboxes, and HasConnections are aggregate + // resource-presence flags across unified and legacy configuration. 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. + // Resource lists are sorted and deduplicated by service and name. ModelRefs []ResourceRef Toolboxes []ResourceRef Connections []ResourceRef + + // Resource load errors let doctor reject incomplete snapshots. + ModelLoadErrors []string + ToolboxLoadErrors []string + ConnectionLoadErrors []string } -// 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 the resource summary used by guidance and doctor. 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 declared resource name. 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 azd service that owns the resource. ServiceName string // Detail carries a kind-specific identifier: @@ -201,6 +154,9 @@ type ResourceRef struct { // Doctor remediation messages render Detail verbatim, so changes // here must match the doctor-message contract. Detail string + + // ManagedByDeploy is true for split deploy-time resource services. + ManagedByDeploy bool } // ServiceState mirrors one entry from the project's services map, plus a @@ -210,9 +166,10 @@ type ResourceRef struct { // 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 - RelativePath string - IsDeployed bool + Name string + Host string + Protocol string + RelativePath string + EnvironmentValues []string + IsDeployed bool } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go index cb8d97e8100..e0aa456b9a5 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go @@ -207,6 +207,21 @@ func (a *OptimizeApplyAction) apply( } } } else { + // A $ref-backed definition is not detected as inline above, but + // its definition lives in a referenced file this command cannot + // safely round-trip. Fail with guidance rather than writing the + // wrong on-disk agent.yaml. + if projectpkg.ConfigContainsFileRef(svc.GetAdditionalProperties()) || + projectpkg.ConfigContainsFileRef(svc.GetConfig()) { + return fmt.Errorf( + "agent service %q defines its agent via $ref; "+ + "'optimize apply' cannot update a referenced file. "+ + "Add OPTIMIZATION_LOCAL_DIR and "+ + "OPTIMIZATION_CANDIDATE_ID to the referenced agent "+ + "file, or inline the definition in azure.yaml", + svc.Name, + ) + } agentYamlPath := filepath.Join(serviceDir, "agent.yaml") fmt.Fprintf(out, " Updating %s...\n", agentYamlPath) if err := upsertAgentYamlEnvVar(agentYamlPath, "OPTIMIZATION_LOCAL_DIR", agentConfigsDir); err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go index bce529919dd..074a0cd85de 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go @@ -13,6 +13,8 @@ import ( "azureaiagent/internal/project" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/foundry" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" ) @@ -355,10 +357,26 @@ func reserveServiceName(used map[string]string, name, source string) error { // service carries any, so an azure.yaml written before the per-resource split // still provisions without re-running init. func collectProjectDeployments(services map[string]*azdext.ServiceConfig) ([]project.Deployment, error) { + return collectProjectDeploymentsAtRoot(services, "") +} + +func collectProjectDeploymentsAtRoot( + services map[string]*azdext.ServiceConfig, + projectRoot string, +) ([]project.Deployment, error) { var out []project.Deployment for _, svc := range sortedServices(services) { - props := project.ServiceConfigProps(svc) - if svc.Host != AiProjectHost || props == nil { + if svc.Host != AiProjectHost { + continue + } + props, err := resolvedResourceServiceProps( + svc, + projectRoot, + ) + if err != nil { + return nil, err + } + if props == nil { continue } var cfg *project.ServiceTargetAgentConfig @@ -372,7 +390,10 @@ func collectProjectDeployments(services map[string]*azdext.ServiceConfig) ([]pro if len(out) > 0 { return out, nil } - legacy, err := collectLegacyAgentConfigs(services) + legacy, err := collectLegacyAgentConfigsAtRoot( + services, + projectRoot, + ) if err != nil { return nil, err } @@ -387,10 +408,26 @@ func collectProjectDeployments(services map[string]*azdext.ServiceConfig) ([]pro // agent service when no connection service carries any, so a pre-split // azure.yaml still provisions without re-running init. func collectConnections(services map[string]*azdext.ServiceConfig) ([]project.Connection, error) { + return collectConnectionsAtRoot(services, "") +} + +func collectConnectionsAtRoot( + services map[string]*azdext.ServiceConfig, + projectRoot string, +) ([]project.Connection, error) { var out []project.Connection for _, svc := range sortedServices(services) { - props := project.ServiceConfigProps(svc) - if svc.Host != AiConnectionHost || props == nil { + if svc.Host != AiConnectionHost { + continue + } + props, err := resolvedResourceServiceProps( + svc, + projectRoot, + ) + if err != nil { + return nil, err + } + if props == nil { continue } var conn *project.Connection @@ -398,13 +435,19 @@ func collectConnections(services map[string]*azdext.ServiceConfig) ([]project.Co return nil, fmt.Errorf("parsing connection service %q config: %w", svc.Name, err) } if conn != nil { + if conn.Name == "" { + conn.Name = svc.Name + } out = append(out, *conn) } } if len(out) > 0 { return out, nil } - legacy, err := collectLegacyAgentConfigs(services) + legacy, err := collectLegacyAgentConfigsAtRoot( + services, + projectRoot, + ) if err != nil { return nil, err } @@ -419,10 +462,26 @@ func collectConnections(services map[string]*azdext.ServiceConfig) ([]project.Co // toolbox service carries any, so a pre-split azure.yaml still provisions // without re-running init. func collectToolboxes(services map[string]*azdext.ServiceConfig) ([]project.Toolbox, error) { + return collectToolboxesAtRoot(services, "") +} + +func collectToolboxesAtRoot( + services map[string]*azdext.ServiceConfig, + projectRoot string, +) ([]project.Toolbox, error) { var out []project.Toolbox for _, svc := range sortedServices(services) { - props := project.ServiceConfigProps(svc) - if svc.Host != AiToolboxHost || props == nil { + if svc.Host != AiToolboxHost { + continue + } + props, err := resolvedResourceServiceProps( + svc, + projectRoot, + ) + if err != nil { + return nil, err + } + if props == nil { continue } var toolbox *project.Toolbox @@ -430,13 +489,19 @@ func collectToolboxes(services map[string]*azdext.ServiceConfig) ([]project.Tool return nil, fmt.Errorf("parsing toolbox service %q config: %w", svc.Name, err) } if toolbox != nil { + if toolbox.Name == "" { + toolbox.Name = svc.Name + } out = append(out, *toolbox) } } if len(out) > 0 { return out, nil } - legacy, err := collectLegacyAgentConfigs(services) + legacy, err := collectLegacyAgentConfigsAtRoot( + services, + projectRoot, + ) if err != nil { return nil, err } @@ -468,6 +533,13 @@ func collectAgentToolConnections(services map[string]*azdext.ServiceConfig) ([]p // connections, and toolboxes here rather than in sibling azure.ai. // services, so the collectors fall back to these when no sibling service exists. func collectLegacyAgentConfigs(services map[string]*azdext.ServiceConfig) ([]*project.ServiceTargetAgentConfig, error) { + return collectLegacyAgentConfigsAtRoot(services, "") +} + +func collectLegacyAgentConfigsAtRoot( + services map[string]*azdext.ServiceConfig, + projectRoot string, +) ([]*project.ServiceTargetAgentConfig, error) { var out []*project.ServiceTargetAgentConfig for _, svc := range sortedServices(services) { if svc.Host != AiAgentHost { @@ -476,7 +548,20 @@ func collectLegacyAgentConfigs(services map[string]*azdext.ServiceConfig) ([]*pr if project.ServiceConfigProps(svc) == nil { continue } - cfg, err := project.LoadServiceTargetAgentConfig(svc) + effective := proto.Clone(svc).(*azdext.ServiceConfig) + if projectRoot != "" { + if err := project.ResolveServiceConfigInPlace( + effective, + projectRoot, + ); err != nil { + return nil, fmt.Errorf( + "resolving agent service %q config: %w", + svc.Name, + err, + ) + } + } + cfg, err := project.LoadServiceTargetAgentConfig(effective) if err != nil { return nil, fmt.Errorf("parsing agent service %q config: %w", svc.Name, err) } @@ -487,6 +572,36 @@ func collectLegacyAgentConfigs(services map[string]*azdext.ServiceConfig) ([]*pr return out, nil } +func resolvedResourceServiceProps( + svc *azdext.ServiceConfig, + projectRoot string, +) (*structpb.Struct, error) { + props := project.ServiceConfigProps(svc) + if props == nil || projectRoot == "" { + return props, nil + } + resolved, err := foundry.ResolveFileRefs( + props.AsMap(), + projectRoot, + ) + if err != nil { + return nil, fmt.Errorf( + "resolving service %q config: %w", + svc.Name, + err, + ) + } + out, err := structpb.NewStruct(resolved) + if err != nil { + return nil, fmt.Errorf( + "encoding service %q config: %w", + svc.Name, + err, + ) + } + return out, nil +} + // sortedServices returns the services ordered by their map key so callers that // serialize collected resources produce deterministic output across runs. func sortedServices(services map[string]*azdext.ServiceConfig) []*azdext.ServiceConfig { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go index 0cfd1caacfc..97686db75fa 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go @@ -6,6 +6,8 @@ package cmd import ( "context" "net" + "os" + "path/filepath" "sync" "testing" @@ -15,6 +17,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/structpb" ) func mustMarshalConfig[T any](t *testing.T, in *T) *azdext.ServiceConfig { @@ -143,6 +146,70 @@ func TestCollectToolboxes(t *testing.T) { require.Len(t, toolboxes[0].Tools, 1) } +func TestCollectResourceServices_ResolvesFileRefs(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "deployment.yaml"), + []byte( + "name: gpt-4o\n"+ + "model: {name: gpt-4o}\n", + ), + 0o600, + )) + require.NoError(t, os.WriteFile( + filepath.Join(root, "project.yaml"), + []byte( + "deployments:\n"+ + " - $ref: ./deployment.yaml\n", + ), + 0o600, + )) + require.NoError(t, os.WriteFile( + filepath.Join(root, "connection.yaml"), + []byte( + "category: ApiKey\n"+ + "target: https://example.test\n", + ), + 0o600, + )) + projectProps, err := structpb.NewStruct(map[string]any{ + "$ref": "./project.yaml", + }) + require.NoError(t, err) + connectionProps, err := structpb.NewStruct(map[string]any{ + "$ref": "./connection.yaml", + }) + require.NoError(t, err) + services := map[string]*azdext.ServiceConfig{ + "ai-project": { + Name: "ai-project", + Host: AiProjectHost, + AdditionalProperties: projectProps, + }, + "search": { + Name: "search", + Host: AiConnectionHost, + AdditionalProperties: connectionProps, + }, + } + + deployments, err := collectProjectDeploymentsAtRoot( + services, + root, + ) + require.NoError(t, err) + require.Len(t, deployments, 1) + assert.Equal(t, "gpt-4o", deployments[0].Name) + + connections, err := collectConnectionsAtRoot(services, root) + require.NoError(t, err) + require.Len(t, connections, 1) + assert.Equal(t, "search", connections[0].Name) + assert.Equal(t, "ApiKey", connections[0].Category) +} + // TestCollectAgentToolConnections verifies tool connections stay on the agent // service and are sourced from there for toolbox enrichment. func TestCollectAgentToolConnections(t *testing.T) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go index 9e93521ea1f..e55e978263a 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go @@ -205,20 +205,36 @@ func runRun(ctx context.Context, flags *runFlags, noPrompt bool) error { fmt.Fprintf(os.Stderr, "Warning: failed to load azd environment values: %s\n", err) } - // Resolve environment_variables from the agent definition (agent.yaml). - // This handles hardcoded values, ${VAR} references (resolved via azd env), - // and ${{connections..credentials.}} references (resolved via - // the Foundry data plane). Agent definition env vars do not override - // values already present in the process environment. endpoint, _ := resolveAgentEndpoint(ctx, "", "") defEnv, defErr := resolveAgentDefinitionEnvVars(ctx, runCtx.Definition, azdEnvVars, endpoint) if defErr != nil { fmt.Fprintf(os.Stderr, "Warning: %s\n", defErr) } - for _, entry := range defEnv { - key, _, _ := strings.Cut(entry, "=") + serviceEnv, serviceEnvErr := resolveServiceEnvironmentVars( + ctx, + runCtx.Environment, + azdEnvVars, + endpoint, + ) + if serviceEnvErr != nil { + fmt.Fprintf(os.Stderr, "Warning: %s\n", serviceEnvErr) + } + configuredEnv := map[string]string{} + for _, entry := range append(defEnv, serviceEnv...) { + key, value, _ := strings.Cut(entry, "=") + configuredEnv[key] = value + } + keys := make([]string, 0, len(configuredEnv)) + for key := range configuredEnv { + keys = append(keys, key) + } + slices.Sort(keys) + for _, key := range keys { if !envSliceHasKey(env, key) { - env = append(env, entry) + env = append( + env, + fmt.Sprintf("%s=%s", key, configuredEnv[key]), + ) } } @@ -561,6 +577,45 @@ func resolveAgentDefinitionEnvVars( return result, nil } +func resolveServiceEnvironmentVars( + ctx context.Context, + values map[string]string, + azdEnvVars map[string]string, + endpoint string, +) ([]string, error) { + if len(values) == 0 { + return nil, nil + } + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + slices.Sort(keys) + envVars := make([]agent_yaml.EnvironmentVariable, 0, len(keys)) + for _, key := range keys { + value := values[key] + if endpoint != "" { + value = strings.ReplaceAll( + value, + "${{project.endpoint}}", + endpoint, + ) + } + envVars = append(envVars, agent_yaml.EnvironmentVariable{ + Name: key, + Value: value, + }) + } + return resolveAgentDefinitionEnvVars( + ctx, + &agent_yaml.ContainerAgent{ + EnvironmentVariables: &envVars, + }, + azdEnvVars, + endpoint, + ) +} + // findAgentYaml locates the agent definition file in the given directory. // After `azd ai agent init`, agent.yaml (and azure.yaml) are the sources of // truth for the agent configuration. We intentionally do not look at diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go index c57cee03665..c373ecf7086 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go @@ -993,6 +993,35 @@ environment_variables: }) } +func TestResolveServiceEnvironmentVars(t *testing.T) { + t.Parallel() + + result, err := resolveServiceEnvironmentVars( + t.Context(), + map[string]string{ + "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}/agents", + "PROJECT": "${{project.endpoint}}", + "STATIC": "value", + }, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://example", + }, + "https://example/project", + ) + + if err != nil { + t.Fatalf("resolve service environment: %v", err) + } + want := []string{ + "ENDPOINT=https://example/agents", + "PROJECT=https://example/project", + "STATIC=value", + } + if !slices.Equal(want, result) { + t.Fatalf("expected %v, got %v", want, result) + } +} + func TestVenvPip(t *testing.T) { t.Parallel() diff --git a/cli/azd/extensions/azure.ai.agents/internal/pkg/projectconfig/environment.go b/cli/azd/extensions/azure.ai.agents/internal/pkg/projectconfig/environment.go new file mode 100644 index 00000000000..0f4898f0f79 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/pkg/projectconfig/environment.go @@ -0,0 +1,127 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package projectconfig + +import ( + "fmt" + "os" + "path/filepath" + "reflect" + + "github.com/azure/azure-dev/cli/azd/pkg/foundry" + "go.yaml.in/yaml/v3" +) + +// LoadServiceEnvironment reads raw env values from azure.yaml. +func LoadServiceEnvironment( + projectRoot string, + serviceName string, +) (map[string]string, error) { + if projectRoot == "" || serviceName == "" { + return nil, nil + } + + data, path, err := readProjectFile(projectRoot) + if err != nil { + return nil, err + } + if len(data) == 0 { + return nil, nil + } + + var document struct { + Services map[string]map[string]any `yaml:"services"` + } + if err := yaml.Unmarshal(data, &document); err != nil { + return nil, fmt.Errorf("parse %s: %w", path, err) + } + entry := document.Services[serviceName] + if entry == nil { + return nil, nil + } + resolved, err := foundry.ResolveFileRefs(entry, projectRoot) + if err != nil { + return nil, fmt.Errorf( + "resolve service %q config: %w", + serviceName, + err, + ) + } + if err := NormalizeEnvironment(resolved); err != nil { + return nil, fmt.Errorf("service %q: %w", serviceName, err) + } + value, exists := resolved["env"] + if !exists { + return nil, nil + } + raw, ok := value.(map[string]any) + if !ok { + return nil, fmt.Errorf( + "service %q env must be a mapping", + serviceName, + ) + } + + env := make(map[string]string, len(raw)) + for key, value := range raw { + text, err := scalarString(value) + if err != nil { + return nil, fmt.Errorf( + "service %q env %q: %w", + serviceName, + key, + err, + ) + } + env[key] = text + } + return env, nil +} + +// NormalizeEnvironment converts scalar env values to strings in-place. +func NormalizeEnvironment(properties map[string]any) error { + value, exists := properties["env"] + if !exists { + return nil + } + raw, ok := value.(map[string]any) + if !ok { + return fmt.Errorf("env must be a mapping") + } + for key, value := range raw { + text, err := scalarString(value) + if err != nil { + return fmt.Errorf("env %q: %w", key, err) + } + raw[key] = text + } + return nil +} + +func readProjectFile(projectRoot string) ([]byte, string, error) { + for _, name := range []string{"azure.yaml", "azure.yml"} { + path := filepath.Join(projectRoot, name) + data, err := os.ReadFile(path) //nolint:gosec + if err == nil { + return data, path, nil + } + if !os.IsNotExist(err) { + return nil, path, fmt.Errorf("read %s: %w", path, err) + } + } + return nil, "", nil +} + +func scalarString(value any) (string, error) { + if value == nil { + return "", nil + } + kind := reflect.TypeOf(value).Kind() + if kind == reflect.Map || + kind == reflect.Slice || + kind == reflect.Array { + return "", fmt.Errorf("must be a scalar") + } + return fmt.Sprint(value), nil +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/pkg/projectconfig/environment_test.go b/cli/azd/extensions/azure.ai.agents/internal/pkg/projectconfig/environment_test.go new file mode 100644 index 00000000000..b59ad3b2639 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/pkg/projectconfig/environment_test.go @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package projectconfig + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadServiceEnvironment(t *testing.T) { + t.Parallel() + + t.Run("preserves expressions and converts scalars", func(t *testing.T) { + t.Parallel() + root := t.TempDir() + writeProjectFile(t, root, `services: + agent: + host: azure.ai.agent + env: + PROJECT: ${{project.endpoint}} + ENABLED: true + RETRIES: 3 + RATIO: 1.5 + EMPTY: +`) + + env, err := LoadServiceEnvironment(root, "agent") + + require.NoError(t, err) + assert.Equal(t, "${{project.endpoint}}", env["PROJECT"]) + assert.Equal(t, "true", env["ENABLED"]) + assert.Equal(t, "3", env["RETRIES"]) + assert.Equal(t, "1.5", env["RATIO"]) + assert.Equal(t, "", env["EMPTY"]) + }) + + t.Run("resolves service file references", func(t *testing.T) { + t.Parallel() + root := t.TempDir() + writeProjectFile(t, root, `services: + agent: + $ref: ./agent.yaml +`) + require.NoError(t, os.WriteFile( + filepath.Join(root, "agent.yaml"), + []byte(`host: azure.ai.agent +env: + CONNECTION: ${{connections.search.credentials.key}} +`), + 0o600, + )) + + env, err := LoadServiceEnvironment(root, "agent") + + require.NoError(t, err) + assert.Equal( + t, + "${{connections.search.credentials.key}}", + env["CONNECTION"], + ) + }) + + for _, test := range []struct { + name string + value string + }{ + {"map", " BAD:\n nested: value\n"}, + {"sequence", " BAD:\n - value\n"}, + } { + t.Run("rejects "+test.name, func(t *testing.T) { + t.Parallel() + root := t.TempDir() + writeProjectFile(t, root, "services:\n"+ + " agent:\n"+ + " host: azure.ai.agent\n"+ + " env:\n"+ + test.value) + + _, err := LoadServiceEnvironment(root, "agent") + + require.ErrorContains(t, err, "must be a scalar") + }) + } +} + +func writeProjectFile(t *testing.T, root string, contents string) { + t.Helper() + require.NoError(t, os.WriteFile( + filepath.Join(root, "azure.yaml"), + []byte(contents), + 0o600, + )) +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go index 0d97d24c693..7ddd51a3968 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go @@ -13,8 +13,10 @@ import ( "azureaiagent/internal/exterrors" "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/paths" + "azureaiagent/internal/pkg/projectconfig" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/foundry" "github.com/braydonk/yaml" "google.golang.org/protobuf/types/known/structpb" ) @@ -167,7 +169,8 @@ func LoadAgentDefinition( svc *azdext.ServiceConfig, projectRoot string, ) (agent_yaml.ContainerAgent, bool, AgentDefinitionSource, error) { - ca, isHosted, found, source, err := AgentDefinitionFromService(svc) + ca, isHosted, found, source, err := + AgentDefinitionFromResolvedService(svc, projectRoot) if err != nil { return agent_yaml.ContainerAgent{}, false, source, err } @@ -175,10 +178,67 @@ func LoadAgentDefinition( return ca, isHosted, source, nil } - // Fall back to a legacy agent.yaml/agent.yml on disk. return agentDefinitionFromDisk(svc, projectRoot) } +// AgentDefinitionFromResolvedService expands local file includes. +func AgentDefinitionFromResolvedService( + svc *azdext.ServiceConfig, + projectRoot string, +) ( + agent_yaml.ContainerAgent, + bool, + bool, + AgentDefinitionSource, + error, +) { + candidates := []struct { + props *structpb.Struct + source AgentDefinitionSource + }{ + {svc.GetAdditionalProperties(), AgentDefinitionSourceInline}, + {svc.GetConfig(), AgentDefinitionSourceLegacyConfig}, + } + for _, candidate := range candidates { + if candidate.props == nil || + len(candidate.props.GetFields()) == 0 { + continue + } + resolved, err := resolveServiceProps( + candidate.props, + svc.GetName(), + projectRoot, + ) + if err != nil { + return agent_yaml.ContainerAgent{}, + false, + false, + candidate.source, + err + } + if !structHasKind(resolved) { + continue + } + image := svc.GetImage() + if image == "" { + if value := resolved.GetFields()["image"]; value != nil { + image = value.GetStringValue() + } + } + ca, isHosted, err := agentDefinitionFromStruct( + resolved, + image, + ) + return ca, isHosted, true, candidate.source, err + } + + return agent_yaml.ContainerAgent{}, + false, + false, + AgentDefinitionSourceInline, + nil +} + // AgentDefinitionFromService returns the agent definition carried inline on the // service entry — the unified service-level shape, or the deprecated // config-nested shape. found is false when the entry carries no inline @@ -223,11 +283,145 @@ func LoadServiceTargetAgentConfig(svc *azdext.ServiceConfig) (*ServiceTargetAgen // which shape a project uses. func ServiceConfigProps(svc *azdext.ServiceConfig) *structpb.Struct { if s := svc.GetAdditionalProperties(); s != nil && len(s.GetFields()) > 0 { + if svc.GetHost() == "azure.ai.agent" && + !structHasKind(s) && + structHasKind(svc.GetConfig()) { + return svc.GetConfig() + } return s } return svc.GetConfig() } +// ResolveServiceConfigProps expands local $ref file includes. +func ResolveServiceConfigProps( + svc *azdext.ServiceConfig, + projectRoot string, +) (*structpb.Struct, error) { + props := ServiceConfigProps(svc) + if props == nil { + return nil, nil + } + return resolveServiceProps(props, svc.GetName(), projectRoot) +} + +// ResolvedServiceProjectPath returns the service source path. +func ResolvedServiceProjectPath( + svc *azdext.ServiceConfig, + projectRoot string, +) (string, error) { + if svc.GetRelativePath() != "" { + return svc.GetRelativePath(), nil + } + for _, props := range []*structpb.Struct{ + svc.GetAdditionalProperties(), + svc.GetConfig(), + } { + if props == nil || len(props.GetFields()) == 0 { + continue + } + resolved, err := resolveServiceProps( + props, + svc.GetName(), + projectRoot, + ) + if err != nil { + return "", err + } + if value := resolved.GetFields()["project"]; value != nil { + if path := value.GetStringValue(); path != "" { + return path, nil + } + } + } + return "", nil +} + +// ResolveServiceConfigInPlace expands service-level local includes. +func ResolveServiceConfigInPlace( + svc *azdext.ServiceConfig, + projectRoot string, +) error { + if props := svc.GetAdditionalProperties(); props != nil && + len(props.GetFields()) > 0 { + resolved, err := resolveServiceProps( + props, + svc.GetName(), + projectRoot, + ) + if err != nil { + return err + } + svc.AdditionalProperties = resolved + hydrateResolvedServiceFields(svc, resolved) + } + if config := svc.GetConfig(); config != nil && + len(config.GetFields()) > 0 { + resolved, err := resolveServiceProps( + config, + svc.GetName(), + projectRoot, + ) + if err != nil { + return err + } + svc.Config = resolved + hydrateResolvedServiceFields(svc, resolved) + } + return nil +} + +func hydrateResolvedServiceFields( + svc *azdext.ServiceConfig, + props *structpb.Struct, +) { + if svc.GetRelativePath() == "" { + if value := props.GetFields()["project"]; value != nil { + svc.RelativePath = value.GetStringValue() + } + } + if svc.GetImage() == "" { + if value := props.GetFields()["image"]; value != nil { + svc.Image = value.GetStringValue() + } + } +} + +func resolveServiceProps( + props *structpb.Struct, + serviceName string, + projectRoot string, +) (*structpb.Struct, error) { + resolved, err := foundry.ResolveFileRefs( + props.AsMap(), + projectRoot, + ) + if err != nil { + return nil, fmt.Errorf( + "resolving service %q config: %w", + serviceName, + err, + ) + } + if err := projectconfig.NormalizeEnvironment(resolved); err != nil { + return nil, fmt.Errorf( + "normalizing service %q environment: %w", + serviceName, + err, + ) + } + + out, err := structpb.NewStruct(resolved) + if err != nil { + return nil, fmt.Errorf( + "encoding resolved service %q config: %w", + serviceName, + err, + ) + } + return out, nil +} + // UpsertAgentEnvVars adds or updates environment variables on the agent // definition carried inline on the service entry, preserving every other key. // It is used by commands that mutate the definition (e.g. `optimize apply`). @@ -329,6 +523,23 @@ func agentDefinitionFromStruct(s *structpb.Struct, coreImage string) (agent_yaml } if inline.Kind != agent_yaml.AgentKindHosted { + // Validate non-hosted kinds (e.g. workflow) with the same + // generic rules the on-disk path uses, so an invalid kind or + // name is rejected instead of silently passing as "not a + // container agent". + if defBytes, marshalErr := yaml.Marshal(s.AsMap()); marshalErr != nil { + log.Printf( + "[debug] skipping non-hosted agent validation: %v", + marshalErr, + ) + } else if err := agent_yaml.ValidateAgentDefinition(defBytes); err != nil { + return agent_yaml.ContainerAgent{}, false, exterrors.Validation( + exterrors.CodeInvalidAgentManifest, + fmt.Sprintf("agent service definition is not valid: %s", err), + "fix the agent service entry in azure.yaml or "+ + "re-run `azd ai agent init`", + ) + } return agent_yaml.ContainerAgent{}, false, nil } diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go index 821f8ad7e18..835ae167965 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go @@ -12,6 +12,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" ) // sampleContainerAgent returns a hosted ContainerAgent with the fields that the @@ -110,6 +111,43 @@ func TestAgentDefinitionFromService_LegacyConfigShape(t *testing.T) { require.Equal(t, "basic-agent", got.Name) } +func TestLoadAgentDefinition_UnrelatedInlineFallsBackToConfig( + t *testing.T, +) { + t.Parallel() + + config, err := AgentDefinitionToServiceProperties( + sampleContainerAgent(), + &ServiceTargetAgentConfig{ + StartupCommand: "python main.py", + }, + ) + require.NoError(t, err) + inline, err := structpb.NewStruct(map[string]any{ + "resumeSessionOnDeploy": true, + }) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: "azure.ai.agent", + AdditionalProperties: inline, + Config: config, + } + + got, isHosted, source, err := LoadAgentDefinition( + svc, + t.TempDir(), + ) + + require.NoError(t, err) + require.True(t, isHosted) + require.Equal(t, AgentDefinitionSourceLegacyConfig, source) + require.Equal(t, "basic-agent", got.Name) + serviceConfig, err := LoadServiceTargetAgentConfig(svc) + require.NoError(t, err) + require.Equal(t, "python main.py", serviceConfig.StartupCommand) +} + // TestAgentDefinitionFromService_NoDefinition verifies that a service without an // inline definition reports not-found (callers then fall back to disk). func TestAgentDefinitionFromService_NoDefinition(t *testing.T) { @@ -192,6 +230,34 @@ func TestAgentDefinitionFromService_InvalidDefinition(t *testing.T) { require.Error(t, err) } +func TestLoadAgentDefinition_ToolboxServiceReference(t *testing.T) { + t.Parallel() + + props, err := structpb.NewStruct(map[string]any{ + "kind": "hosted", + "name": "basic-agent", + "toolboxes": []any{"research-tools"}, + }) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: "azure.ai.agent", + AdditionalProperties: props, + } + + _, isHosted, _, err := LoadAgentDefinition( + svc, + t.TempDir(), + ) + require.NoError(t, err) + require.True(t, isHosted) + + cfg, err := LoadServiceTargetAgentConfig(svc) + require.NoError(t, err) + require.Len(t, cfg.Toolboxes, 1) + require.Equal(t, "research-tools", cfg.Toolboxes[0].Name) +} + // TestLoadAgentDefinition_DiskFallback verifies the legacy on-disk agent.yaml // fallback used during the migration window. func TestLoadAgentDefinition_DiskFallback(t *testing.T) { @@ -208,6 +274,97 @@ func TestLoadAgentDefinition_DiskFallback(t *testing.T) { require.Equal(t, "disk-agent", got.Name) } +func TestLoadAgentDefinition_FileRef(t *testing.T) { + dir := t.TempDir() + definitionsDir := filepath.Join(dir, "definitions") + require.NoError(t, os.MkdirAll(definitionsDir, 0o700)) + require.NoError(t, os.WriteFile( + filepath.Join(definitionsDir, "agent.yaml"), + []byte( + "kind: hosted\n"+ + "name: referenced-agent\n"+ + "project: ../src/agent\n"+ + "image: registry.example/agent:v1\n"+ + "startupCommand: python main.py\n"+ + "protocols:\n"+ + " - protocol: responses\n"+ + " version: \"1.0.0\"\n", + ), + 0o600, + )) + + props, err := structpb.NewStruct(map[string]any{ + "$ref": "./definitions/agent.yaml", + "name": "overlay-agent", + }) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "agent-service", + Host: "azure.ai.agent", + AdditionalProperties: props, + } + + got, isHosted, source, err := LoadAgentDefinition(svc, dir) + require.NoError(t, err) + require.True(t, isHosted) + require.Equal(t, AgentDefinitionSourceInline, source) + require.Equal(t, "overlay-agent", got.Name) + require.Equal(t, "responses", got.Protocols[0].Protocol) + require.Equal(t, "registry.example/agent:v1", got.Image) + + projectPath, err := ResolvedServiceProjectPath(svc, dir) + require.NoError(t, err) + require.Equal(t, "src/agent", projectPath) + + require.NoError(t, ResolveServiceConfigInPlace(svc, dir)) + _, hasRef := svc.GetAdditionalProperties().GetFields()["$ref"] + require.False(t, hasRef) + cfg, err := LoadServiceTargetAgentConfig(svc) + require.NoError(t, err) + require.Equal(t, "python main.py", cfg.StartupCommand) + require.Equal(t, "registry.example/agent:v1", svc.GetImage()) +} + +// TestLoadAgentDefinition_InlineWorkflow verifies a valid inline +// `kind: workflow` definition loads without error and is reported as a +// non-hosted (non-container) agent rather than being rejected. +func TestLoadAgentDefinition_InlineWorkflow(t *testing.T) { + props, err := structpb.NewStruct(map[string]any{ + "kind": "workflow", + "name": "my-workflow", + }) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "my-workflow", + Host: "azure.ai.agent", + AdditionalProperties: props, + } + + _, isHosted, source, err := LoadAgentDefinition(svc, t.TempDir()) + require.NoError(t, err) + require.False(t, isHosted) + require.Equal(t, AgentDefinitionSourceInline, source) +} + +// TestLoadAgentDefinition_InlineWorkflow_InvalidRejected verifies an +// inline non-hosted definition is still validated: an invalid name is +// rejected rather than silently passing as "not a container agent". +func TestLoadAgentDefinition_InlineWorkflow_InvalidRejected(t *testing.T) { + props, err := structpb.NewStruct(map[string]any{ + "kind": "workflow", + "name": "Invalid_Name", + }) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "wf", + Host: "azure.ai.agent", + AdditionalProperties: props, + } + + _, _, _, err = LoadAgentDefinition(svc, t.TempDir()) + require.Error(t, err) +} + // TestUpsertAgentEnvVars verifies that env vars are added/updated on the inline // definition while preserving the other definition keys. func TestUpsertAgentEnvVars(t *testing.T) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/config.go b/cli/azd/extensions/azure.ai.agents/internal/project/config.go index b37b190409e..8a581fc73a8 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/config.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/config.go @@ -113,6 +113,22 @@ type Toolbox struct { Tools []map[string]any `json:"tools"` } +// UnmarshalJSON accepts split-service names and legacy objects. +func (t *Toolbox) UnmarshalJSON(data []byte) error { + var name string + if err := json.Unmarshal(data, &name); err == nil { + t.Name = name + return nil + } + type toolbox Toolbox + var value toolbox + if err := json.Unmarshal(data, &value); err != nil { + return err + } + *t = Toolbox(value) + return nil +} + // MemoryStore represents a Foundry memory store provisioned (create-if-not-exists) // during deployment. It backs the agent's memory_search tool, letting the agent retain // context across sessions. ChatModel and EmbeddingModel reference model deployment names diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider.go b/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider.go index b6f0663007f..caf4671562c 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider.go @@ -28,6 +28,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/exec" + "github.com/azure/azure-dev/cli/azd/pkg/foundry" "github.com/azure/azure-dev/cli/azd/pkg/grpcbroker" "github.com/azure/azure-dev/cli/azd/pkg/input" "github.com/azure/azure-dev/cli/azd/pkg/tools/bicep" @@ -150,7 +151,22 @@ func (p *FoundryProvisioningProvider) Initialize( "skipping synthesizer", filepath.Join(projectPath, onDiskInfraDir)) // endpoint: (brownfield) reuse skips provisioning even on the on-disk // path; connect to the existing project instead of compiling Bicep. - if endpoint := foundryServiceEndpoint(rawYAML, svcName); endpoint != "" { + endpoint, endpointErr := foundryServiceEndpointAtRoot( + rawYAML, + projectPath, + svcName, + ) + if endpointErr != nil { + return exterrors.Validation( + exterrors.CodeInvalidAzureYaml, + fmt.Sprintf( + "resolve existing Foundry project endpoint: %s", + endpointErr, + ), + "fix the project service configuration in azure.yaml", + ) + } + if endpoint != "" { warnNetworkIgnoredInBrownfield(rawYAML, svcName) p.brownfieldEndpoint = endpoint if err := p.captureBrownfieldDeployments(ctx, rawYAML, svcName); err != nil { @@ -173,7 +189,22 @@ func (p *FoundryProvisioningProvider) Initialize( // endpoint: reuse — connect to the existing project, skip provisioning. // network: has no effect in brownfield mode; warn if both are present. warnNetworkIgnoredInBrownfield(rawYAML, svcName) - p.brownfieldEndpoint = foundryServiceEndpoint(rawYAML, svcName) + endpoint, endpointErr := foundryServiceEndpointAtRoot( + rawYAML, + projectPath, + svcName, + ) + if endpointErr != nil { + return exterrors.Validation( + exterrors.CodeInvalidAzureYaml, + fmt.Sprintf( + "resolve existing Foundry project endpoint: %s", + endpointErr, + ), + "fix the project service configuration in azure.yaml", + ) + } + p.brownfieldEndpoint = endpoint if err := p.captureBrownfieldDeployments(ctx, rawYAML, svcName); err != nil { return err } @@ -275,23 +306,44 @@ func (p *FoundryProvisioningProvider) onDiskTemplatePresent() bool { fileExistsAt(filepath.Join(infraDir, onDiskBicepFile)) } -// foundryServiceEndpoint returns the endpoint: value set on the named foundry -// service, or "" when none is set. A non-empty endpoint means bring-your-own -// (brownfield): the provider connects to that existing project instead of -// provisioning a new one. -func foundryServiceEndpoint(rawYAML []byte, svcName string) string { +func foundryServiceEndpointAtRoot( + rawYAML []byte, + projectRoot string, + svcName string, +) (string, error) { type svc struct { Endpoint string `yaml:"endpoint,omitempty"` } type root struct { - Services map[string]svc `yaml:"services"` + Services map[string]map[string]any `yaml:"services"` } var r root if err := yaml.Unmarshal(rawYAML, &r); err != nil { - // Malformed yaml is surfaced upstream; don't mask the parser error. - return "" + return "", err + } + values := r.Services[svcName] + if values == nil { + return "", nil + } + if projectRoot != "" { + resolved, err := foundry.ResolveFileRefs( + values, + projectRoot, + ) + if err != nil { + return "", err + } + values = resolved } - return strings.TrimSpace(r.Services[svcName].Endpoint) + data, err := yaml.Marshal(values) + if err != nil { + return "", err + } + var service svc + if err := yaml.Unmarshal(data, &service); err != nil { + return "", err + } + return strings.TrimSpace(service.Endpoint), nil } // resolveEnvName resolves just the active azd environment name. The brownfield @@ -702,7 +754,11 @@ func (p *FoundryProvisioningProvider) Deploy( func (p *FoundryProvisioningProvider) captureBrownfieldDeployments( ctx context.Context, rawYAML []byte, svcName string, ) error { - deployments, err := synthesis.BrownfieldDeployments(rawYAML, svcName) + deployments, err := synthesis.BrownfieldDeploymentsAtRoot( + rawYAML, + p.projectPath, + svcName, + ) if err != nil { return exterrors.Validation( exterrors.CodeInvalidAzureYaml, @@ -712,7 +768,11 @@ func (p *FoundryProvisioningProvider) captureBrownfieldDeployments( } p.brownfieldDeployments = deployments - connections, err := synthesis.BrownfieldConnections(rawYAML, p.networkEnvMap(ctx)) + connections, err := synthesis.BrownfieldConnectionsAtRoot( + rawYAML, + p.projectPath, + p.networkEnvMap(ctx), + ) if err != nil { return exterrors.Validation( exterrors.CodeInvalidAzureYaml, diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider_test.go index ee92cf65807..f1ead64bdfd 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/foundry_provisioning_provider_test.go @@ -803,13 +803,14 @@ func TestResolveTemplate_OnDiskFallsBackWhenSourceLoaderReturnsNil(t *testing.T) assert.Equal(t, templateModeEmbedded, got.mode) } -func TestFoundryServiceEndpoint(t *testing.T) { +func TestFoundryServiceEndpointAtRoot(t *testing.T) { t.Parallel() tests := []struct { name string yaml string svcName string wantEndpoint string + wantErr bool }{ { name: "greenfield (no endpoint:) -> empty", @@ -850,20 +851,64 @@ services: wantEndpoint: "", }, { - name: "malformed yaml -> empty (upstream surfaces parse error)", - yaml: "not: : valid: yaml", - svcName: "foundry", - wantEndpoint: "", + name: "malformed yaml returns parse error", + yaml: "not: : valid: yaml", + svcName: "foundry", + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tt.wantEndpoint, foundryServiceEndpoint([]byte(tt.yaml), tt.svcName)) + endpoint, err := foundryServiceEndpointAtRoot( + []byte(tt.yaml), + "", + tt.svcName, + ) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantEndpoint, endpoint) }) } } +func TestFoundryServiceEndpointAtRoot_ResolvesFileRef( + t *testing.T, +) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "project.yaml"), + []byte( + "endpoint: https://acct.services.ai.azure.com/"+ + "api/projects/existing\n", + ), + 0o600, + )) + raw := []byte(`services: + foundry: + host: azure.ai.project + $ref: ./project.yaml +`) + + endpoint, err := foundryServiceEndpointAtRoot( + raw, + root, + "foundry", + ) + + require.NoError(t, err) + assert.Equal( + t, + "https://acct.services.ai.azure.com/api/projects/existing", + endpoint, + ) +} + func TestProjectNameFromEndpoint(t *testing.T) { t.Parallel() assert.Equal(t, "my-project", projectNameFromEndpoint( diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check.go b/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check.go index 252418d04fc..6ec95e7ea4d 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check.go @@ -275,8 +275,8 @@ func (c *ResourceGroupLocationCheck) isBrownfieldFoundryProject(ctx context.Cont return false } - // ProjectConfig.Path is the project directory that contains azure.yaml. - rawYAML, err := os.ReadFile(filepath.Join(resp.GetProject().GetPath(), "azure.yaml")) + projectPath := resp.GetProject().GetPath() + rawYAML, err := os.ReadFile(filepath.Join(projectPath, "azure.yaml")) if err != nil { return false } @@ -286,7 +286,12 @@ func (c *ResourceGroupLocationCheck) isBrownfieldFoundryProject(ctx context.Cont return false } - return foundryServiceEndpoint(rawYAML, svcName) != "" + endpoint, err := foundryServiceEndpointAtRoot( + rawYAML, + projectPath, + svcName, + ) + return err == nil && endpoint != "" } // envValueOrEmpty returns the trimmed value of key in the named azd environment, diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check_validate_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check_validate_test.go index 2047d9d12fd..c61b1703b2e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check_validate_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/resource_group_location_check_validate_test.go @@ -159,6 +159,57 @@ func TestValidate_Gates(t *testing.T) { assert.False(t, called, "resource group lookup must not run for a brownfield project") }) + t.Run("skips brownfield project with referenced endpoint", func(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "project.yaml"), + []byte( + "endpoint: https://acct.services.ai.azure.com/"+ + "api/projects/p\n", + ), + 0o600, + )) + require.NoError(t, os.WriteFile( + filepath.Join(root, "azure.yaml"), + []byte(`name: rgloc-test +services: + ai-project: + host: azure.ai.project + $ref: ./project.yaml +`), + 0o600, + )) + proj := &validateStubProjectServer{project: &azdext.ProjectConfig{ + Path: root, + Infra: &azdext.InfraOptions{Provider: FoundryProviderName}, + }} + env := &validateStubEnvServer{ + envName: "rgloc-test", + get: map[string]string{}, + } + client := newValidateTestClient(t, proj, env) + + var called bool + c := &ResourceGroupLocationCheck{azdClient: client} + c.resourceGroupLocation = func( + context.Context, + string, + string, + ) (string, bool, error) { + called = true + return "eastus", true, nil + } + + resp, err := c.Validate( + t.Context(), + provisionContext(sub, "westus2", "rg-x"), + &azdext.ValidationCheckRequest{}, + ) + require.NoError(t, err) + assert.Empty(t, resp.Results) + assert.False(t, called) + }) + t.Run("skips when required values are missing", func(t *testing.T) { proj := &validateStubProjectServer{project: &azdext.ProjectConfig{ Path: writeAzureYAML(t, ""), diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go index 610a6d46e22..2eb5e50279e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go @@ -21,6 +21,7 @@ import ( "os/exec" "path/filepath" "regexp" + "slices" "strings" "time" @@ -31,6 +32,7 @@ import ( "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/azure" "azureaiagent/internal/pkg/paths" + "azureaiagent/internal/pkg/projectconfig" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" @@ -40,6 +42,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/output" "github.com/fatih/color" "github.com/google/uuid" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" ) @@ -145,6 +148,7 @@ var _ azdext.ServiceTargetProvider = &AgentServiceTargetProvider{} type AgentServiceTargetProvider struct { azdClient *azdext.AzdClient serviceConfig *azdext.ServiceConfig + sourceServiceConfig *azdext.ServiceConfig agentDefinitionPath string projectPath string servicePath string @@ -182,6 +186,7 @@ func NewAgentServiceTargetProvider(azdClient *azdext.AzdClient) azdext.ServiceTa // only when a deploy-time entrypoint needs it. func (p *AgentServiceTargetProvider) Initialize(ctx context.Context, serviceConfig *azdext.ServiceConfig) error { p.serviceConfig = serviceConfig + p.sourceServiceConfig = proto.Clone(serviceConfig).(*azdext.ServiceConfig) return nil } @@ -207,7 +212,36 @@ func (p *AgentServiceTargetProvider) ensureDeployContext(ctx context.Context) er "run 'azd init' to initialize your project", ) } - servicePath := p.serviceConfig.RelativePath + if err := ResolveServiceConfigInPlace( + p.serviceConfig, + proj.Project.Path, + ); err != nil { + return exterrors.Validation( + exterrors.CodeInvalidServiceConfig, + fmt.Sprintf( + "failed to resolve service config for %s: %s", + p.serviceConfig.Name, + err, + ), + "fix the agent service configuration in azure.yaml", + ) + } + servicePath, err := ResolvedServiceProjectPath( + p.serviceConfig, + proj.Project.Path, + ) + if err != nil { + return exterrors.Validation( + exterrors.CodeInvalidServiceConfig, + fmt.Sprintf( + "failed to resolve service path for %s: %s", + p.serviceConfig.Name, + err, + ), + "fix the agent service configuration in azure.yaml", + ) + } + p.serviceConfig.RelativePath = servicePath fullPath, err := paths.JoinAllowRoot(proj.Project.Path, servicePath) if err != nil { return exterrors.Validation( @@ -300,7 +334,10 @@ func (p *AgentServiceTargetProvider) ensureDeployContext(ctx context.Context) er // Unified shape: the agent definition is carried inline on the service entry, // so no on-disk agent.yaml is required. - if _, _, found, _, defErr := AgentDefinitionFromService(p.serviceConfig); defErr != nil { + if _, _, found, _, defErr := AgentDefinitionFromResolvedService( + p.serviceConfig, + proj.Project.Path, + ); defErr != nil { return defErr } else if found { p.deployContextReady = true @@ -449,6 +486,7 @@ func (p *AgentServiceTargetProvider) GetTargetResource( if err := p.ensureDeployContext(ctx); err != nil { return nil, err } + serviceConfig = p.serviceConfig // Ensure Foundry project is loaded if err := p.ensureFoundryProject(ctx); err != nil { return nil, err @@ -507,7 +545,7 @@ func (p *AgentServiceTargetProvider) Package( serviceConfig *azdext.ServiceConfig, serviceContext *azdext.ServiceContext, progress azdext.ProgressReporter, -) (*azdext.ServicePackageResult, error) { +) (result *azdext.ServicePackageResult, err error) { if err := p.ensureDeployContext(ctx); err != nil { return nil, err } @@ -552,6 +590,26 @@ func (p *AgentServiceTargetProvider) Package( Artifacts: []*azdext.Artifact{preBuiltImageArtifact(agentDef.Image)}, }, nil } + restore, err := p.persistBuildService(ctx) + if err != nil { + return nil, exterrors.Internal( + exterrors.OpContainerBuild, + fmt.Sprintf("prepare referenced service: %s", err), + ) + } + if restore != nil { + defer func() { + err = combineBuildServiceRestore( + ctx, + restore, + err, + exterrors.OpContainerBuild, + ) + if err != nil { + result = nil + } + }() + } var packageArtifact *azdext.Artifact var newArtifacts []*azdext.Artifact @@ -605,6 +663,88 @@ func (p *AgentServiceTargetProvider) Package( }, nil } +func (p *AgentServiceTargetProvider) persistBuildService( + ctx context.Context, +) (func(context.Context) error, error) { + source := p.sourceServiceConfig + if source == nil || + (!ConfigContainsFileRef(source.GetAdditionalProperties()) && + !ConfigContainsFileRef(source.GetConfig())) { + return nil, nil + } + resolvedPath := p.serviceConfig.GetRelativePath() + if resolvedPath == source.GetRelativePath() { + return nil, nil + } + persisted := proto.Clone(source).(*azdext.ServiceConfig) + persisted.RelativePath = resolvedPath + if err := AddServiceSerialized( + ctx, + p.azdClient, + persisted, + ); err != nil { + return nil, fmt.Errorf("persist resolved service fields: %w", err) + } + return func(restoreCtx context.Context) error { + original := proto.Clone(source).(*azdext.ServiceConfig) + if err := AddServiceSerialized( + restoreCtx, + p.azdClient, + original, + ); err != nil { + return fmt.Errorf("restore referenced service: %w", err) + } + return nil + }, nil +} + +func combineBuildServiceRestore( + ctx context.Context, + restore func(context.Context) error, + operationErr error, + code string, +) error { + if restore == nil { + return operationErr + } + restoreErr := restore(context.WithoutCancel(ctx)) + if restoreErr == nil { + return operationErr + } + classified := exterrors.Internal(code, restoreErr.Error()) + if operationErr == nil { + return classified + } + return errors.Join(operationErr, classified) +} + +// ConfigContainsFileRef reports whether config contains a local $ref. +func ConfigContainsFileRef(config *structpb.Struct) bool { + if config == nil { + return false + } + var contains func(any) bool + contains = func(value any) bool { + switch typed := value.(type) { + case map[string]any: + if _, exists := typed["$ref"]; exists { + return true + } + for _, child := range typed { + if contains(child) { + return true + } + } + case []any: + if slices.ContainsFunc(typed, contains) { + return true + } + } + return false + } + return contains(config.AsMap()) +} + // Publish performs the publish operation for the agent service func (p *AgentServiceTargetProvider) Publish( ctx context.Context, @@ -613,7 +753,7 @@ func (p *AgentServiceTargetProvider) Publish( targetResource *azdext.TargetResource, publishOptions *azdext.PublishOptions, progress azdext.ProgressReporter, -) (*azdext.ServicePublishResult, error) { +) (result *azdext.ServicePublishResult, err error) { // Pre-built image: nothing to package or push. Skip deploy-context // resolution so this path stays cheap and doesn't require agent.yaml. if preBuiltArtifact := findPreBuiltImageArtifact(serviceContext.Package); preBuiltArtifact != nil { @@ -639,6 +779,27 @@ func (p *AgentServiceTargetProvider) Publish( return &azdext.ServicePublishResult{}, nil } + restore, err := p.persistBuildService(ctx) + if err != nil { + return nil, exterrors.Internal( + exterrors.OpContainerPublish, + fmt.Sprintf("prepare referenced service: %s", err), + ) + } + if restore != nil { + defer func() { + err = combineBuildServiceRestore( + ctx, + restore, + err, + exterrors.OpContainerPublish, + ) + if err != nil { + result = nil + } + }() + } + progress("Publishing container") publishResponse, err := p.azdClient. Container(). @@ -975,7 +1136,11 @@ func (p *AgentServiceTargetProvider) loadContainerAgentDefinition() (agent_yaml. // Prefer the agent definition carried inline on the service entry (the // unified service-level shape, or the deprecated config-nested shape). - if ca, isHosted, found, source, err := AgentDefinitionFromService(p.serviceConfig); found || err != nil { + if ca, isHosted, found, source, err := + AgentDefinitionFromResolvedService( + p.serviceConfig, + p.projectPath, + ); found || err != nil { if found && source.IsLegacy() { WarnLegacyAgentShape(source) } @@ -1000,6 +1165,7 @@ func (p *AgentServiceTargetProvider) Deploy( if err := p.ensureDeployContext(ctx); err != nil { return nil, err } + serviceConfig = p.serviceConfig // Ensure Foundry project is loaded if err := p.ensureFoundryProject(ctx); err != nil { return nil, err @@ -1416,14 +1582,6 @@ func (p *AgentServiceTargetProvider) prepareDeploy( fmt.Fprintf(os.Stderr, "Using endpoint: %s\n", azdEnv["FOUNDRY_PROJECT_ENDPOINT"]) fmt.Fprintf(os.Stderr, "Agent Name: %s\n", agentDef.Name) - // Resolve environment variables from YAML using azd environment values - resolvedEnvVars := make(map[string]string) - if agentDef.EnvironmentVariables != nil { - for _, envVar := range *agentDef.EnvironmentVariables { - resolvedEnvVars[envVar.Name] = p.resolveEnvironmentVariables(envVar.Value, azdEnv) - } - } - // Parse service config for container resource overrides foundryAgentConfig, err := LoadServiceTargetAgentConfig(serviceConfig) if err != nil { @@ -1433,6 +1591,33 @@ func (p *AgentServiceTargetProvider) prepareDeploy( "check the service configuration in azure.yaml", ) } + serviceEnv, err := p.serviceEnvironment(serviceConfig) + if err != nil { + return nil, exterrors.Validation( + exterrors.CodeInvalidServiceConfig, + fmt.Sprintf( + "failed to load service environment: %s", + err, + ), + "fix the service env configuration in azure.yaml", + ) + } + + resolvedEnvVars := make(map[string]string) + if agentDef.EnvironmentVariables != nil { + for _, envVar := range *agentDef.EnvironmentVariables { + resolvedEnvVars[envVar.Name] = + p.resolveEnvironmentVariables(envVar.Value, azdEnv) + } + } + for name, value := range foundryAgentConfig.Environment { + resolvedEnvVars[name] = + p.resolveEnvironmentVariables(value, azdEnv) + } + for name, value := range serviceEnv { + resolvedEnvVars[name] = + p.resolveEnvironmentVariables(value, azdEnv) + } warnDeprecatedScaleSettings(ServiceConfigProps(serviceConfig)) @@ -1441,6 +1626,15 @@ func (p *AgentServiceTargetProvider) prepareDeploy( cpu = foundryAgentConfig.Container.Resources.Cpu memory = foundryAgentConfig.Container.Resources.Memory } + // $ref services never persist resolved defaults to azure.yaml, + // so deploy applies the extension defaults here to keep $ref + // and inline services consistent. + if cpu == "" { + cpu = DefaultCpu + } + if memory == "" { + memory = DefaultMemory + } // Build options: env vars + cpu/memory (if set) + caller-provided extras options := []agent_yaml.AgentBuildOption{ @@ -1480,6 +1674,22 @@ func (p *AgentServiceTargetProvider) prepareDeploy( }, nil } +func (p *AgentServiceTargetProvider) serviceEnvironment( + serviceConfig *azdext.ServiceConfig, +) (map[string]string, error) { + raw, err := projectconfig.LoadServiceEnvironment( + p.projectPath, + serviceConfig.GetName(), + ) + if err != nil { + return nil, err + } + if raw != nil { + return raw, nil + } + return serviceConfig.GetEnvironment(), nil +} + // deployResult holds the intermediate results from a deploy method (code or container) // before the common post-deploy steps (polling, patching, finalization) are applied. type deployResult struct { diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go index af7f0f8cc3a..c90cc8fba40 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go @@ -24,6 +24,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/structpb" ) func TestApplyAgentMetadata(t *testing.T) { @@ -204,6 +205,7 @@ func newServiceTargetTestClient( t *testing.T, containerSrv azdext.ContainerServiceServer, promptSrv azdext.PromptServiceServer, + projectSrvs ...azdext.ProjectServiceServer, ) *azdext.AzdClient { t.Helper() @@ -214,6 +216,9 @@ func newServiceTargetTestClient( if promptSrv != nil { azdext.RegisterPromptServiceServer(srv, promptSrv) } + if len(projectSrvs) > 0 && projectSrvs[0] != nil { + azdext.RegisterProjectServiceServer(srv, projectSrvs[0]) + } lis, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) @@ -233,7 +238,9 @@ func newServiceTargetTestClient( type stubProjectServer struct { azdext.UnimplementedProjectServiceServer - project *azdext.ProjectConfig + project *azdext.ProjectConfig + addedService *azdext.ServiceConfig + addedServices []*azdext.ServiceConfig } func (s *stubProjectServer) Get( @@ -242,6 +249,15 @@ func (s *stubProjectServer) Get( return &azdext.GetProjectResponse{Project: s.project}, nil } +func (s *stubProjectServer) AddService( + _ context.Context, + request *azdext.AddServiceRequest, +) (*azdext.EmptyResponse, error) { + s.addedService = request.GetService() + s.addedServices = append(s.addedServices, request.GetService()) + return &azdext.EmptyResponse{}, nil +} + type stubInitializeEnvServer struct { azdext.UnimplementedEnvironmentServiceServer } @@ -294,6 +310,29 @@ func newInitializeTestClient(t *testing.T, projectRoot string) *azdext.AzdClient return client } +func newProjectTestClient( + t *testing.T, + projectServer azdext.ProjectServiceServer, +) *azdext.AzdClient { + t.Helper() + + srv := grpc.NewServer() + azdext.RegisterProjectServiceServer(srv, projectServer) + lis, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = srv.Serve(lis) }() + t.Cleanup(func() { + srv.Stop() + _ = lis.Close() + }) + client, err := azdext.NewAzdClient( + azdext.WithAddress(lis.Addr().String()), + ) + require.NoError(t, err) + t.Cleanup(func() { client.Close() }) + return client +} + type stubPromptServer struct { azdext.UnimplementedPromptServiceServer selectedIndex int32 @@ -1021,6 +1060,329 @@ func TestLoadContainerAgentDefinition_EnvPathOverridesInlineDefinition(t *testin require.Equal(t, "override-agent", got.Name) } +func TestLoadContainerAgentDefinition_FileRef(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(dir, "agent.yaml"), + []byte( + "kind: hosted\n"+ + "name: referenced-agent\n"+ + "protocols:\n"+ + " - protocol: responses\n"+ + " version: \"1.0.0\"\n", + ), + 0o600, + )) + props, err := structpb.NewStruct(map[string]any{ + "$ref": "./agent.yaml", + }) + require.NoError(t, err) + provider := &AgentServiceTargetProvider{ + projectPath: dir, + serviceConfig: &azdext.ServiceConfig{ + Name: "referenced-agent", + Host: "azure.ai.agent", + AdditionalProperties: props, + }, + } + + got, isHosted, err := provider.loadContainerAgentDefinition() + + require.NoError(t, err) + require.True(t, isHosted) + require.Equal(t, "referenced-agent", got.Name) +} + +func TestPersistBuildService_RestoresSourceFields( + t *testing.T, +) { + t.Parallel() + + source, err := structpb.NewStruct(map[string]any{ + "$ref": "./agent.yaml", + }) + require.NoError(t, err) + resolved, err := structpb.NewStruct(map[string]any{ + "kind": "hosted", + "name": "referenced-agent", + }) + require.NoError(t, err) + projectServer := &stubProjectServer{} + provider := &AgentServiceTargetProvider{ + azdClient: newProjectTestClient(t, projectServer), + sourceServiceConfig: &azdext.ServiceConfig{ + Name: "referenced-agent", + Host: "azure.ai.agent", + AdditionalProperties: source, + }, + serviceConfig: &azdext.ServiceConfig{ + Name: "referenced-agent", + Host: "azure.ai.agent", + RelativePath: "src/agent", + Image: "example.azurecr.io/agent:v1", + AdditionalProperties: resolved, + }, + } + + restore, err := provider.persistBuildService(t.Context()) + + require.NoError(t, err) + require.NotNil(t, restore) + require.NotNil(t, projectServer.addedService) + require.Equal( + t, + "src/agent", + projectServer.addedService.GetRelativePath(), + ) + require.Contains( + t, + projectServer.addedService. + GetAdditionalProperties(). + GetFields(), + "$ref", + ) + require.Equal( + t, + "", + projectServer.addedService.GetImage(), + ) + + require.NoError(t, restore(t.Context())) + require.Empty(t, projectServer.addedService.GetRelativePath()) + require.Empty(t, projectServer.addedService.GetImage()) + require.Contains( + t, + projectServer.addedService. + GetAdditionalProperties(). + GetFields(), + "$ref", + ) +} + +func TestPackage_RestoresReferencedService(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + agentPath := writeHostedAgentYAML(t, dir) + source, err := structpb.NewStruct(map[string]any{ + "$ref": "./agent.yaml", + }) + require.NoError(t, err) + resolved, err := structpb.NewStruct(map[string]any{ + "kind": "hosted", + "name": "referenced-agent", + }) + require.NoError(t, err) + projectServer := &stubProjectServer{} + client := newServiceTargetTestClient( + t, + &stubContainerServer{}, + nil, + projectServer, + ) + provider := &AgentServiceTargetProvider{ + azdClient: client, + agentDefinitionPath: agentPath, + env: &azdext.Environment{Name: "test-env"}, + sourceServiceConfig: &azdext.ServiceConfig{ + Name: "referenced-agent", + Host: "azure.ai.agent", + AdditionalProperties: source, + }, + serviceConfig: &azdext.ServiceConfig{ + Name: "referenced-agent", + Host: "azure.ai.agent", + RelativePath: "src/agent", + AdditionalProperties: resolved, + }, + } + + _, err = provider.Package( + t.Context(), + &azdext.ServiceConfig{Name: "referenced-agent"}, + &azdext.ServiceContext{}, + func(string) {}, + ) + + require.NoError(t, err) + require.Len(t, projectServer.addedServices, 2) + require.Equal( + t, + "src/agent", + projectServer.addedServices[0].GetRelativePath(), + ) + require.Empty( + t, + projectServer.addedServices[1].GetRelativePath(), + ) + require.Contains( + t, + projectServer.addedServices[1]. + GetAdditionalProperties(). + GetFields(), + "$ref", + ) +} + +func TestPrepareDeploy_MergesUnifiedEnvironment(t *testing.T) { + t.Parallel() + + agentDef := sampleContainerAgent() + agentDef.EnvironmentVariables = &[]agent_yaml.EnvironmentVariable{ + {Name: "LEGACY_ONLY", Value: "${LEGACY_VALUE}"}, + {Name: "SHARED", Value: "legacy"}, + } + props, err := AgentDefinitionToServiceProperties( + agentDef, + &ServiceTargetAgentConfig{ + Environment: map[string]string{ + "REF_ONLY": "${REF_VALUE}", + "SHARED": "ref", + }, + }, + ) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + AdditionalProperties: props, + Environment: map[string]string{ + "DIRECT_ONLY": "direct", + "SHARED": "direct", + }, + } + provider := &AgentServiceTargetProvider{} + + prep, err := provider.prepareDeploy( + svc, + agentDef, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://example", + "LEGACY_VALUE": "legacy-value", + "REF_VALUE": "ref-value", + }, + []agent_yaml.AgentBuildOption{ + agent_yaml.WithImageURL("registry.example/agent:v1"), + }, + ) + + require.NoError(t, err) + definition, ok := prep.request.Definition.(agent_api.HostedAgentDefinition) + require.True(t, ok) + require.Equal( + t, + "legacy-value", + definition.EnvironmentVariables["LEGACY_ONLY"], + ) + require.Equal( + t, + "ref-value", + definition.EnvironmentVariables["REF_ONLY"], + ) + require.Equal( + t, + "direct", + definition.EnvironmentVariables["DIRECT_ONLY"], + ) + require.Equal( + t, + "direct", + definition.EnvironmentVariables["SHARED"], + ) +} + +func TestPrepareDeployUsesRawUnifiedEnvironment(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "azure.yaml"), + []byte(`services: + basic-agent: + host: azure.ai.agent + env: + PROJECT: ${{project.endpoint}} + ENABLED: true +`), + 0o600, + )) + agentDef := sampleContainerAgent() + props, err := AgentDefinitionToServiceProperties( + agentDef, + &ServiceTargetAgentConfig{}, + ) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + AdditionalProperties: props, + Environment: map[string]string{ + "PROJECT": "", + "ENABLED": "", + }, + } + provider := &AgentServiceTargetProvider{projectPath: root} + + prep, err := provider.prepareDeploy( + svc, + agentDef, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://example", + }, + []agent_yaml.AgentBuildOption{ + agent_yaml.WithImageURL("registry.example/agent:v1"), + }, + ) + + require.NoError(t, err) + definition, ok := prep.request.Definition.(agent_api.HostedAgentDefinition) + require.True(t, ok) + require.Equal( + t, + "${{project.endpoint}}", + definition.EnvironmentVariables["PROJECT"], + ) + require.Equal( + t, + "true", + definition.EnvironmentVariables["ENABLED"], + ) +} + +func TestPrepareDeployAppliesDefaultResources(t *testing.T) { + t.Parallel() + + agentDef := sampleContainerAgent() + agentDef.Resources = nil + props, err := AgentDefinitionToServiceProperties( + agentDef, + &ServiceTargetAgentConfig{}, + ) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + AdditionalProperties: props, + } + provider := &AgentServiceTargetProvider{} + + prep, err := provider.prepareDeploy( + svc, + agentDef, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://example", + }, + []agent_yaml.AgentBuildOption{ + agent_yaml.WithImageURL("registry.example/agent:v1"), + }, + ) + + require.NoError(t, err) + definition, ok := prep.request.Definition.(agent_api.HostedAgentDefinition) + require.True(t, ok) + require.Equal(t, DefaultCpu, definition.CPU) + require.Equal(t, DefaultMemory, definition.Memory) +} + func TestShouldUsePreBuiltImage_NoImageDefaultsToBuild(t *testing.T) { t.Parallel() diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_update.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_update.go new file mode 100644 index 00000000000..13a530114b3 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_update.go @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "context" + "sync" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" +) + +var projectServiceUpdateMu sync.Mutex + +// AddServiceSerialized prevents concurrent azure.yaml writes. +func AddServiceSerialized( + ctx context.Context, + client *azdext.AzdClient, + service *azdext.ServiceConfig, +) error { + projectServiceUpdateMu.Lock() + defer projectServiceUpdateMu.Unlock() + + _, err := client.Project().AddService( + ctx, + &azdext.AddServiceRequest{Service: service}, + ) + return err +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go index 4e7c9071042..a892f53a95c 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go +++ b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go @@ -252,14 +252,26 @@ func Synthesize(in Input) (*Result, error) { return nil, ErrEndpointBrownfield } - includeAcr := deriveIncludeAcr(root.Services, svc) + includeAcr, err := deriveIncludeAcr( + root.Services, + svc, + in.ProjectRoot, + ) + if err != nil { + return nil, err + } deployments := svc.Deployments if deployments == nil { deployments = []Deployment{} } - connections, err := collectConnections(root.Services, in.Env, !in.PreserveVarRefs) + connections, err := collectConnections( + root.Services, + in.ProjectRoot, + in.Env, + !in.PreserveVarRefs, + ) if err != nil { return nil, err } @@ -288,6 +300,15 @@ func Synthesize(in Input) (*Result, error) { // to learn which model deployments to create on the existing account. Returns // nil (not an error) when the service declares no deployments. func BrownfieldDeployments(raw []byte, serviceName string) ([]Deployment, error) { + return BrownfieldDeploymentsAtRoot(raw, "", serviceName) +} + +// BrownfieldDeploymentsAtRoot resolves local deployment includes. +func BrownfieldDeploymentsAtRoot( + raw []byte, + projectRoot string, + serviceName string, +) ([]Deployment, error) { if len(raw) == 0 { return nil, errors.New("synthesis: raw azure.yaml is empty") } @@ -304,6 +325,17 @@ func BrownfieldDeployments(raw []byte, serviceName string) ([]Deployment, error) if !ok { return nil, ErrServiceNotFound } + if projectRoot != "" { + var err error + node, err = resolveServiceRefs( + node, + projectRoot, + serviceName, + ) + if err != nil { + return nil, err + } + } var svc projectService if err := node.Decode(&svc); err != nil { @@ -321,6 +353,15 @@ func BrownfieldDeployments(raw []byte, serviceName string) ([]Deployment, error) // expressions pass through. Returns an empty slice (not an error) when none // are declared. func BrownfieldConnections(raw []byte, env map[string]string) ([]Connection, error) { + return BrownfieldConnectionsAtRoot(raw, "", env) +} + +// BrownfieldConnectionsAtRoot resolves local connection includes. +func BrownfieldConnectionsAtRoot( + raw []byte, + projectRoot string, + env map[string]string, +) ([]Connection, error) { if len(raw) == 0 { return nil, errors.New("synthesis: raw azure.yaml is empty") } @@ -330,7 +371,7 @@ func BrownfieldConnections(raw []byte, env map[string]string) ([]Connection, err return nil, fmt.Errorf("parse azure.yaml: %w", err) } - return collectConnections(root.Services, env, true) + return collectConnections(root.Services, projectRoot, env, true) } // resolveServiceRefs expands $ref file includes in one service entry. It decodes @@ -367,12 +408,16 @@ func resolveServiceRefs(node yaml.Node, projectRoot, serviceName string) (yaml.N // Keying on image/codeConfiguration rather than the optional docker: block is // deliberate: docker: is not in the agent schema and is dropped by omitempty // when remoteBuild is false, so it is not a reliable build signal. -func deriveIncludeAcr(services map[string]yaml.Node, svc projectService) bool { +func deriveIncludeAcr( + services map[string]yaml.Node, + svc projectService, + projectRoot string, +) (bool, error) { if slices.ContainsFunc(svc.Agents, agentNeedsAcr) { - return true + return true, nil } - for _, node := range services { + for serviceName, node := range services { var service serviceBlock if err := node.Decode(&service); err != nil { continue @@ -380,15 +425,33 @@ func deriveIncludeAcr(services map[string]yaml.Node, svc projectService) bool { if service.Host != "azure.ai.agent" { continue } + if projectRoot != "" { + resolved, err := resolveServiceRefs( + node, + projectRoot, + serviceName, + ) + if err != nil { + return false, err + } + node = resolved + if err := node.Decode(&service); err != nil { + return false, fmt.Errorf( + "decode service %q: %w", + serviceName, + err, + ) + } + } if agentNeedsAcr(agentBlock{ Kind: service.Kind, Image: service.Image, CodeConfiguration: service.CodeConfiguration, }) { - return true + return true, nil } } - return false + return false, nil } // agentNeedsAcr reports whether a single agent entry builds a container image @@ -415,6 +478,7 @@ func agentNeedsAcr(a agentBlock) bool { // ${{...}} expressions are always preserved, mirroring synthesizeNetwork. func collectConnections( services map[string]yaml.Node, + projectRoot string, env map[string]string, resolve bool, ) ([]Connection, error) { @@ -427,6 +491,17 @@ func collectConnections( if err := node.Decode(&host); err != nil || host.Host != aiConnectionHost { continue } + if projectRoot != "" { + resolved, err := resolveServiceRefs( + node, + projectRoot, + name, + ) + if err != nil { + return nil, err + } + node = resolved + } var svc connectionService if err := node.Decode(&svc); err != nil { return nil, fmt.Errorf("services.%s: decode connection: %w", name, err) diff --git a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go index d72300a5f80..feb9d929563 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go @@ -629,6 +629,47 @@ services: }) } +func TestSynthesizeConnectionsAtRootResolvesFileRef(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "connection.yaml"), + []byte(`category: CognitiveSearch +target: https://search.example +authType: ApiKey +credentials: + key: ${SEARCH_KEY} +`), + 0o600, + )) + raw := []byte(`services: + project: + host: azure.ai.project + search: + host: azure.ai.connection + uses: [project] + $ref: ./connection.yaml +`) + + result, err := Synthesize(Input{ + RawAzureYAML: raw, + ServiceName: "project", + AcceptedHosts: []string{"azure.ai.project"}, + ProjectRoot: root, + Env: map[string]string{"SEARCH_KEY": "secret"}, + }) + + require.NoError(t, err) + connections, ok := result.Parameters["connections"].([]Connection) + require.True(t, ok) + require.Len(t, connections, 1) + assert.Equal(t, "search", connections[0].Name) + assert.Equal(t, "CognitiveSearch", connections[0].Category) + assert.Equal(t, "https://search.example", connections[0].Target) + assert.Equal(t, "secret", connections[0].Credentials["key"]) +} + // TestBrownfieldConnections verifies connection services are collected for a // brownfield (endpoint:) project, with ${VAR} resolved (brownfield provisions // so references must be concrete) and Foundry ${{...}} preserved. @@ -680,6 +721,41 @@ services: _, err := BrownfieldConnections(nil, nil) require.Error(t, err) }) + + t.Run("resolves connection file references", func(t *testing.T) { + t.Parallel() + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "connection.yaml"), + []byte(`category: CognitiveSearch +target: https://search.example +authType: ApiKey +credentials: + key: ${SEARCH_KEY} +`), + 0o600, + )) + raw := []byte(`services: + project: + host: azure.ai.project + endpoint: https://existing.example/api/projects/p1 + search: + host: azure.ai.connection + uses: [project] + $ref: ./connection.yaml +`) + + connections, err := BrownfieldConnectionsAtRoot( + raw, + root, + map[string]string{"SEARCH_KEY": "secret"}, + ) + + require.NoError(t, err) + require.Len(t, connections, 1) + assert.Equal(t, "CognitiveSearch", connections[0].Category) + assert.Equal(t, "secret", connections[0].Credentials["key"]) + }) } func TestBrownfieldDeployments(t *testing.T) { @@ -792,6 +868,45 @@ func TestBrownfieldDeployments_EmptyRaw(t *testing.T) { require.Error(t, err) } +func TestBrownfieldDeploymentsAtRoot_ResolvesFileRef( + t *testing.T, +) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "deployment.yaml"), + []byte( + "name: gpt-4o\n"+ + "model:\n"+ + " format: OpenAI\n"+ + " name: gpt-4o\n"+ + " version: \"2024-08-06\"\n"+ + "sku:\n"+ + " capacity: 10\n"+ + " name: GlobalStandard\n", + ), + 0o600, + )) + raw := []byte(`services: + my-project: + host: azure.ai.project + endpoint: https://example + deployments: + - $ref: ./deployment.yaml +`) + + deployments, err := BrownfieldDeploymentsAtRoot( + raw, + root, + "my-project", + ) + + require.NoError(t, err) + require.Len(t, deployments, 1) + assert.Equal(t, "gpt-4o", deployments[0].Name) +} + func TestSynthesize_NetworkPreserveVarRefs(t *testing.T) { // Eject path: ${VAR} references must pass through verbatim (and skip the // format checks that cannot run on an unexpanded placeholder), so the @@ -870,6 +985,40 @@ services: assert.Equal(t, 10, deployments[0].Sku.Capacity) } +func TestSynthesize_ResolvesAgentRefForAcr(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "agent.yaml"), + []byte( + "kind: hosted\n"+ + "name: referenced-agent\n"+ + "image: registry.example/agent:v1\n", + ), + 0o600, + )) + raw := []byte(`services: + my-project: + host: azure.ai.project + referenced-agent: + host: azure.ai.agent + $ref: ./agent.yaml +`) + + result, err := Synthesize(Input{ + RawAzureYAML: raw, + ServiceName: "my-project", + AcceptedHosts: []string{"azure.ai.project"}, + ProjectRoot: root, + }) + + require.NoError(t, err) + includeAcr, ok := result.Parameters["includeAcr"].(bool) + require.True(t, ok) + assert.False(t, includeAcr) +} + func TestSynthesize_InputValidation(t *testing.T) { tests := []struct { name string