fix: diagnose unified connection services

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
-//     "<Category> | <Target>" identifier surfaced by the C2 walker).
+//     (each entry carries Name, ServiceName, Detail — the
+//     "<Category> | <Target>" 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.",
 				}
 			}
 
@@ -137,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(
@@ -192,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
@@ -281,11 +334,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,

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) {
@@ -125,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{
@@ -140,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) {
@@ -220,11 +302,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 +327,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)")
 }
 
@@ -288,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",
 		},
@@ -307,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) {

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")
 }