fix(azure.ai.agents): doctor local checks — 3-of-3 review fix-ups (transport-error suggestions, unparseable-version message, version-agnostic Suggestion)

Three fix-ups from the 3-reviewer pass on ba365ab. All three are 3/3
consensus on the mechanic; the implementation chosen for each is the
reviewer-preferred shape (and minimally invasive).

G1 — transport-aware suggestion in checks 2 & 3 (was: GPT/Sonnet
"blocker", Opus "do not block, fix in checks 2/3 instead of with a
probe")

`azdext.NewAzdClient` constructs a *lazy* gRPC channel via
`grpc.NewClient`, so `deps.AzdClient != nil` cannot detect a
stale/unreachable `AZD_SERVER`. The transport failure first surfaces in
the next RPC — `Project().Get` in check 2, or `Environment().GetCurrent`
in check 3 — where the existing suggestions ("Run from a directory
containing azure.yaml, or `azd init`" / "Create one with `azd env new`")
are then actively wrong: they tell the user to fix project / env state
when the actual root cause is a broken channel.

Fix: a new `isTransportFailure(err)` helper inspects the gRPC status and
returns true for `codes.Unavailable` and `codes.DeadlineExceeded`.
Checks 2 and 3 swap the suggestion to "Re-run via `azd ai agent doctor`;
the extension cannot reach azd's gRPC channel." for transport-class
errors only. Server-side errors (`codes.NotFound`, `codes.Internal`,
etc.) keep the domain-specific suggestion. `codes.Canceled` is
user-initiated, not a transport failure.

Rejected: adding a dedicated probe RPC in check 1. Opus pointed out that
the probe adds latency to every doctor invocation, while the fix in
checks 2/3 achieves the same UX with zero extra RPCs.

S1 — distinguish "above floor" from "couldn't verify floor" (was: Sonnet
"blocker → Warn", GPT "Low → Warn", Opus "Low → Pass with distinguished
message")

When the extension version string is non-empty and non-"dev" but still
unparseable (e.g. "canary", "preview-beta-1", "1.2"), the previous code
fell through to the floor-pass branch with message `"azd extension
reachable (version canary)."` — indistinguishable from a genuinely-
above-floor pass.

Fix: call `parseMainVersion` explicitly in check 1 before the floor
compare. On parse failure, return `StatusPass` with a distinguished
message ("floor check skipped: version string not parseable") and
`Details["floorChecked"] = false`. Preserves the fail-open philosophy
(no nagging Warn on a build-label drift) while killing the false-green:
JSON consumers can detect the inconclusive case via the Details bit.

Rejected: Sonnet's StatusWarn proposal. Build labels that don't match
strict semver are not user errors — surfacing them as Warnings would
nag a long tail of legitimate non-standard build strings.

O1 — drop hard-coded "1.24.0" from the nil-client Suggestion (was: Opus
"Low", Sonnet "confirm Low", GPT "confirm Low")

The old Suggestion told users to "ensure azd is at least 1.24.0", but
the extension declares its actual floor in extension.yaml
(`requiredAzdVersion: ">1.23.13"`) and `go.mod` pins azd v1.23.14. A
user on 1.23.14 would have been told to perform an unneeded upgrade.

Fix: drop the version claim entirely. The Suggestion is now
version-agnostic ("Run the extension via `azd ai agent doctor` rather
than launching the extension binary directly.") so it cannot drift from
the extension's declared floor again. Test pins the version-agnostic
contract via `require.NotContains(t, got.Suggestion, "1.24.0")`.

Rejected (S2 — hardcoded `"local.azure-yaml"` ID in cascade-skip): 1/3
votes. GPT and Opus both rejected it as a maintainability preference,
not a defect; deferred indefinitely.

Net: +3 tests (transport-error swap in check 2 with 2 subcases,
transport-error swap in check 3, unparseable-version pass with 3 ver
subcases) + 1 helper test (`TestIsTransportFailure` with 7 subcases,
including the Canceled-is-not-transport boundary case). Existing
`TestCheckGRPCAndVersion_NoClient_Fails` pins the version-agnostic
contract for O1.

Pre-flight clean: gofmt, vet, build, doctor 8.8s (was 5.5s), full
extension suite green (cmd 16.0s, doctor 5.2s, nextstep 6.2s, others
unchanged), golangci-lint 0 issues, cspell 0 issues.

Per workflow precedent (2.4.1, 2.5.1, 2.6.5, 4.1.1, 4.1.2), trivial
3/3-consensus fix-ups skip a second review pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Antriksh Jain

cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local.go

@@ -10,6 +10,8 @@ import (
 	"strings"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // MinNewBackendVersion is the floor extension version required to talk to
@@ -73,7 +75,7 @@ func newCheckGRPCAndVersion(deps Dependencies) Check {
 				return Result{
 					Status:     StatusFail,
 					Message:    msg,
-					Suggestion: "Run the extension via `azd ai agent doctor` (not the extension binary directly) and ensure azd is at least 1.24.0.",
+					Suggestion: "Run the extension via `azd ai agent doctor` rather than launching the extension binary directly.",
 				}
 			}
 
@@ -85,6 +87,23 @@ func newCheckGRPCAndVersion(deps Dependencies) Check {
 				}
 			}
 
+			// If the version string is non-empty/non-"dev" but still can't be parsed
+			// (e.g. a build label like "canary" or an unexpected future format),
+			// surface Pass but mark the floor check as skipped rather than silently
+			// claiming the floor was verified.
+			if _, ok := parseMainVersion(ver); !ok {
+				return Result{
+					Status: StatusPass,
+					Message: fmt.Sprintf(
+						"azd extension reachable (version %s; floor check skipped: version string not parseable).",
+						ver),
+					Details: map[string]any{
+						"extensionVersion": ver,
+						"floorChecked":     false,
+					},
+				}
+			}
+
 			if compareVersions(ver, MinNewBackendVersion) < 0 {
 				return Result{
 					Status: StatusWarn,
@@ -132,10 +151,19 @@ func newCheckProjectConfig(deps Dependencies) Check {
 
 			resp, err := deps.AzdClient.Project().Get(ctx, &azdext.EmptyRequest{})
 			if err != nil {
+				suggestion := "Run from a directory containing `azure.yaml`, or initialize one with `azd init`."
+				if isTransportFailure(err) {
+					// `azdext.NewAzdClient` constructs a lazy gRPC channel, so the
+					// nil-client check above cannot detect a stale/unreachable
+					// `AZD_SERVER` endpoint. The transport failure surfaces here on
+					// the first RPC — swap the suggestion so the user looks at the
+					// channel, not at `azure.yaml`.
+					suggestion = "Re-run via `azd ai agent doctor`; the extension cannot reach azd's gRPC channel."
+				}
 				return Result{
 					Status:     StatusFail,
 					Message:    fmt.Sprintf("failed to get project config: %v", err),
-					Suggestion: "Run from a directory containing `azure.yaml`, or initialize one with `azd init`.",
+					Suggestion: suggestion,
 				}
 			}
 			if resp == nil || resp.Project == nil {
@@ -188,10 +216,14 @@ func newCheckEnvironmentSelected(deps Dependencies) Check {
 
 			resp, err := deps.AzdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{})
 			if err != nil {
+				suggestion := "Create one with `azd env new <name>` or select an existing one with `azd env select <name>`."
+				if isTransportFailure(err) {
+					suggestion = "Re-run via `azd ai agent doctor`; the extension cannot reach azd's gRPC channel."
+				}
 				return Result{
 					Status:     StatusFail,
 					Message:    fmt.Sprintf("failed to get current environment: %v", err),
-					Suggestion: "Create one with `azd env new <name>` or select an existing one with `azd env select <name>`.",
+					Suggestion: suggestion,
 				}
 			}
 			if resp == nil || resp.Environment == nil || resp.Environment.Name == "" {
@@ -213,6 +245,26 @@ func newCheckEnvironmentSelected(deps Dependencies) Check {
 	}
 }
 
+// isTransportFailure reports whether err is a gRPC transport-class failure
+// (channel unreachable, deadline exceeded) as opposed to a server-side
+// application error. Used by downstream checks to swap the user-facing
+// suggestion when an RPC fails because the channel itself is broken,
+// rather than because the project/environment is misconfigured.
+func isTransportFailure(err error) bool {
+	if err == nil {
+		return false
+	}
+	st, ok := status.FromError(err)
+	if !ok {
+		return false
+	}
+	switch st.Code() {
+	case codes.Unavailable, codes.DeadlineExceeded:
+		return true
+	}
+	return false
+}
+
 // coalesce returns the first non-empty string in values, or "" if all
 // are empty. Used to keep the version-floor check's Pass message
 // readable when the version string is blank.

cli/azd/extensions/azure.ai.agents/internal/cmd/doctor/checks_local_test.go

@@ -107,6 +107,9 @@ func TestCheckGRPCAndVersion_NoClient_Fails(t *testing.T) {
 	require.Contains(t, got.Message, "gRPC channel to azd unavailable")
 	require.Contains(t, got.Message, "connection refused")
 	require.NotEmpty(t, got.Suggestion)
+	// Suggestion should be version-agnostic — the extension declares its
+	// own required azd floor in extension.yaml; doctor must not duplicate it.
+	require.NotContains(t, got.Suggestion, "1.24.0")
 }
 
 func TestCheckGRPCAndVersion_NoClient_NilErr_StillFails(t *testing.T) {
@@ -184,6 +187,34 @@ func TestCheckGRPCAndVersion_AboveFloor_Passes(t *testing.T) {
 	require.Equal(t, StatusPass, got.Status)
 }
 
+// TestCheckGRPCAndVersion_UnparseableVersion_PassesButFlagsFloorSkipped
+// pins the contract for non-empty/non-"dev" version strings that fail to
+// parse (e.g. "canary", "preview-beta-1"). The check must still Pass — the
+// gRPC channel is healthy — but the message must distinguish "above floor"
+// from "couldn't verify floor", and Details["floorChecked"] must be false
+// so downstream consumers (e.g. JSON output) can tell the difference.
+func TestCheckGRPCAndVersion_UnparseableVersion_PassesButFlagsFloorSkipped(t *testing.T) {
+	t.Parallel()
+
+	client := newTestAzdClient(t, &fakeProjectServer{}, &fakeEnvironmentServer{})
+
+	for _, ver := range []string{"canary", "preview-beta-1", "1.2"} {
+		check := newCheckGRPCAndVersion(Dependencies{
+			AzdClient:        client,
+			ExtensionVersion: ver,
+		})
+		got := check.Fn(t.Context(), Options{}, nil)
+
+		require.Equalf(t, StatusPass, got.Status, "ver=%q", ver)
+		require.Containsf(t, got.Message, ver, "ver=%q: message should echo the version", ver)
+		require.Containsf(t, got.Message, "floor check skipped", "ver=%q", ver)
+		require.NotContainsf(t, got.Message, "older than", "ver=%q must not claim below-floor", ver)
+		require.Emptyf(t, got.Suggestion, "ver=%q: unparseable version should not nag", ver)
+		require.Equalf(t, false, got.Details["floorChecked"], "ver=%q", ver)
+		require.Equalf(t, ver, got.Details["extensionVersion"], "ver=%q", ver)
+	}
+}
+
 // ---- Check `local.azure-yaml` ----
 
 func TestCheckProjectConfig_NoClient_Skips(t *testing.T) {
@@ -246,6 +277,42 @@ func TestCheckProjectConfig_Pass(t *testing.T) {
 	require.Equal(t, "my-agent", got.Details["projectName"])
 }
 
+// TestCheckProjectConfig_TransportError_SwapsSuggestion locks the
+// transport-aware suggestion swap. `azdext.NewAzdClient` constructs the
+// gRPC channel lazily, so a non-nil client can still fail on the first
+// RPC if AZD_SERVER is stale or unreachable. When the resulting error
+// carries a transport-class gRPC code, the suggestion must point the
+// user at the channel rather than at `azure.yaml`.
+func TestCheckProjectConfig_TransportError_SwapsSuggestion(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		code codes.Code
+	}{
+		{"Unavailable", codes.Unavailable},
+		{"DeadlineExceeded", codes.DeadlineExceeded},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			client := newTestAzdClient(t,
+				&fakeProjectServer{err: status.Error(tc.code, "transport boom")},
+				&fakeEnvironmentServer{},
+			)
+			check := newCheckProjectConfig(Dependencies{AzdClient: client})
+			got := check.Fn(t.Context(), Options{}, nil)
+
+			require.Equal(t, StatusFail, got.Status)
+			require.Contains(t, got.Message, "failed to get project config")
+			require.Contains(t, got.Suggestion, "azd ai agent doctor")
+			require.Contains(t, got.Suggestion, "gRPC channel")
+			// And explicitly *not* the misleading "azd init" path.
+			require.NotContains(t, got.Suggestion, "azd init")
+		})
+	}
+}
+
 // ---- Check `local.environment-selected` ----
 
 func TestCheckEnvironmentSelected_NoClient_Skips(t *testing.T) {
@@ -336,6 +403,27 @@ func TestCheckEnvironmentSelected_Pass(t *testing.T) {
 	require.Equal(t, "staging", got.Details["environmentName"])
 }
 
+// TestCheckEnvironmentSelected_TransportError_SwapsSuggestion is the
+// `local.environment-selected` sibling of the project-config transport
+// test. Same rationale: a transport-class gRPC code means the channel is
+// the root cause, not the absence of an environment.
+func TestCheckEnvironmentSelected_TransportError_SwapsSuggestion(t *testing.T) {
+	t.Parallel()
+
+	client := newTestAzdClient(t,
+		&fakeProjectServer{},
+		&fakeEnvironmentServer{err: status.Error(codes.Unavailable, "transport boom")},
+	)
+	check := newCheckEnvironmentSelected(Dependencies{AzdClient: client})
+	got := check.Fn(t.Context(), Options{}, nil)
+
+	require.Equal(t, StatusFail, got.Status)
+	require.Contains(t, got.Message, "failed to get current environment")
+	require.Contains(t, got.Suggestion, "azd ai agent doctor")
+	require.Contains(t, got.Suggestion, "gRPC channel")
+	require.NotContains(t, got.Suggestion, "azd env new")
+}
+
 // ---- NewLocalChecks ordering / IDs ----
 
 func TestNewLocalChecks_OrderAndIDs(t *testing.T) {
@@ -424,3 +512,33 @@ func TestCoalesce(t *testing.T) {
 	require.Equal(t, "", coalesce("", "", ""))
 	require.Equal(t, "", coalesce())
 }
+
+// ---- transport-failure helper ----
+
+func TestIsTransportFailure(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{"nil error", nil, false},
+		{"plain error (not a status)", errors.New("boom"), false},
+		{"Unavailable", status.Error(codes.Unavailable, "x"), true},
+		{"DeadlineExceeded", status.Error(codes.DeadlineExceeded, "x"), true},
+		// Server-side errors must NOT swap the suggestion: the project / env
+		// check then reports the real domain failure with its own wording.
+		{"NotFound", status.Error(codes.NotFound, "x"), false},
+		{"Internal", status.Error(codes.Internal, "x"), false},
+		{"InvalidArgument", status.Error(codes.InvalidArgument, "x"), false},
+		// Canceled is user-initiated, not a transport issue.
+		{"Canceled", status.Error(codes.Canceled, "x"), false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			require.Equal(t, tc.want, isTransportFailure(tc.err))
+		})
+	}
+}