feat(azure.ai.agents): scaffold doctor remote-check pipeline (P5.1 C10)

Wire the Cobra/runner pipe for remote (network-dependent) doctor
checks so commits C11-C17 can add individual checks (auth, foundry
endpoint reachability, RBAC, agent status) without touching the
doctor command's Cobra surface or runner internals.

Most of C10's framework was already in place from Phase 4:

  - `Check.Remote bool` (runner.go:32-37)
  - `Runner.Run` skips remote checks under `Options.LocalOnly`
    (runner.go:74-82)
  - `report.Remote = true` flipped whenever an executed check is
    Remote (runner.go:128-130)
  - `--local-only` / `--unredacted` flags bound on the Cobra surface
    and threaded through `doctor.Options`

What was missing was the *factory slot* — a `NewRemoteChecks` mirror
of `NewLocalChecks` that the doctor command appends to its check
list. Without that slot the only place to add a remote check was the
command file itself, breaking the convention established by
`NewLocalChecks` (every doctor check lives in the `doctor` package;
the command file is plumbing only).

Changes
-------

* New `internal/cmd/doctor/checks_remote.go`:
  - `NewRemoteChecks(deps Dependencies) []Check` — empty today;
    documents the conventions C11+ remote checks must follow
    (Remote: true, ctx cancellation, skip-cascade against the
    local chain via `priorBlocked`, redaction discipline under
    `!Options.Unredacted`).
  - Names the four follow-up checks the slot is reserved for so
    a future reader can immediately see the scope without
    cross-referencing the plan: C11 auth, C12 foundry endpoint,
    C16 RBAC, C17 agent status.
  - Explicitly documents that local-checks-then-remote-checks
    ordering is load-bearing for `priorBlocked` skip-cascade.

* `internal/cmd/doctor.go`:
  - `runDoctor` now builds the runner from
    `append(NewLocalChecks(deps), NewRemoteChecks(deps)...)`
    instead of `NewLocalChecks(deps)` alone. Comment on the call
    site pins the ordering contract.
  - `doctorFlags` doc comment rewritten to describe today's
    reality (the wire is fully exercised; remote-checks factory
    is empty but populated transparently when C11+ land) instead
    of the pre-C10 wording "no-op today, reserved for an upcoming
    pass."
  - `--local-only` and `--unredacted` user-visible help text
    trimmed of internal plan-tracking jargon (no more "subsequent
    commits" or "(P5 C11+)"). The first sentence now stands on
    its own and reads cleanly under `--help`.

* `internal/cmd/doctor/types.go`:
  - `Options.LocalOnly` doc comment updated from "no-op in phase 4
    — no remote checks are wired yet" to match the new post-C10
    reality (the factory returns empty today; the wire is
    exercised).

* New `internal/cmd/doctor/checks_remote_test.go` (5 tests):
  - `TestNewRemoteChecks_EmptyTodayButCallable` — pins the
    contract; when C11 lands the empty-slice assertion fires and
    forces the author to update the count.
  - `TestNewLocalAndRemoteChecks_ProductionCompositionLocalsFirst`
    — pins the load-bearing local-then-remote ordering by reading
    the actual production factories (not synthesized checks).
    Asserts (a) every check in NewLocalChecks has Remote=false,
    (b) every check in NewRemoteChecks has Remote=true, (c) no
    local check appears after any remote check in the combined
    slice. Catches a future contributor swapping the append order
    or forgetting the Remote flag on a remote check.
  - `TestRunner_LocalThenRemote_RemoteSeesLocalPriorResults` —
    asserts the runner preserves the slice order so a remote
    check's `priorBlocked` guard reads the local results.
  - `TestRunner_LocalOnly_AppendedRemoteCheck_NotInvoked` —
    exercises the production-shaped `append(local, remote...)`
    slice under `LocalOnly: true`.
  - `TestRunner_RemoteCheck_RanProducesReportRemoteFlag` —
    asserts `report.Remote = true` against the production-shaped
    slice when a remote check executes.

User-visible behavior
---------------------

None. The remote-checks factory is empty, so:

  - `azd ai agent doctor` produces the same 7-local-check report
    it did before this commit.
  - `azd ai agent doctor --local-only` produces the same 7-check
    report (no remote checks to skip).
  - `azd ai agent doctor --output json` envelope has `"remote":
    false` (no remote check executed).

The change is exclusively in the *plumbing* for the follow-up
commits.

Preflight
---------

* gofmt -s -w . — clean
* go vet ./... — clean
* go build ./... — clean
* go test ./... -count=1 — green (cmd 15.0s, doctor 6.1s, nextstep
  3.7s, etc.)
* golangci-lint run ./internal/cmd/... — 0 issues
* npx cspell lint "internal/cmd/doctor.go" "internal/cmd/doctor/**/*.go"
  --relative --config ../../.vscode/cspell.yaml --no-progress —
  0 issues across 7 files

Closes Phase 5 commit slot C10.

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

Author: Antriksh Jain

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

@@ -19,20 +19,23 @@ import (
 
 // doctorFlags are the Cobra-bound flags for `azd ai agent doctor`.
 //
-// localOnly is exposed today as a no-op: every shipped check is local
-// (Phase 4 covers checks 1–6). The Cobra surface is locked early so the
-// Phase 5 follow-up that adds remote checks does not need to introduce
-// the flag in the same commit as the new check implementations.
+// localOnly skips remote (network-dependent) checks. The runner gates
+// remote checks via the Check.Remote field (see runner.go); doctor
+// remains responsive when network is unreachable, behind a proxy, or
+// the user just wants a fast local triage. Today the remote-checks
+// factory returns an empty slice, so the flag has no observable
+// effect — but the wire is fully exercised so the remote checks land
+// transparently.
 //
 // output selects the rendering path: "text" (default, human-readable
 // with a trailing Next: block on success) or "json" (structured envelope
 // for scripted consumers).
 //
-// unredacted is reserved for Phase 5 — once remote checks surface
-// principal IDs, scope ARNs, and UPNs, this flag will toggle the
-// redaction layer. It is bound today and threaded into doctor.Options
-// so that callers (and tests) can already exercise the wire without
-// the future Phase 5 fix-up touching the Cobra surface.
+// unredacted toggles the redaction of principal IDs, scope ARNs, and
+// UPNs in the report. The flag is surfaced today and threaded into
+// doctor.Options so remote checks can read `opts.Unredacted` from
+// their CheckFunc signature; the redaction layer itself lands with
+// the first check that produces sensitive identifiers.
 type doctorFlags struct {
 	localOnly  bool
 	output     string
@@ -123,8 +126,8 @@ Exit codes:
 
 	cmd.Flags().BoolVar(
 		&flags.localOnly, "local-only", false,
-		"Run only local checks (no network calls). "+
-			"All checks are local today; this flag is reserved for an upcoming remote-checks pass.",
+		"Skip remote (network-dependent) checks. "+
+			"Useful when offline, behind a proxy, or for a fast local triage.",
 	)
 	cmd.Flags().StringVarP(
 		&flags.output, "output", "o", "text",
@@ -133,7 +136,7 @@ Exit codes:
 	cmd.Flags().BoolVar(
 		&flags.unredacted, "unredacted", false,
 		"Show raw principal IDs, scope ARNs, and UPNs in the report. "+
-			"Reserved for the upcoming remote-checks pass (no-op today).",
+			"Has no effect today; takes effect when remote checks are added.",
 	)
 
 	return cmd
@@ -171,7 +174,14 @@ func runDoctor(
 	opts doctor.Options,
 	azdClient *azdext.AzdClient,
 ) (doctor.Report, []nextstep.Suggestion) {
-	runner := doctor.Runner{Checks: doctor.NewLocalChecks(deps)}
+	// Local checks run first so their Results are available to
+	// remote checks' skip-cascade guards (each remote check inspects
+	// `prior []Result` via `priorBlocked` to decide whether to skip
+	// when an upstream local precondition failed). The slice order
+	// here is the source of truth for that contract — do not
+	// reorder.
+	checks := append(doctor.NewLocalChecks(deps), doctor.NewRemoteChecks(deps)...)
+	runner := doctor.Runner{Checks: checks}
 	report := runner.Run(ctx, opts)
 
 	// Trailing Next: block is only meaningful when checks all pass

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

@@ -0,0 +1,64 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package doctor
+
+// NewRemoteChecks returns the canonical sequence of remote (network-
+// dependent) doctor checks in execution order. Today the slice is
+// empty — the framework is wired through `--local-only`, the runner's
+// `Remote: true` gating (runner.go:74-82), and `report.Remote` (set
+// when any executed check is Remote) so that downstream commits (P5
+// C11 / C12 / C16 / C17) can append individual checks without
+// touching the doctor command's Cobra wiring.
+//
+// # Conventions for remote checks added in C11+
+//
+//   - Set Remote: true on the Check value. The runner uses this both
+//     to skip the check under --local-only and to flip
+//     report.Remote = true when the check runs (used by the JSON
+//     envelope and the formatter's "remote checks were exercised"
+//     decisions).
+//   - Forward the `Dependencies` struct to each check's closure. C11+
+//     checks that require auth credentials or a REST client should
+//     add those fields to `Dependencies` (defined in checks_local.go)
+//     and document them there. Tests inject fakes via the same fields
+//     that production wiring populates from the Cobra surface.
+//   - Skip-cascade against the local chain. Most remote checks
+//     require at least:
+//   - `local.grpc-extension` to have produced an AzdClient
+//   - `local.azure-yaml` for the project root
+//   - `local.environment-selected` for the active azd env name
+//   - `local.project-endpoint-set` for AZURE_AI_PROJECT_ENDPOINT
+//     Guard with one or more `priorBlocked(prior, "<id>")` calls and
+//     return Result{Status: StatusSkip, Message: "..."}. Doing the
+//     work inside the check (rather than in the runner) keeps the
+//     skip-message specific to the inherited failure so users see a
+//     pointed suggestion instead of a generic "upstream check failed".
+//   - Honor ctx cancellation. Remote checks own a network round trip;
+//     the runner only checks ctx.Err between checks, so a long-blocked
+//     HTTP call would otherwise stall a Ctrl-C.
+//   - When Unredacted is false (the default), elide raw principal IDs
+//     / scope ARNs / UPNs from the Message. The full payload still
+//     goes into Details for callers that opt in via --unredacted.
+//
+// # Ordering relative to local checks
+//
+// In `doctor.go:runDoctor`, remote checks are appended AFTER all
+// local checks. This is deliberate: every remote check's skip-cascade
+// reads `prior []Result`, and the local results must be available in
+// that slice when the remote check runs. The runner's loop preserves
+// the order of `Runner.Checks`, so appending remote-after-local is
+// sufficient.
+func NewRemoteChecks(deps Dependencies) []Check {
+	// Phase 5 commits C11-C17 will append entries here:
+	//   - C11: auth probe (`remote.auth`)
+	//   - C12: foundry project endpoint reachability (`remote.foundry-endpoint`)
+	//   - C16: RBAC permissions (`remote.rbac`)
+	//   - C17: agent status on backend (`remote.agent-status`)
+	// Until those land the slice is empty; the framework is fully
+	// exercised by tests using injected fake remote checks. `deps` is
+	// named (rather than `_`) so the production call site reads
+	// naturally and future contributors see the param contract; Go
+	// does not flag unused function parameters.
+	return []Check{}
+}

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

@@ -0,0 +1,194 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package doctor
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// ---- NewRemoteChecks contract ----
+
+func TestNewRemoteChecks_EmptyTodayButCallable(t *testing.T) {
+	// Today the function returns an empty slice — remote checks land
+	// in P5 commits C11-C17. This test pins the contract so a future
+	// reviewer can immediately see that an empty result is intentional
+	// (not an accidental wipe) and so that the production wiring in
+	// doctor.go can build the runner unconditionally without a nil
+	// check. A panic in NewRemoteChecks would also fail this test (the
+	// direct call below has no recover); no separate panic-guard test
+	// is needed.
+	t.Parallel()
+
+	got := NewRemoteChecks(Dependencies{})
+
+	require.NotNil(t, got, "NewRemoteChecks must return a non-nil slice "+
+		"(empty is allowed) so doctor.go can append unconditionally")
+	require.Empty(t, got, "NewRemoteChecks must return zero checks "+
+		"until the first remote check lands in P5 C11+")
+}
+
+// TestNewLocalAndRemoteChecks_ProductionCompositionLocalsFirst pins the
+// load-bearing local-then-remote ordering that doctor.go:runDoctor
+// composes via `append(NewLocalChecks, NewRemoteChecks...)`. Without
+// this test, a future contributor could accidentally swap the
+// composition order (or land a remote check inside NewLocalChecks /
+// vice versa) and every other existing test would still pass, while
+// remote checks' `priorBlocked(prior, "local.X")` skip-cascade guards
+// would silently always return false.
+//
+// We assert two invariants on the production composition:
+//
+//  1. No local check (Remote == false) appears AFTER any remote check.
+//     Locals must run first so their results are in `prior` when remote
+//     checks evaluate `priorBlocked`.
+//  2. Every check returned by NewRemoteChecks carries Remote == true
+//     (the same convention bullet documented in checks_remote.go).
+//     Forgetting the flag would cause the runner to (a) not skip the
+//     check under --local-only and (b) not flip report.Remote.
+func TestNewLocalAndRemoteChecks_ProductionCompositionLocalsFirst(t *testing.T) {
+	t.Parallel()
+
+	locals := NewLocalChecks(Dependencies{})
+	remotes := NewRemoteChecks(Dependencies{})
+
+	for i, c := range locals {
+		require.Falsef(t, c.Remote,
+			"NewLocalChecks[%d] %q has Remote=true; locals must declare Remote=false",
+			i, c.ID)
+	}
+	for i, c := range remotes {
+		require.Truef(t, c.Remote,
+			"NewRemoteChecks[%d] %q has Remote=false; remotes must declare Remote=true",
+			i, c.ID)
+	}
+
+	// Invariant 1: combined ordering must place every local before
+	// every remote. Equivalent to the contract `runDoctor` relies on.
+	combined := append(locals, remotes...)
+	sawRemote := false
+	for _, c := range combined {
+		if c.Remote {
+			sawRemote = true
+			continue
+		}
+		require.Falsef(t, sawRemote,
+			"local check %q appears after a remote check in the "+
+				"combined doctor pipeline; runDoctor's skip-cascade "+
+				"contract requires local-then-remote ordering",
+			c.ID)
+	}
+}
+
+// ---- Framework integration: local + remote interaction ----
+
+// TestRunner_LocalThenRemote_RemoteSeesLocalPriorResults proves the
+// runner preserves the order `NewLocalChecks ++ NewRemoteChecks` so a
+// remote check's skip-cascade can read the local check results. This
+// is the load-bearing contract C11+ remote checks depend on (each one
+// calls `priorBlocked(prior, "local.X")` to decide whether to skip).
+//
+// We don't use the real NewLocalChecks here because that would couple
+// this test to the live gRPC stack. Instead we synthesize a local +
+// remote pair using the same Check shape and assert the ordering.
+func TestRunner_LocalThenRemote_RemoteSeesLocalPriorResults(t *testing.T) {
+	t.Parallel()
+
+	var observed []Result
+	runner := &Runner{
+		Checks: append(
+			[]Check{
+				{ID: "local.x", Name: "local x", Fn: func(_ context.Context, _ Options, _ []Result) Result {
+					return Result{Status: StatusFail, Message: "local x failed"}
+				}},
+			},
+			Check{
+				ID:     "remote.y",
+				Name:   "remote y",
+				Remote: true,
+				Fn: func(_ context.Context, _ Options, prior []Result) Result {
+					observed = append([]Result(nil), prior...)
+					// Mirror the convention C11+ checks will follow:
+					// inspect prior, skip when a local precondition
+					// failed.
+					if priorBlocked(prior, "local.x") {
+						return Result{Status: StatusSkip, Message: "skipped: upstream local.x"}
+					}
+					return Result{Status: StatusPass, Message: "remote y ran"}
+				},
+			},
+		),
+	}
+
+	report := runner.Run(t.Context(), Options{})
+
+	require.Len(t, observed, 1, "remote check must see exactly the one local prior result")
+	require.Equal(t, "local.x", observed[0].ID)
+	require.Equal(t, StatusFail, observed[0].Status)
+	require.Equal(t, StatusSkip, report.Checks[1].Status, "remote check should have skipped via priorBlocked")
+	require.Contains(t, report.Checks[1].Message, "upstream local.x")
+}
+
+// TestRunner_LocalOnly_RemoteCheckNotInvoked complements the runner's
+// existing TestRunner_Run_LocalOnly_SkipsRemoteChecks by exercising the
+// combination used by the doctor command in production:
+// `append(NewLocalChecks, NewRemoteChecks...)`. We synthesize a remote
+// check that would Fail if invoked, then assert it produces a Skip
+// without running.
+func TestRunner_LocalOnly_AppendedRemoteCheck_NotInvoked(t *testing.T) {
+	t.Parallel()
+
+	invoked := false
+	checks := append(
+		[]Check{
+			{ID: "local.x", Name: "local x", Fn: func(_ context.Context, _ Options, _ []Result) Result {
+				return Result{Status: StatusPass, Message: "ok"}
+			}},
+		},
+		Check{
+			ID: "remote.y", Name: "remote y", Remote: true,
+			Fn: func(_ context.Context, _ Options, _ []Result) Result {
+				invoked = true
+				return Result{Status: StatusFail, Message: "remote check ran when it should not have"}
+			},
+		},
+	)
+
+	runner := &Runner{Checks: checks}
+	report := runner.Run(t.Context(), Options{LocalOnly: true})
+
+	require.False(t, invoked, "remote check function must not be invoked under --local-only")
+	require.Len(t, report.Checks, 2)
+	require.Equal(t, StatusPass, report.Checks[0].Status)
+	require.Equal(t, StatusSkip, report.Checks[1].Status)
+	require.Contains(t, report.Checks[1].Message, "local-only")
+	require.False(t, report.Remote, "report.Remote must remain false when only local checks executed")
+}
+
+// TestRunner_RemoteCheck_RanProducesReportRemoteFlag mirrors the
+// existing TestRunner_Run_RemoteCheck_FlipsReportRemoteFlag but
+// scoped to the combined local+remote shape used in production.
+func TestRunner_RemoteCheck_RanProducesReportRemoteFlag(t *testing.T) {
+	t.Parallel()
+
+	checks := append(
+		[]Check{
+			{ID: "local.x", Name: "local x", Fn: func(_ context.Context, _ Options, _ []Result) Result {
+				return Result{Status: StatusPass}
+			}},
+		},
+		Check{
+			ID: "remote.y", Name: "remote y", Remote: true,
+			Fn: func(_ context.Context, _ Options, _ []Result) Result {
+				return Result{Status: StatusPass}
+			},
+		},
+	)
+
+	report := (&Runner{Checks: checks}).Run(t.Context(), Options{})
+
+	require.True(t, report.Remote, "any executed remote check must flip report.Remote")
+}

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

@@ -103,12 +103,13 @@ type Report struct {
 }
 
 // Options are the runtime flags that influence the runner. LocalOnly
-// excludes any check whose Remote field is true (no-op in phase 4 — no
-// remote checks are wired yet; the field is exposed early so the Cobra
-// surface can be locked without churn when phase 5 lands). Unredacted
-// inverts Redacted on the produced Report; it is also surfaced to checks
-// that decide whether to include identifiers in their Message / Details
-// strings.
+// excludes any check whose Remote field is true. Today the remote-
+// checks factory (doctor.NewRemoteChecks) returns an empty slice, so
+// the flag has no observable effect; the wire is fully exercised in
+// the runner and tests so C11+ remote checks land transparently.
+// Unredacted inverts Redacted on the produced Report; it is also
+// surfaced to checks that decide whether to include identifiers in
+// their Message / Details strings.
 type Options struct {
 	LocalOnly  bool
 	Unredacted bool