From f799f081c512104e1279cce9f50f84af7da74502 Mon Sep 17 00:00:00 2001 From: huimiu Date: Fri, 21 Aug 2026 13:19:35 +0800 Subject: [PATCH 1/9] fix: diagnose unified connection services --- .../extensions/azure.ai.agents/CHANGELOG.md | 6 + .../internal/cmd/doctor/checks_connections.go | 60 +- .../cmd/doctor/checks_connections_test.go | 53 +- .../internal/cmd/doctor/checks_remote_test.go | 2 +- .../internal/cmd/nextstep/condition.go | 73 +-- .../internal/cmd/nextstep/connections.go | 425 +++++++++++++++ .../internal/cmd/nextstep/connections_test.go | 511 ++++++++++++++++++ .../internal/cmd/nextstep/evaluate.go | 95 ++++ .../internal/cmd/nextstep/manifest.go | 22 +- .../internal/cmd/nextstep/state.go | 12 + .../internal/cmd/nextstep/types.go | 26 +- 11 files changed, 1177 insertions(+), 108 deletions(-) create mode 100644 cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go create mode 100644 cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go create mode 100644 cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go diff --git a/cli/azd/extensions/azure.ai.agents/CHANGELOG.md b/cli/azd/extensions/azure.ai.agents/CHANGELOG.md index a5d3569ccf9..cbec7e72735 100644 --- a/cli/azd/extensions/azure.ai.agents/CHANGELOG.md +++ b/cli/azd/extensions/azure.ai.agents/CHANGELOG.md @@ -1,5 +1,11 @@ # Release History +## 1.0.0-beta.12 (Unreleased) + +### Bugs Fixed + +- Diagnose unified, bundled, and legacy connection services in Doctor and next-step. + ## 1.0.0-beta.11 (2026-08-20) ### Features Added 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 c8fc40ef91e..2e289f13cc3 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 @@ -35,13 +35,13 @@ type foundryConnectionsProbeFn func( accountName, projectName string, ) ([]string, error) -// newCheckConnections produces Check `remote.connections` (P5.1 -// C15). For each `ConnectionResource` declared in any service's -// `agent.manifest.yaml` (collected by the C2 manifest walker), the -// check queries the Foundry project's connection list and verifies a -// connection with the matching name exists. The check Passes when -// every manifest-declared connection has a corresponding entry; -// Fails when one or more are missing. +// newCheckConnections produces Check `remote.connections`. For each +// enabled connection collected from unified azure.yaml services or +// compatible sources, the check queries the Foundry project's +// connection list and verifies a connection with the matching name +// exists. The check Passes when every configured connection has a +// corresponding entry; Fails when one or more are missing or when +// connection configuration cannot be loaded. // // # Skip cascade // @@ -55,26 +55,27 @@ type foundryConnectionsProbeFn func( // let the auth check own the diagnosis. // - `remote.foundry-endpoint` failed → same root cause, same // remediation. -// - state.HasConnections == false → no manifest connection -// declarations; the check has nothing to verify. Surface as -// Skip with a short explanation rather than a vacuous Pass. +// - state.ConnectionLoadErrors set → configuration could not be +// read. Fail without probing so a bad $ref is not Skip. +// - state.HasConnections == false → no enabled connection +// services or legacy resources; Skip rather than a vacuous Pass. // - `AZURE_AI_PROJECT_ID` not set / cannot be parsed → can not // derive the account + project to probe. Skip cleanly; the // rbac check already emits the canonical `azd env set` fix. // // # Classification // -// - Every manifest connection matches a Foundry connection name → -// Pass with the matched count. +// - Every configured connection matches a Foundry connection +// name → Pass with the matched count. // - One or more missing → Fail with the missing names listed in // the Message and structured under `Details["missingConnections"]` -// (each entry carries Name, ServiceName, Detail — the manifest's -// " | " identifier surfaced by the C2 walker). +// (each entry carries Name, ServiceName, Detail — the +// " | " identifier from collected state). // - Probe error → Skip with the underlying error verbatim. 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 { @@ -125,10 +126,26 @@ 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( + "failed to load configured connections: %s", + strings.Join(state.ConnectionLoadErrors, "; "), + ), + Suggestion: "Fix azure.yaml, its $ref files, or the " + + "legacy agent.manifest.yaml, then retry " + + "`azd ai agent doctor`.", + 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 enabled connection services " + + "or legacy connection resources found.", } } @@ -281,11 +298,12 @@ func classifyConnections( return Result{ Status: StatusFail, Message: fmt.Sprintf( - "%d connection(s) referenced by agent.manifest.yaml are missing on project %s: %s", + "%d configured connection(s) 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: "Run `azd provision` to create or reconcile the " + + "missing connection(s), or update the configured connection " + + "services or legacy manifest resources to match connections " + + "that already exist on the Foundry project.", 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..75fba2b89f9 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 @@ -91,13 +91,54 @@ func TestCheckConnections_SkipsCascadeFromUpstream(t *testing.T) { func TestCheckConnections_SkipsWhenNoManifestConnections(t *testing.T) { t.Parallel() + var probeCalls int deps := Dependencies{ - assembleState: fixedAssembler(&nextstep.State{HasConnections: false}), - probeFoundryConnections: fixedConnectionsProbe(nil, nil, nil), + assembleState: fixedAssembler(&nextstep.State{HasConnections: false}), + probeFoundryConnections: func( + _ context.Context, _, _ string, + ) ([]string, error) { + probeCalls++ + return nil, nil + }, } res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) require.Equal(t, StatusSkip, res.Status) - require.Contains(t, res.Message, "no connection resources declared") + require.Contains(t, res.Message, "no enabled connection services") + require.Contains(t, res.Message, "legacy connection resources found") + require.Equal(t, 0, probeCalls) +} + +func TestCheckConnections_FailsOnLoadErrorsBeforeSkip(t *testing.T) { + t.Parallel() + var probeCalls int + deps := Dependencies{ + assembleState: fixedAssembler(&nextstep.State{ + HasConnections: false, + ConnectionLoadErrors: []string{ + `connection service "bad-conn" has condition in its resolved $ref; ` + + `put condition beside host in azure.yaml`, + }, + }), + probeFoundryConnections: func( + _ context.Context, _, _ string, + ) ([]string, error) { + probeCalls++ + return nil, nil + }, + } + res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) + require.Equal(t, StatusFail, res.Status) + require.Equal(t, 0, probeCalls) + require.Contains(t, res.Message, "failed to load configured connections") + require.Contains(t, res.Message, `connection service "bad-conn"`) + require.Contains(t, res.Message, "put condition beside host in azure.yaml") + require.Contains(t, res.Suggestion, "Fix azure.yaml") + require.Contains(t, res.Suggestion, "azd ai agent doctor") + require.NotContains(t, res.Suggestion, "azd deploy") + require.Equal(t, []string{ + `connection service "bad-conn" has condition in its resolved $ref; ` + + `put condition beside host in azure.yaml`, + }, res.Details["loadErrors"]) } func TestCheckConnections_FailsWhenAssemblerReturnsNilState(t *testing.T) { @@ -220,11 +261,13 @@ func TestCheckConnections_FailsWithMissing(t *testing.T) { } res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) require.Equal(t, StatusFail, res.Status) - require.Contains(t, res.Message, "2 connection(s)") + require.Contains(t, res.Message, "2 configured connection(s)") require.Contains(t, res.Message, "openai-default [AzureOpenAI | https://openai.test] (service chat)") require.Contains(t, res.Message, "search-conn [CognitiveSearch | search.test] (service search)") require.NotContains(t, res.Message, "blob-storage") require.Contains(t, res.Suggestion, "azd provision") + require.NotContains(t, res.Suggestion, "azd deploy") + require.Contains(t, res.Suggestion, "configured connection services") require.EqualValues(t, 1, res.Details["matchedCount"]) } @@ -243,7 +286,7 @@ func TestCheckConnections_FailsWhenAllMissing(t *testing.T) { } res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) require.Equal(t, StatusFail, res.Status) - require.Contains(t, res.Message, "1 connection(s)") + require.Contains(t, res.Message, "1 configured connection(s)") require.Contains(t, res.Message, "blob-storage (service chat)") } 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/nextstep/condition.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/condition.go index dac09cf52c9..616e33ef5e5 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/condition.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/condition.go @@ -7,10 +7,7 @@ import ( "context" "fmt" "os" - "strconv" - "strings" - "github.com/azure/azure-dev/cli/azd/pkg/foundry" "google.golang.org/protobuf/types/known/structpb" ) @@ -32,66 +29,50 @@ func isServiceEnabled( return true, nil } - condition, err := conditionValueString(value) - if err != nil { - return false, err - } - if strings.TrimSpace(condition) == "" { - return true, nil - } - - expanded, err := expandServiceCondition( - ctx, - src, - envName, - condition, + var lookupErr error + enabled, err := evaluateCondition( + conditionRawValue(value), + serviceConditionLookup(ctx, src, envName, &lookupErr), ) if err != nil { return false, err } - return isTruthyCondition(expanded), nil + if lookupErr != nil { + return false, lookupErr + } + return enabled, nil } -func conditionValueString(value *structpb.Value) (string, error) { +func conditionRawValue(value *structpb.Value) any { if value == nil { - return "", nil + return nil } - - switch kind := value.Kind.(type) { - case *structpb.Value_StringValue: - return kind.StringValue, nil - case *structpb.Value_BoolValue: - return strconv.FormatBool(kind.BoolValue), nil - case *structpb.Value_NumberValue: - return strconv.FormatFloat(kind.NumberValue, 'g', -1, 64), nil + switch value.Kind.(type) { case *structpb.Value_NullValue: - return "", nil + return nil default: - return "", fmt.Errorf( - "condition must be a string, boolean, or number", - ) + return value.AsInterface() } } -func expandServiceCondition( +func serviceConditionLookup( ctx context.Context, src Source, envName string, - condition string, -) (string, error) { + lookupErr *error, +) func(string) string { if envName == "" { - return foundry.ExpandEnv(condition, os.Getenv) + return os.Getenv } values := map[string]string{} - var lookupErr error - expanded, err := foundry.ExpandEnv(condition, func(name string) string { + return func(name string) string { if value, ok := values[name]; ok { return value } value, err := src.EnvValue(ctx, envName, name) if err != nil { - lookupErr = fmt.Errorf( + *lookupErr = fmt.Errorf( "read condition environment variable %q: %w", name, err, @@ -100,21 +81,5 @@ func expandServiceCondition( } values[name] = value return value - }) - if err != nil { - return "", err - } - if lookupErr != nil { - return "", lookupErr - } - return expanded, nil -} - -func isTruthyCondition(value string) bool { - switch value { - case "1", "true", "TRUE", "True", "yes", "YES", "Yes": - return true - default: - return false } } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go new file mode 100644 index 00000000000..97a129da14b --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go @@ -0,0 +1,425 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package nextstep + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "slices" + "strings" + + "azureaiagent/internal/pkg/agents/agent_yaml" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/foundry" + "google.golang.org/protobuf/types/known/structpb" +) + +type bundledConnection struct { + Name string `json:"name"` + Category string `json:"category"` + Target string `json:"target"` +} + +type bundledConnectionConfig struct { + Connections []bundledConnection `json:"connections"` +} + +// populateConnections merges enabled unified connection services, +// bundled agent config, and legacy manifest resources. Same Foundry +// names keep split > bundled > manifest precedence. +func populateConnections( + ctx context.Context, + src Source, + envName string, + projectCfg *azdext.ProjectConfig, + state *State, + errs *[]error, +) { + if projectCfg == nil || state == nil { + return + } + + collected := map[string]ResourceRef{} + collectSplitConnections( + ctx, + src, + envName, + projectCfg, + state, + errs, + collected, + ) + collectBundledConnections( + ctx, + src, + envName, + projectCfg, + state, + errs, + collected, + ) + collectManifestConnections( + ctx, + src, + envName, + projectCfg, + state, + errs, + collected, + ) + + refs := make([]ResourceRef, 0, len(collected)) + for _, ref := range collected { + refs = append(refs, ref) + } + slices.SortFunc(refs, func(a, b ResourceRef) int { + if c := strings.Compare(a.Name, b.Name); c != 0 { + return c + } + return strings.Compare(a.ServiceName, b.ServiceName) + }) + if len(refs) == 0 { + state.Connections = nil + } else { + state.Connections = refs + } + state.HasConnections = len(refs) > 0 + slices.Sort(state.ConnectionLoadErrors) +} + +func collectSplitConnections( + ctx context.Context, + src Source, + envName string, + projectCfg *azdext.ProjectConfig, + state *State, + errs *[]error, + collected map[string]ResourceRef, +) { + for _, serviceName := range sortedServiceKeys(projectCfg) { + svc := projectCfg.Services[serviceName] + if svc == nil || svc.GetHost() != connectionHost { + continue + } + + enabled, err := isServiceEnabled(ctx, src, envName, serviceName) + if err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "connection service %q has an invalid deployment condition: %v", + serviceName, + err, + ), + ) + continue + } + if !enabled { + continue + } + + resolved, err := resolveServiceProperties(svc, projectCfg.Path) + if err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "connection service %q: %v", + serviceName, + err, + ), + ) + continue + } + recordResolvedConditionError( + state, + errs, + "connection service", + serviceName, + resolved, + ) + + var decoded bundledConnection + if err := decodeJSONMap(resolved, &decoded); err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "connection service %q: decode connection: %v", + serviceName, + err, + ), + ) + continue + } + if _, exists := collected[serviceName]; exists { + continue + } + collected[serviceName] = ResourceRef{ + Name: serviceName, + ServiceName: serviceName, + Detail: formatConnectionDetail( + decoded.Category, + decoded.Target, + ), + } + } +} + +func collectBundledConnections( + ctx context.Context, + src Source, + envName string, + projectCfg *azdext.ProjectConfig, + state *State, + errs *[]error, + collected map[string]ResourceRef, +) { + for _, serviceName := range sortedServiceKeys(projectCfg) { + svc := projectCfg.Services[serviceName] + if svc == nil || svc.GetHost() != agentHost { + continue + } + enabled, err := isServiceEnabled(ctx, src, envName, serviceName) + if err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "agent service %q deployment condition: %v", + serviceName, + err, + ), + ) + continue + } + if !enabled { + continue + } + + resolved, err := resolveAgentDefinition(svc, projectCfg.Path) + if err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "agent service %q: %v", + serviceName, + err, + ), + ) + continue + } + if resolved == nil { + continue + } + recordResolvedConditionError( + state, + errs, + "agent service", + serviceName, + resolved, + ) + + var decoded bundledConnectionConfig + if err := decodeJSONMap(resolved, &decoded); err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "agent service %q: decode connections: %v", + serviceName, + err, + ), + ) + continue + } + for _, conn := range decoded.Connections { + if conn.Name == "" { + continue + } + if _, exists := collected[conn.Name]; exists { + continue + } + collected[conn.Name] = ResourceRef{ + Name: conn.Name, + ServiceName: serviceName, + Detail: formatConnectionDetail( + conn.Category, + conn.Target, + ), + } + } + } +} + +func collectManifestConnections( + ctx context.Context, + src Source, + envName string, + projectCfg *azdext.ProjectConfig, + state *State, + errs *[]error, + collected map[string]ResourceRef, +) { + for _, serviceName := range sortedServiceKeys(projectCfg) { + svc := projectCfg.Services[serviceName] + if svc == nil || svc.GetHost() != agentHost { + continue + } + enabled, err := isServiceEnabled(ctx, src, envName, serviceName) + if err != nil { + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "agent service %q deployment condition: %v", + serviceName, + err, + ), + ) + continue + } + if !enabled { + continue + } + + data := readManifestBytes(projectCfg.Path, svc.GetRelativePath()) + if data == nil { + continue + } + resources, err := agent_yaml.ExtractResourceDefinitions(data) + if err != nil { + continue + } + for _, resource := range resources { + conn, ok := resource.(agent_yaml.ConnectionResource) + if !ok || conn.Name == "" { + continue + } + if _, exists := collected[conn.Name]; exists { + continue + } + collected[conn.Name] = ResourceRef{ + Name: conn.Name, + ServiceName: serviceName, + Detail: connectionDetail(conn), + } + } + } +} + +func resolveServiceProperties( + svc *azdext.ServiceConfig, + projectRoot string, +) (map[string]any, error) { + raw := map[string]any{} + if props := svc.GetAdditionalProperties(); props != nil { + raw = props.AsMap() + } + if projectRoot == "" { + return raw, nil + } + resolved, err := foundry.ResolveFileRefs(raw, projectRoot) + if err != nil { + return nil, fmt.Errorf("resolve $ref includes: %w", err) + } + return resolved, nil +} + +func resolveAgentDefinition( + svc *azdext.ServiceConfig, + projectRoot string, +) (map[string]any, error) { + for _, candidate := range []struct { + name string + props *structpb.Struct + }{ + { + name: "service-level properties", + props: svc.GetAdditionalProperties(), + }, + { + name: "deprecated config", + props: svc.GetConfig(), + }, + } { + if candidate.props == nil || + len(candidate.props.GetFields()) == 0 { + continue + } + raw := candidate.props.AsMap() + if projectRoot == "" { + return raw, nil + } + resolved, err := foundry.ResolveFileRefs(raw, projectRoot) + if err != nil { + return nil, fmt.Errorf( + "resolve %s: %w", + candidate.name, + err, + ) + } + return resolved, nil + } + return nil, nil +} + +func decodeJSONMap(values map[string]any, out any) error { + data, err := json.Marshal(values) + if err != nil { + return err + } + return json.Unmarshal(data, out) +} + +func sortedServiceKeys(projectCfg *azdext.ProjectConfig) []string { + keys := make([]string, 0, len(projectCfg.Services)) + for name := range projectCfg.Services { + keys = append(keys, name) + } + slices.Sort(keys) + return keys +} + +func recordConnectionLoadError( + state *State, + errs *[]error, + issue string, +) { + if slices.Contains(state.ConnectionLoadErrors, issue) { + return + } + state.ConnectionLoadErrors = append( + state.ConnectionLoadErrors, + issue, + ) + *errs = append(*errs, errors.New(issue)) +} + +func recordResolvedConditionError( + state *State, + errs *[]error, + serviceType string, + serviceName string, + resolved map[string]any, +) { + if _, found := resolved["condition"]; !found { + return + } + recordConnectionLoadError( + state, + errs, + fmt.Sprintf( + "%s %q has condition in its resolved $ref; "+ + "put condition beside host in azure.yaml", + serviceType, + serviceName, + ), + ) +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go new file mode 100644 index 00000000000..031aede9cbd --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -0,0 +1,511 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package nextstep + +import ( + "testing" + + "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" +) + +func TestAssembleState_SplitConnectionsOnly(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "search-conn": { + Name: "search-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "CognitiveSearch", + "target": "https://search.example", + }), + }, + "bing-conn": { + Name: "bing-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "ApiKey", + "target": "https://api.bing.example", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.True(t, state.HasConnections) + require.Empty(t, state.ConnectionLoadErrors) + require.Len(t, state.Connections, 2) + assert.Equal(t, "bing-conn", state.Connections[0].Name) + assert.Equal(t, "bing-conn", state.Connections[0].ServiceName) + assert.Equal(t, "ApiKey | https://api.bing.example", state.Connections[0].Detail) + assert.Equal(t, "search-conn", state.Connections[1].Name) + assert.Equal(t, "CognitiveSearch | https://search.example", state.Connections[1].Detail) +} + +func TestAssembleState_ConnectionUsesServiceKeyAsName(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "azure-search": { + Name: "azure-search", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "name": "ignored-body-name", + "category": "CognitiveSearch", + "target": "https://search.example", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "azure-search", state.Connections[0].Name) +} + +func TestAssembleState_DisabledConnectionIsSkipped(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + configValues: map[string]*structpb.Value{ + "off-conn/condition": structpb.NewBoolValue(false), + }, + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "live-conn": { + Name: "live-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "ApiKey", + "target": "https://live.example", + }), + }, + "off-conn": { + Name: "off-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "ApiKey", + "target": "https://off.example", + "credentials": map[string]any{"key": "super-secret"}, + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Empty(t, state.ConnectionLoadErrors) + require.Len(t, state.Connections, 1) + assert.Equal(t, "live-conn", state.Connections[0].Name) + assert.NotContains(t, state.Connections[0].Detail, "super-secret") +} + +func TestAssembleState_DisabledConnectionSkipsRefErrors(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + configValues: map[string]*structpb.Value{ + "off-conn/condition": structpb.NewBoolValue(false), + }, + project: &azdext.ProjectConfig{ + Path: t.TempDir(), + Services: map[string]*azdext.ServiceConfig{ + "off-conn": { + Name: "off-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "$ref": "./missing-connection.yaml", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Empty(t, state.ConnectionLoadErrors) + require.False(t, state.HasConnections) + assert.Empty(t, state.Connections) +} + +func TestEvaluateConditionString_WhitespaceOnlyIsFalse(t *testing.T) { + t.Parallel() + + for _, value := range []string{"", " \t\n"} { + enabled, err := evaluateConditionString(value, nil) + require.NoError(t, err) + assert.False(t, enabled) + } +} + +func TestAssembleState_ResolvedConnectionConditionUsesRootField(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rootCondition *structpb.Value + wantEnabled bool + wantLoadError bool + }{ + { + name: "root false short circuits ref", + rootCondition: structpb.NewBoolValue(false), + wantLoadError: false, + }, + { + name: "root true remains authoritative", + rootCondition: structpb.NewBoolValue(true), + wantEnabled: true, + wantLoadError: true, + }, + { + name: "root condition absent", + wantEnabled: true, + wantLoadError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeProjectFile(t, root, "connection.yaml", ` +category: ApiKey +target: https://connection.example +condition: false +`) + configValues := map[string]*structpb.Value{} + if tc.rootCondition != nil { + configValues["conn/condition"] = tc.rootCondition + } + src := &fakeSource{ + envName: "dev", + configValues: configValues, + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "conn": { + Name: "conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "$ref": "./connection.yaml", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Equal(t, tc.wantLoadError, len(errs) > 0) + require.Equal(t, tc.wantLoadError, len(state.ConnectionLoadErrors) > 0) + require.Equal(t, tc.wantEnabled, state.HasConnections) + if tc.wantLoadError { + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ConnectionLoadErrors[0], "resolved $ref") + assert.Contains( + t, + state.ConnectionLoadErrors[0], + "put condition beside host in azure.yaml", + ) + } + if tc.wantEnabled { + require.Len(t, state.Connections, 1) + assert.Equal(t, "conn", state.Connections[0].Name) + } else { + assert.Empty(t, state.Connections) + } + }) + } +} + +func TestAssembleState_InvalidConnectionConditionIsLoadError(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + configValues: map[string]*structpb.Value{ + "bad-conn/condition": structpb.NewStringValue("${"), + }, + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "bad-conn": { + Name: "bad-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "ApiKey", + "target": "https://bad.example", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.NotEmpty(t, errs) + require.False(t, state.HasConnections) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ConnectionLoadErrors[0], `connection service "bad-conn"`) + assert.Contains(t, state.ConnectionLoadErrors[0], "invalid deployment condition") +} + +func TestAssembleState_InvalidBundledAgentConditionIsLoadError(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + configValues: map[string]*structpb.Value{ + "agent/condition": structpb.NewStringValue("${"), + }, + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "agent": { + Name: "agent", + Host: agentHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "search", + "category": "ApiKey", + "target": "https://search.example", + }, + }, + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.NotEmpty(t, errs) + require.False(t, state.HasConnections) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ConnectionLoadErrors[0], `agent service "agent"`) + assert.Contains(t, state.ConnectionLoadErrors[0], "deployment condition") +} + +func TestAssembleState_InvalidManifestAgentConditionIsLoadError(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeManifest(t, root, "src/echo", ` +template: + kind: containerAgent + name: echo +resources: + - name: search + kind: connection + category: ApiKey + target: https://search.example +`) + src := &fakeSource{ + envName: "dev", + configValues: map[string]*structpb.Value{ + "echo/condition": structpb.NewStringValue("${"), + }, + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "echo": { + Name: "echo", + Host: agentHost, + RelativePath: "src/echo", + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.NotEmpty(t, errs) + require.False(t, state.HasConnections) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ConnectionLoadErrors[0], `agent service "echo"`) + assert.Contains(t, state.ConnectionLoadErrors[0], "deployment condition") +} + +func TestAssembleState_ActiveConnectionRefErrorIsLoadError(t *testing.T) { + t.Parallel() + + root := t.TempDir() + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "broken-conn": { + Name: "broken-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "$ref": "./missing-connection.yaml", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.NotEmpty(t, errs) + require.False(t, state.HasConnections) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ConnectionLoadErrors[0], `connection service "broken-conn"`) + assert.Contains(t, state.ConnectionLoadErrors[0], "resolve $ref") +} + +func TestAssembleState_ConnectionTargetKeepsVarRef(t *testing.T) { + t.Parallel() + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "search-conn": { + Name: "search-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "CognitiveSearch", + "target": "${SEARCH_URL}", + "credentials": map[string]any{"key": "super-secret"}, + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "CognitiveSearch | ${SEARCH_URL}", state.Connections[0].Detail) + assert.NotContains(t, state.Connections[0].Detail, "super-secret") +} + +func TestAssembleState_ConnectionSourcePrecedence(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeManifest(t, root, "src/echo", ` +template: + kind: containerAgent + name: echo +resources: + - name: shared-conn + kind: connection + category: BingLLMSearch + target: https://manifest.example + - name: manifest-only + kind: connection + category: ApiKey + target: https://manifest-only.example +`) + + agent := newAgentService(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "shared-conn", + "category": "ApiKey", + "target": "https://bundled.example", + }, + map[string]any{ + "name": "bundled-only", + "category": "RemoteTool", + "target": "https://bundled-only.example", + }, + }, + }) + agent.RelativePath = "src/echo" + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + "shared-conn": { + Name: "shared-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "category": "CognitiveSearch", + "target": "https://split.example", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.True(t, state.HasConnections) + require.Len(t, state.Connections, 3) + + byName := map[string]ResourceRef{} + for _, ref := range state.Connections { + byName[ref.Name] = ref + } + assert.Equal(t, "CognitiveSearch | https://split.example", byName["shared-conn"].Detail) + assert.Equal(t, "shared-conn", byName["shared-conn"].ServiceName) + assert.Equal(t, "RemoteTool | https://bundled-only.example", byName["bundled-only"].Detail) + assert.Equal(t, "echo", byName["bundled-only"].ServiceName) + assert.Equal(t, "ApiKey | https://manifest-only.example", byName["manifest-only"].Detail) + assert.Equal(t, "echo", byName["manifest-only"].ServiceName) +} + +func TestAssembleState_BundledWinsOverManifest(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeManifest(t, root, "src/echo", ` +template: + kind: containerAgent + name: echo +resources: + - name: shared-conn + kind: connection + category: BingLLMSearch + target: https://manifest.example +`) + + agent := newAgentService(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "shared-conn", + "category": "ApiKey", + "target": "https://bundled.example", + }, + }, + }) + agent.RelativePath = "src/echo" + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "ApiKey | https://bundled.example", state.Connections[0].Detail) +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go new file mode 100644 index 00000000000..9cd598500bb --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package nextstep + +import ( + "encoding/json" + "fmt" + "strconv" + "strings" + + "github.com/azure/azure-dev/cli/azd/pkg/foundry" +) + +// evaluateCondition matches foundry.EvaluateCondition. Extensions +// pin a published azd module, so they cannot import new core +// helpers until that module is bumped. +func evaluateCondition( + value any, + getenv func(string) string, +) (bool, error) { + if value == nil { + return true, nil + } + + switch v := value.(type) { + case bool: + return v, nil + case string: + return evaluateConditionString(v, getenv) + case json.Number: + return evaluateConditionString(string(v), getenv) + case int: + return isTruthyCondition(strconv.Itoa(v)), nil + case int8: + return isTruthyCondition(strconv.Itoa(int(v))), nil + case int16: + return isTruthyCondition(strconv.Itoa(int(v))), nil + case int32: + return isTruthyCondition(strconv.Itoa(int(v))), nil + case int64: + return isTruthyCondition(strconv.FormatInt(v, 10)), nil + case uint: + return isTruthyCondition(strconv.FormatUint(uint64(v), 10)), nil + case uint8: + return isTruthyCondition(strconv.FormatUint(uint64(v), 10)), nil + case uint16: + return isTruthyCondition(strconv.FormatUint(uint64(v), 10)), nil + case uint32: + return isTruthyCondition(strconv.FormatUint(uint64(v), 10)), nil + case uint64: + return isTruthyCondition(strconv.FormatUint(v, 10)), nil + case float32: + return isTruthyCondition( + strconv.FormatFloat(float64(v), 'g', -1, 32), + ), nil + case float64: + return isTruthyCondition( + strconv.FormatFloat(v, 'g', -1, 64), + ), nil + default: + return false, fmt.Errorf( + "condition must be a string, boolean, or number", + ) + } +} + +func evaluateConditionString( + value string, + getenv func(string) string, +) (bool, error) { + if strings.TrimSpace(value) == "" { + return false, nil + } + if getenv == nil { + getenv = func(string) string { return "" } + } + expanded, err := foundry.ExpandEnv(value, getenv) + if err != nil { + return false, fmt.Errorf( + "malformed condition template: %w", + err, + ) + } + return isTruthyCondition(expanded), nil +} + +func isTruthyCondition(value string) bool { + switch value { + case "1", "true", "TRUE", "True", "yes", "YES", "Yes": + return true + default: + return false + } +} 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 4f4baa93af0..a12dfcd9d93 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 @@ -68,7 +68,6 @@ func populateManifestResources(projectPath string, state *State) { models := map[resourceKey]ResourceRef{} toolboxes := map[resourceKey]ResourceRef{} - connections := map[resourceKey]ResourceRef{} for _, svc := range state.Services { data := readManifestBytes(projectPath, svc.RelativePath) @@ -107,29 +106,14 @@ func populateManifestResources(projectPath string, state *State) { ServiceName: svc.Name, ToolboxSource: ToolboxSourceLegacyManifest, } - case agent_yaml.ConnectionResource: - if r.Name == "" { - continue - } - k := resourceKey{service: svc.Name, name: r.Name} - if _, dup := connections[k]; dup { - continue - } - connections[k] = ResourceRef{ - Name: r.Name, - ServiceName: svc.Name, - Detail: connectionDetail(r), - } } } } state.ModelRefs = sortedResourceRefs(models) state.Toolboxes = sortedResourceRefs(toolboxes) - state.Connections = sortedResourceRefs(connections) state.HasModels = len(state.ModelRefs) > 0 state.HasToolboxes = len(state.Toolboxes) > 0 - state.HasConnections = len(state.Connections) > 0 } // populateSplitToolboxes adds active toolbox dependencies to state. @@ -424,8 +408,10 @@ func readManifestBytes(projectPath, relativePath string) []byte { // 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 + return formatConnectionDetail(string(r.Category), r.Target) +} + +func formatConnectionDetail(category, target string) string { switch { case category != "" && target != "": return category + " | " + target 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 5ffe129eeb1..8973527c4e8 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 @@ -31,6 +31,10 @@ const ( // wire cmd → nextstep, so the reverse import would close a cycle. agentHost = "azure.ai.agent" + // connectionHost matches azure.yaml for an azure.ai.connection + // service. Duplicated here so nextstep stays free of cmd imports. + connectionHost = "azure.ai.connection" + // agentVersionVarFormat is the env-var name that signals a deployed // agent service. Filled with the upper-cased service key. agentVersionVarFormat = "AGENT_%s_VERSION" @@ -339,6 +343,14 @@ func assembleState(ctx context.Context, src Source, opts ...Option) (*State, []e state, &errs, ) + populateConnections( + ctx, + src, + envName, + project, + state, + &errs, + ) } if project != nil && envName != "" { 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 e70cc6637fc..6cb2b0d2500 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 @@ -159,13 +159,15 @@ type State struct { CreatedFolderDisplay string // HasModels, HasToolboxes, and HasConnections are aggregate flags. - // They describe resources. Models and connections come from - // agent manifests. Toolboxes include split services and manifest - // resources. Doctor checks skip when no matching resource exists, - // while resolvers can tailor remediation. + // They describe resources. Models still come from agent manifests. + // Toolboxes include split services and manifest resources. + // Connections include enabled azure.ai.connection services, + // bundled agent config, and legacy manifest resources. Doctor + // checks skip when no matching resource exists, while resolvers + // can tailor remediation. // - // All three flags are false when the manifest file is missing, - // malformed, or declares no resources — the walker is deliberately + // Model and toolbox flags stay false when the manifest file is + // missing, malformed, or declares no resources — the walker is // silent on those failure modes so a missing/in-flight manifest // never blocks the rest of state assembly. HasModels bool @@ -173,12 +175,18 @@ type State struct { HasConnections bool // ModelRefs, Toolboxes, and Connections list collected resources. - // ModelRefs and Connections still come from manifests. Entries are - // sorted by Name, then ServiceName, so callers can render them - // deterministically. + // ModelRefs still come from manifests. Connections come from + // enabled unified connection services or compatible sources. + // Entries are sorted by Name, then ServiceName, so callers can + // render them deterministically. ModelRefs []ResourceRef Toolboxes []ResourceRef Connections []ResourceRef + + // ConnectionLoadErrors lists failures while reading enabled + // connection configuration. Doctor fails on these instead of + // treating the project as having no connections. + ConnectionLoadErrors []string } // ResourceRef is a slim summary of a manifest resource that the From 8051333c307a0f8f5eb9ee359cab7ca2b4351dac Mon Sep 17 00:00:00 2001 From: huimiu Date: Fri, 21 Aug 2026 15:09:08 +0800 Subject: [PATCH 2/9] fix: preserve legacy bundled connections --- .../internal/cmd/nextstep/connections.go | 27 +++++++---- .../internal/cmd/nextstep/connections_test.go | 48 +++++++++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go index 97a129da14b..72a75efd880 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go @@ -354,22 +354,31 @@ func resolveAgentDefinition( continue } raw := candidate.props.AsMap() - if projectRoot == "" { - return raw, nil + resolved := raw + if projectRoot != "" { + var err error + resolved, err = foundry.ResolveFileRefs(raw, projectRoot) + if err != nil { + return nil, fmt.Errorf( + "resolve %s: %w", + candidate.name, + err, + ) + } } - resolved, err := foundry.ResolveFileRefs(raw, projectRoot) - if err != nil { - return nil, fmt.Errorf( - "resolve %s: %w", - candidate.name, - err, - ) + if !mapHasKind(resolved) { + continue } return resolved, nil } return nil, nil } +func mapHasKind(values map[string]any) bool { + kind, ok := values["kind"].(string) + return ok && strings.TrimSpace(kind) != "" +} + func decodeJSONMap(values map[string]any, out any) error { data, err := json.Marshal(values) if err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go index 031aede9cbd..8912de27ae1 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -509,3 +509,51 @@ resources: require.Len(t, state.Connections, 1) assert.Equal(t, "ApiKey | https://bundled.example", state.Connections[0].Detail) } + +func TestAssembleState_BundledLegacyConfigFallback(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path string + }{ + {name: "empty project root"}, + {name: "project root", path: t.TempDir()}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + agent := newAgentService(t, map[string]any{ + "resumeSessionOnDeploy": true, + }) + agent.Config = mustStruct(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "legacy-bundled", + "category": "ApiKey", + "target": "https://legacy.example", + }, + }, + }) + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: test.path, + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "legacy-bundled", state.Connections[0].Name) + assert.Equal(t, "ApiKey | https://legacy.example", + state.Connections[0].Detail) + }) + } +} From d019a8854f0fd684498df0adf6016dc3df6dcf39 Mon Sep 17 00:00:00 2001 From: huimiu Date: Fri, 21 Aug 2026 15:11:16 +0800 Subject: [PATCH 3/9] fix: remove diagnostics changelog entry --- cli/azd/extensions/azure.ai.agents/CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/CHANGELOG.md b/cli/azd/extensions/azure.ai.agents/CHANGELOG.md index cbec7e72735..a5d3569ccf9 100644 --- a/cli/azd/extensions/azure.ai.agents/CHANGELOG.md +++ b/cli/azd/extensions/azure.ai.agents/CHANGELOG.md @@ -1,11 +1,5 @@ # Release History -## 1.0.0-beta.12 (Unreleased) - -### Bugs Fixed - -- Diagnose unified, bundled, and legacy connection services in Doctor and next-step. - ## 1.0.0-beta.11 (2026-08-20) ### Features Added From 400af0b1bbf0f0de07b8ec44b3e3dec26a4402d7 Mon Sep 17 00:00:00 2001 From: huimiu Date: Fri, 21 Aug 2026 15:31:24 +0800 Subject: [PATCH 4/9] fix: validate connection project IDs --- .../internal/cmd/doctor/checks_connections.go | 56 ++++++++++++++---- .../cmd/doctor/checks_connections_test.go | 59 +++++++++++++++++-- 2 files changed, 101 insertions(+), 14 deletions(-) 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 2e289f13cc3..87822df5045 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 @@ -154,7 +154,18 @@ func newCheckConnections(deps Dependencies) Check { projectIDReader = readProjectResourceID } projectID, err := projectIDReader(ctx, deps.AzdClient) - if err != nil || projectID == "" { + if err != nil { + return Result{ + Status: StatusSkip, + Message: fmt.Sprintf( + "skipped: could not read %s from the current azd "+ + "environment (%s).", + projectIDVar, err), + Suggestion: "Retry `azd ai agent doctor`. If the error " + + "persists, verify the selected azd environment.", + } + } + if projectID == "" { return Result{ Status: StatusSkip, Message: fmt.Sprintf( @@ -209,18 +220,43 @@ func newCheckConnections(deps Dependencies) Check { // normalizes casing on round-trip. func parseAccountProjectFromProjectID(projectID string) (account, project string, err error) { parts := strings.Split(projectID, "/") - for i := 0; i+1 < len(parts); i++ { - switch strings.ToLower(parts[i]) { - case "accounts": - account = parts[i+1] - case "projects": - project = parts[i+1] + if len(parts) != 11 || parts[0] != "" { + return "", "", fmt.Errorf( + "invalid Foundry project resource ID %q", + projectID, + ) + } + + markers := map[int]string{ + 1: "subscriptions", + 3: "resourceGroups", + 5: "providers", + 7: "accounts", + 9: "projects", + } + for index, marker := range markers { + if !strings.EqualFold(parts[index], marker) { + return "", "", fmt.Errorf( + "invalid Foundry project resource ID %q", + projectID, + ) } } - if account == "" || project == "" { - return "", "", fmt.Errorf("missing account / project in %q", projectID) + if !strings.EqualFold(parts[6], "Microsoft.CognitiveServices") { + return "", "", fmt.Errorf( + "invalid Foundry project resource ID %q", + projectID, + ) + } + for _, index := range []int{2, 4, 6, 8, 10} { + if strings.TrimSpace(parts[index]) == "" { + return "", "", fmt.Errorf( + "invalid Foundry project resource ID %q", + projectID, + ) + } } - return account, project, nil + return parts[8], parts[10], nil } // classifyConnections produces the Pass/Fail Result by joining the 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 75fba2b89f9..2738b9cd74e 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 @@ -166,13 +166,42 @@ func TestCheckConnections_SkipsWhenProjectIDUnset(t *testing.T) { } deps := Dependencies{ assembleState: fixedAssembler(state), - readProjectResourceIDFn: fixedProjectIDReader("", errors.New("not set")), + readProjectResourceIDFn: fixedProjectIDReader("", nil), } res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) require.Equal(t, StatusSkip, res.Status) require.Contains(t, res.Message, "AZURE_AI_PROJECT_ID") } +func TestCheckConnections_SkipsWhenProjectIDReadFails(t *testing.T) { + t.Parallel() + state := &nextstep.State{ + HasConnections: true, + Connections: []nextstep.ResourceRef{ + {Name: "blob-storage", ServiceName: "chat"}, + }, + } + var probeCalls int + deps := Dependencies{ + assembleState: fixedAssembler(state), + readProjectResourceIDFn: fixedProjectIDReader( + "", errors.New("environment service unavailable"), + ), + probeFoundryConnections: func( + _ context.Context, _, _ string, + ) ([]string, error) { + probeCalls++ + return nil, nil + }, + } + res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) + require.Equal(t, StatusSkip, res.Status) + require.Contains(t, res.Message, "could not read AZURE_AI_PROJECT_ID") + require.Contains(t, res.Message, "environment service unavailable") + require.NotContains(t, res.Message, "is not set") + require.Equal(t, 0, probeCalls) +} + func TestCheckConnections_SkipsWhenProjectIDUnparsable(t *testing.T) { t.Parallel() state := &nextstep.State{ @@ -181,13 +210,25 @@ func TestCheckConnections_SkipsWhenProjectIDUnparsable(t *testing.T) { {Name: "blob-storage", ServiceName: "chat"}, }, } + var probeCalls int deps := Dependencies{ - assembleState: fixedAssembler(state), - readProjectResourceIDFn: fixedProjectIDReader("garbage", nil), + assembleState: fixedAssembler(state), + readProjectResourceIDFn: fixedProjectIDReader( + "/subscriptions/sub/resourceGroups/rg/providers/Other.Provider/"+ + "accounts/acct/projects/proj", + nil, + ), + probeFoundryConnections: func( + _ context.Context, _, _ string, + ) ([]string, error) { + probeCalls++ + return nil, nil + }, } res := runConnectionsCheck(t, deps, healthyConnectionsPrior()) require.Equal(t, StatusSkip, res.Status) require.Contains(t, res.Message, "could not parse account / project") + require.Equal(t, 0, probeCalls) } func TestCheckConnections_SkipsWhenProbeErrors(t *testing.T) { @@ -331,7 +372,7 @@ func TestParseAccountProjectFromProjectID(t *testing.T) { { name: "mixed-case segment markers", input: "/SUBSCRIPTIONS/sub-1/RESOURCEGROUPS/rg-1" + - "/providers/Microsoft.CognitiveServices/ACCOUNTS/acct-2/PROJECTS/p-2", + "/PROVIDERS/MICROSOFT.COGNITIVESERVICES/ACCOUNTS/acct-2/PROJECTS/p-2", wantAccount: "acct-2", wantProject: "p-2", }, @@ -350,6 +391,16 @@ func TestParseAccountProjectFromProjectID(t *testing.T) { input: "not-a-resource-id", wantError: true, }, + { + name: "account and project without ARM path", + input: "accounts/acct/projects/proj", + wantError: true, + }, + { + name: "extra resource segment", + input: validProjectResourceID + "/child", + wantError: true, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { From 2a0787f3e9c9814aab4867a72cbb76c73a7a5f9e Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 24 Aug 2026 12:03:54 +0800 Subject: [PATCH 5/9] fix: match configured connection names --- .../internal/cmd/nextstep/connections.go | 10 +++++++--- .../internal/cmd/nextstep/connections_test.go | 14 ++++++++------ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go index 72a75efd880..b96a29f1101 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go @@ -157,11 +157,15 @@ func collectSplitConnections( ) continue } - if _, exists := collected[serviceName]; exists { + connectionName := decoded.Name + if connectionName == "" { + connectionName = serviceName + } + if _, exists := collected[connectionName]; exists { continue } - collected[serviceName] = ResourceRef{ - Name: serviceName, + collected[connectionName] = ResourceRef{ + Name: connectionName, ServiceName: serviceName, Detail: formatConnectionDetail( decoded.Category, diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go index 8912de27ae1..4baf3d293be 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -51,7 +51,7 @@ func TestAssembleState_SplitConnectionsOnly(t *testing.T) { assert.Equal(t, "CognitiveSearch | https://search.example", state.Connections[1].Detail) } -func TestAssembleState_ConnectionUsesServiceKeyAsName(t *testing.T) { +func TestAssembleState_ConnectionUsesPayloadName(t *testing.T) { t.Parallel() src := &fakeSource{ @@ -62,7 +62,7 @@ func TestAssembleState_ConnectionUsesServiceKeyAsName(t *testing.T) { Name: "azure-search", Host: connectionHost, AdditionalProperties: mustStruct(t, map[string]any{ - "name": "ignored-body-name", + "name": "payload-name", "category": "CognitiveSearch", "target": "https://search.example", }), @@ -74,7 +74,8 @@ func TestAssembleState_ConnectionUsesServiceKeyAsName(t *testing.T) { state, errs := assembleState(t.Context(), src) require.Empty(t, errs) require.Len(t, state.Connections, 1) - assert.Equal(t, "azure-search", state.Connections[0].Name) + assert.Equal(t, "payload-name", state.Connections[0].Name) + assert.Equal(t, "azure-search", state.Connections[0].ServiceName) } func TestAssembleState_DisabledConnectionIsSkipped(t *testing.T) { @@ -438,10 +439,11 @@ resources: Path: root, Services: map[string]*azdext.ServiceConfig{ "echo": agent, - "shared-conn": { - Name: "shared-conn", + "split-service": { + Name: "split-service", Host: connectionHost, AdditionalProperties: mustStruct(t, map[string]any{ + "name": "shared-conn", "category": "CognitiveSearch", "target": "https://split.example", }), @@ -460,7 +462,7 @@ resources: byName[ref.Name] = ref } assert.Equal(t, "CognitiveSearch | https://split.example", byName["shared-conn"].Detail) - assert.Equal(t, "shared-conn", byName["shared-conn"].ServiceName) + assert.Equal(t, "split-service", byName["shared-conn"].ServiceName) assert.Equal(t, "RemoteTool | https://bundled-only.example", byName["bundled-only"].Detail) assert.Equal(t, "echo", byName["bundled-only"].ServiceName) assert.Equal(t, "ApiKey | https://manifest-only.example", byName["manifest-only"].Detail) From be843f5cdc2a6de9f57c7e0f0f50564cf530f56c Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 24 Aug 2026 14:09:06 +0800 Subject: [PATCH 6/9] fix: align Doctor connection source fallback --- .../internal/cmd/nextstep/connections.go | 44 ++++++++++--------- .../internal/cmd/nextstep/connections_test.go | 22 ++++------ .../internal/cmd/nextstep/types.go | 10 ++--- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go index b96a29f1101..696538cc684 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go @@ -28,9 +28,9 @@ type bundledConnectionConfig struct { Connections []bundledConnection `json:"connections"` } -// populateConnections merges enabled unified connection services, -// bundled agent config, and legacy manifest resources. Same Foundry -// names keep split > bundled > manifest precedence. +// populateConnections prefers enabled unified connections. +// Bundled and manifest sources are used only as a fallback. +// This matches provision's connection source selection. func populateConnections( ctx context.Context, src Source, @@ -53,24 +53,26 @@ func populateConnections( errs, collected, ) - collectBundledConnections( - ctx, - src, - envName, - projectCfg, - state, - errs, - collected, - ) - collectManifestConnections( - ctx, - src, - envName, - projectCfg, - state, - errs, - collected, - ) + if len(collected) == 0 { + collectBundledConnections( + ctx, + src, + envName, + projectCfg, + state, + errs, + collected, + ) + collectManifestConnections( + ctx, + src, + envName, + projectCfg, + state, + errs, + collected, + ) + } refs := make([]ResourceRef, 0, len(collected)) for _, ref := range collected { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go index 4baf3d293be..f188d29109b 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -397,7 +397,7 @@ func TestAssembleState_ConnectionTargetKeepsVarRef(t *testing.T) { assert.NotContains(t, state.Connections[0].Detail, "super-secret") } -func TestAssembleState_ConnectionSourcePrecedence(t *testing.T) { +func TestAssembleState_UnifiedConnectionsSuppressFallbackSources(t *testing.T) { t.Parallel() root := t.TempDir() @@ -455,18 +455,14 @@ resources: state, errs := assembleState(t.Context(), src) require.Empty(t, errs) require.True(t, state.HasConnections) - require.Len(t, state.Connections, 3) - - byName := map[string]ResourceRef{} - for _, ref := range state.Connections { - byName[ref.Name] = ref - } - assert.Equal(t, "CognitiveSearch | https://split.example", byName["shared-conn"].Detail) - assert.Equal(t, "split-service", byName["shared-conn"].ServiceName) - assert.Equal(t, "RemoteTool | https://bundled-only.example", byName["bundled-only"].Detail) - assert.Equal(t, "echo", byName["bundled-only"].ServiceName) - assert.Equal(t, "ApiKey | https://manifest-only.example", byName["manifest-only"].Detail) - assert.Equal(t, "echo", byName["manifest-only"].ServiceName) + require.Len(t, state.Connections, 1) + assert.Equal(t, "shared-conn", state.Connections[0].Name) + assert.Equal(t, "split-service", state.Connections[0].ServiceName) + assert.Equal( + t, + "CognitiveSearch | https://split.example", + state.Connections[0].Detail, + ) } func TestAssembleState_BundledWinsOverManifest(t *testing.T) { 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 6cb2b0d2500..d0d8514d70b 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 @@ -161,10 +161,10 @@ type State struct { // HasModels, HasToolboxes, and HasConnections are aggregate flags. // They describe resources. Models still come from agent manifests. // Toolboxes include split services and manifest resources. - // Connections include enabled azure.ai.connection services, - // bundled agent config, and legacy manifest resources. Doctor - // checks skip when no matching resource exists, while resolvers - // can tailor remediation. + // Connections prefer enabled azure.ai.connection services. + // Bundled agent config and legacy manifest resources are fallback + // sources. Doctor checks skip when no matching resource exists, + // while resolvers can tailor remediation. // // Model and toolbox flags stay false when the manifest file is // missing, malformed, or declares no resources — the walker is @@ -176,7 +176,7 @@ type State struct { // ModelRefs, Toolboxes, and Connections list collected resources. // ModelRefs still come from manifests. Connections come from - // enabled unified connection services or compatible sources. + // unified services or fallback sources. // Entries are sorted by Name, then ServiceName, so callers can // render them deterministically. ModelRefs []ResourceRef From ed80e7d969a9466817a3c9455c0543b51b4026e4 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 24 Aug 2026 15:22:34 +0800 Subject: [PATCH 7/9] fix: preserve empty service conditions --- .../internal/cmd/nextstep/connections_test.go | 10 +++++++++- .../azure.ai.agents/internal/cmd/nextstep/evaluate.go | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go index f188d29109b..1d9a734a548 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -146,10 +146,18 @@ func TestAssembleState_DisabledConnectionSkipsRefErrors(t *testing.T) { assert.Empty(t, state.Connections) } +func TestEvaluateConditionString_EmptyIsTrue(t *testing.T) { + t.Parallel() + + enabled, err := evaluateConditionString("", nil) + require.NoError(t, err) + assert.True(t, enabled) +} + func TestEvaluateConditionString_WhitespaceOnlyIsFalse(t *testing.T) { t.Parallel() - for _, value := range []string{"", " \t\n"} { + for _, value := range []string{" ", "\t", "\n", " \t\n"} { enabled, err := evaluateConditionString(value, nil) require.NoError(t, err) assert.False(t, enabled) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go index 9cd598500bb..c0ccff87694 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/evaluate.go @@ -69,6 +69,9 @@ func evaluateConditionString( value string, getenv func(string) string, ) (bool, error) { + if value == "" { + return true, nil + } if strings.TrimSpace(value) == "" { return false, nil } From a1827b91dc0eddf71cdf8e06879083c232cda5de Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 24 Aug 2026 16:01:52 +0800 Subject: [PATCH 8/9] fix: suppress fallback after connection load errors --- .../internal/cmd/nextstep/connections.go | 22 +++++--- .../internal/cmd/nextstep/connections_test.go | 53 +++++++++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go index 696538cc684..9e089003b97 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go @@ -44,7 +44,7 @@ func populateConnections( } collected := map[string]ResourceRef{} - collectSplitConnections( + hasSplitLoadError := collectSplitConnections( ctx, src, envName, @@ -53,7 +53,7 @@ func populateConnections( errs, collected, ) - if len(collected) == 0 { + if len(collected) == 0 && !hasSplitLoadError { collectBundledConnections( ctx, src, @@ -101,7 +101,8 @@ func collectSplitConnections( state *State, errs *[]error, collected map[string]ResourceRef, -) { +) bool { + hasLoadError := false for _, serviceName := range sortedServiceKeys(projectCfg) { svc := projectCfg.Services[serviceName] if svc == nil || svc.GetHost() != connectionHost { @@ -119,6 +120,7 @@ func collectSplitConnections( err, ), ) + hasLoadError = true continue } if !enabled { @@ -136,15 +138,18 @@ func collectSplitConnections( err, ), ) + hasLoadError = true continue } - recordResolvedConditionError( + if recordResolvedConditionError( state, errs, "connection service", serviceName, resolved, - ) + ) { + hasLoadError = true + } var decoded bundledConnection if err := decodeJSONMap(resolved, &decoded); err != nil { @@ -157,6 +162,7 @@ func collectSplitConnections( err, ), ) + hasLoadError = true continue } connectionName := decoded.Name @@ -175,6 +181,7 @@ func collectSplitConnections( ), } } + return hasLoadError } func collectBundledConnections( @@ -423,9 +430,9 @@ func recordResolvedConditionError( serviceType string, serviceName string, resolved map[string]any, -) { +) bool { if _, found := resolved["condition"]; !found { - return + return false } recordConnectionLoadError( state, @@ -437,4 +444,5 @@ func recordResolvedConditionError( serviceName, ), ) + return true } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go index 1d9a734a548..a095e4e2492 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -378,6 +378,59 @@ func TestAssembleState_ActiveConnectionRefErrorIsLoadError(t *testing.T) { assert.Contains(t, state.ConnectionLoadErrors[0], "resolve $ref") } +func TestAssembleState_UnifiedLoadErrorSuppressesFallbackSources(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeManifest(t, root, "src/echo", ` +template: + kind: containerAgent + name: echo +resources: + - name: manifest-connection + kind: connection + category: ApiKey + target: https://manifest.example +`) + + agent := newAgentService(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "bundled-connection", + "category": "ApiKey", + "target": "https://bundled.example", + }, + }, + }) + agent.RelativePath = "src/echo" + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + "broken-conn": { + Name: "broken-conn", + Host: connectionHost, + AdditionalProperties: mustStruct(t, map[string]any{ + "$ref": "./missing-connection.yaml", + }), + }, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.NotEmpty(t, errs) + require.Len(t, state.ConnectionLoadErrors, 1) + assert.Contains(t, state.ConnectionLoadErrors[0], + `connection service "broken-conn"`) + assert.False(t, state.HasConnections) + assert.Empty(t, state.Connections) +} + func TestAssembleState_ConnectionTargetKeepsVarRef(t *testing.T) { t.Parallel() From 8c5a35069f7c4af6a67c4869bbfd67a235c993c5 Mon Sep 17 00:00:00 2001 From: huimiu Date: Mon, 24 Aug 2026 16:34:48 +0800 Subject: [PATCH 9/9] fix: align bundled connection config precedence --- .../internal/cmd/nextstep/connections.go | 98 +++++++++------ .../internal/cmd/nextstep/connections_test.go | 114 ++++++++++++++++++ .../internal/cmd/resource_services_test.go | 86 +++++++++++++ 3 files changed, 260 insertions(+), 38 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go index 9e089003b97..30a6fa02dfa 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections.go @@ -215,7 +215,7 @@ func collectBundledConnections( continue } - resolved, err := resolveAgentDefinition(svc, projectCfg.Path) + resolved, err := resolveAgentConnectionConfig(svc, projectCfg.Path) if err != nil { recordConnectionLoadError( state, @@ -345,51 +345,73 @@ func resolveServiceProperties( return resolved, nil } -func resolveAgentDefinition( +func resolveAgentConnectionConfig( svc *azdext.ServiceConfig, projectRoot string, ) (map[string]any, error) { - for _, candidate := range []struct { - name string - props *structpb.Struct - }{ - { - name: "service-level properties", - props: svc.GetAdditionalProperties(), - }, - { - name: "deprecated config", - props: svc.GetConfig(), - }, - } { - if candidate.props == nil || - len(candidate.props.GetFields()) == 0 { - continue - } - raw := candidate.props.AsMap() - resolved := raw - if projectRoot != "" { - var err error - resolved, err = foundry.ResolveFileRefs(raw, projectRoot) - if err != nil { - return nil, fmt.Errorf( - "resolve %s: %w", - candidate.name, - err, - ) - } - } - if !mapHasKind(resolved) { - continue - } + inline, err := resolveAgentConnectionProperties( + svc.GetAdditionalProperties(), + projectRoot, + "service-level properties", + ) + if err != nil { + return nil, err + } + legacy, err := resolveAgentConnectionProperties( + svc.GetConfig(), + projectRoot, + "deprecated config", + ) + if err != nil { + return nil, err + } + resolved := selectAgentConnectionProperties(inline, legacy) + if len(resolved) == 0 { + return nil, nil + } + if _, found := resolved["connections"]; !found { + return nil, nil + } + return resolved, nil +} + +func resolveAgentConnectionProperties( + props *structpb.Struct, + projectRoot string, + source string, +) (map[string]any, error) { + if props == nil || len(props.GetFields()) == 0 { + return nil, nil + } + resolved := props.AsMap() + if projectRoot == "" { return resolved, nil } - return nil, nil + resolved, err := foundry.ResolveFileRefs(resolved, projectRoot) + if err != nil { + return nil, fmt.Errorf("resolve %s: %w", source, err) + } + return resolved, nil +} + +// Mirrors provision's source precedence. +// Importing project would create a package cycle. +func selectAgentConnectionProperties( + inline, legacy map[string]any, +) map[string]any { + if len(inline) == 0 { + return legacy + } + if !mapHasConnectionKind(inline) && + mapHasConnectionKind(legacy) { + return legacy + } + return inline } -func mapHasKind(values map[string]any) bool { +func mapHasConnectionKind(values map[string]any) bool { kind, ok := values["kind"].(string) - return ok && strings.TrimSpace(kind) != "" + return ok && kind != "" } func decodeJSONMap(values map[string]any, out any) error { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go index a095e4e2492..91627b5ec53 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/connections_test.go @@ -569,6 +569,120 @@ resources: assert.Equal(t, "ApiKey | https://bundled.example", state.Connections[0].Detail) } +func TestAssembleState_BundledConnectionsDoNotRequireKind(t *testing.T) { + t.Parallel() + + agent := newAgentService(t, map[string]any{ + "connections": []any{ + map[string]any{ + "name": "legacy-bundled", + "category": "ApiKey", + "target": "https://legacy.example", + }, + }, + }) + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "legacy-bundled", state.Connections[0].Name) + assert.Equal(t, "ApiKey | https://legacy.example", + state.Connections[0].Detail) +} + +func TestAssembleState_BundledConnectionsUseProvisionConfigPrecedence(t *testing.T) { + t.Parallel() + + agent := newAgentService(t, map[string]any{ + "connections": []any{ + map[string]any{ + "name": "inline-connection", + "category": "ApiKey", + "target": "https://inline.example", + }, + }, + }) + agent.Config = mustStruct(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "legacy-connection", + "category": "ApiKey", + "target": "https://legacy.example", + }, + }, + }) + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "legacy-connection", state.Connections[0].Name) + assert.Equal(t, "ApiKey | https://legacy.example", + state.Connections[0].Detail) +} + +func TestAssembleState_BundledConnectionsUseResolvedInlineConfig(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeProjectFile(t, root, "agent.yaml", ` +kind: hostedAgent +connections: + - name: inline-connection + category: ApiKey + target: https://inline.example +`) + + agent := newAgentService(t, map[string]any{ + "$ref": "./agent.yaml", + }) + agent.Config = mustStruct(t, map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "legacy-connection", + "category": "ApiKey", + "target": "https://legacy.example", + }, + }, + }) + + src := &fakeSource{ + envName: "dev", + project: &azdext.ProjectConfig{ + Path: root, + Services: map[string]*azdext.ServiceConfig{ + "echo": agent, + }, + }, + } + + state, errs := assembleState(t.Context(), src) + require.Empty(t, errs) + require.Len(t, state.Connections, 1) + assert.Equal(t, "inline-connection", state.Connections[0].Name) + assert.Equal(t, "ApiKey | https://inline.example", + state.Connections[0].Detail) +} + func TestAssembleState_BundledLegacyConfigFallback(t *testing.T) { t.Parallel() 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 e01c46c4801..e97cd7c36a0 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 @@ -230,6 +230,92 @@ func TestCollectConnections(t *testing.T) { assert.Equal(t, "zeta", connections[1].Name) } +func TestCollectConnections_UsesAgentConfigPrecedence(t *testing.T) { + t.Parallel() + + inline, err := structpb.NewStruct(map[string]any{ + "connections": []any{ + map[string]any{ + "name": "inline-connection", + "category": "ApiKey", + "target": "https://inline.example", + }, + }, + }) + require.NoError(t, err) + legacy, err := structpb.NewStruct(map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "legacy-connection", + "category": "ApiKey", + "target": "https://legacy.example", + }, + }, + }) + require.NoError(t, err) + + services := map[string]*azdext.ServiceConfig{ + "agent": { + Name: "agent", + Host: AiAgentHost, + AdditionalProperties: inline, + Config: legacy, + }, + } + + connections, err := collectConnections(services, "") + require.NoError(t, err) + require.Len(t, connections, 1) + assert.Equal(t, "legacy-connection", connections[0].Name) +} + +func TestCollectConnections_UsesResolvedInlineConfig(t *testing.T) { + t.Parallel() + + root := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(root, "agent.yaml"), + []byte( + "kind: hostedAgent\n"+ + "connections:\n"+ + " - name: inline-connection\n"+ + " category: ApiKey\n"+ + " target: https://inline.example\n", + ), + 0o600, + )) + inline, err := structpb.NewStruct(map[string]any{ + "$ref": "./agent.yaml", + }) + require.NoError(t, err) + legacy, err := structpb.NewStruct(map[string]any{ + "kind": "hostedAgent", + "connections": []any{ + map[string]any{ + "name": "legacy-connection", + "category": "ApiKey", + "target": "https://legacy.example", + }, + }, + }) + require.NoError(t, err) + + services := map[string]*azdext.ServiceConfig{ + "agent": { + Name: "agent", + Host: AiAgentHost, + AdditionalProperties: inline, + Config: legacy, + }, + } + + connections, err := collectConnections(services, root) + require.NoError(t, err) + require.Len(t, connections, 1) + assert.Equal(t, "inline-connection", connections[0].Name) +} + // TestCollectToolboxes verifies toolboxes are sourced from azure.ai.toolbox // services only. func TestCollectToolboxes(t *testing.T) {