feat(azure.ai.agents): add doctor check local.manual-env-vars (P5.1 C9)

Adds the 7th local doctor check, surfacing the same MissingManualVars
signal that the post-init `Next:` block renders — but at any point in
the flow (post-deploy, mid-debug, before reporting a bug). Closes the
"manual config values missing" branch from issue Azure#7975
lines 117-127 on the doctor side.

## Why

The doctor's job is to short-circuit user frustration: "I get the same
guidance from `azd ai agent doctor` regardless of where I am in the
flow." Today the manual-vars signal only surfaces at the tail of `azd
ai agent init`, so a user who hits the issue mid-cycle (e.g. they
cloned a friend's project, ran `azd env select`, and went straight to
`azd ai agent run`) sees no clear pointer to the missing config.

## What

- `internal/cmd/doctor/checks_manual_env.go` — new check produces ID
  `local.manual-env-vars`, name "manual env vars set". On Pass, message
  is "no manual env vars are missing"; on Fail, lists every missing var
  in the message and stages a paste-ready `azd env set <FIRST> <value>`
  suggestion (sorted alphabetically — the renderer paints Suggestion as
  a single line, multi-line breaks indentation). When 2+ vars are
  missing the suggestion adds a "Repeat for each of the other
  variables listed above." clause; when exactly 1 is missing the bare
  command is the suggestion (no misleading "and others" wording).
- `internal/cmd/doctor/checks_local.go` — adds the check as the 7th
  entry in `NewLocalChecks` (after `local.agent-yaml-valid`); introduces
  a lowercase `assembleState` field on the exported `Dependencies`
  struct as a test seam (production code leaves it nil; tests inject
  directly).
- Skip cascade: skips when AzdClient is nil OR when
  `local.agent-yaml-valid` failed/skipped OR when
  `local.environment-selected` failed/skipped. The first guard
  transitively covers azure-yaml → agent-service-detected →
  agent-yaml-valid; the second is required because env-selection is a
  sibling chain (not upstream of agent-yaml-valid) and
  `nextstep.AssembleState` silently early-exits its detectMissingVars
  block when no env is selected (state.go: `if project != nil &&
  envName != ""`). Without the env guard the check would falsely Pass
  in a state where no env values were ever examined.

## Architecture: why reuse `nextstep.AssembleState`

The "manual vs infra" classification logic lives in nextstep — the same
place that drives every other `Next:` recommendation. Adding a separate
classifier inside the doctor would split the source of truth, and
future improvements (e.g. C1's Bicep-output discovery replacing the
`AZURE_` prefix shortcut) would have to be ported twice. Forwarding to
`AssembleState` keeps the doctor as a presentation layer.

## Tests

12 new sub-tests in `checks_manual_env_test.go`:

- No client → Skip
- Prior `local.agent-yaml-valid` failed → Skip
- Prior `local.agent-yaml-valid` skipped → Skip (cascade propagation)
- Prior `local.environment-selected` failed → Skip (regression for
  Opus xhigh review HIGH finding; asserts assembler is NOT called via
  t.Fatalf in the fake)
- Prior `local.environment-selected` skipped → Skip (symmetric)
- 0 missing → Pass with "no manual env vars are missing"
- 1 missing → Fail with bare `azd env set <NAME> <value>` suggestion
  (asserts no "Repeat" / "likewise" wording — regression for Sonnet
  4.6 review MEDIUM finding)
- 4 missing → Fail with sorted list; suggestion has "Repeat for each
  of the other variables" clause
- nil State from assembler → Fail with assembly error message
- nil State + nil errs → Fail with fallback "unknown error" message
- Non-fatal errs but State populated → Pass (state.MissingManualVars
  is the authoritative signal; ancillary errors like missing
  AI_AGENT_PENDING_PROVISION key don't dirty the result)
- Existing `TestNewLocalChecks_OrderAndIDs` extended from 6 to 7 checks;
  new `TestNewLocalChecks_IncludesManualEnvVarsLast` pins the
  agent-yaml-valid → manual-env-vars ordering invariant so a future
  reorder can't silently break the skip-cascade.

## Out of scope

- Bicep-output classifier (B1 fix / C1) — `assembleState` today routes
  non-`AZURE_*` Bicep outputs (e.g. `TOOLBOX_*`) into MissingManualVars
  rather than MissingInfraVars. This check will surface that bug
  verbatim until C1 lands; that's the correct phasing because (a)
  every Phase 5 consumer benefits from the same fix at once, and (b)
  showing the current behavior in the doctor makes the bug debuggable
  in the meantime.

Refs: Azure#7975 lines 117-127, 308-312 (issue spec)
Refs: Azure#8057 (PR being implemented)

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

@@ -9,6 +9,8 @@ import (
 	"strconv"
 	"strings"
 
+	"azureaiagent/internal/cmd/nextstep"
+
 	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -39,12 +41,23 @@ type Dependencies struct {
 	AzdClient        *azdext.AzdClient
 	AzdClientErr     error
 	ExtensionVersion string
+
+	// assembleState is a test seam: when non-nil it replaces the
+	// production `nextstep.AssembleState` call inside the
+	// `local.manual-env-vars` check, letting unit tests inject a
+	// pre-computed State without standing up a temp project on disk.
+	// Lowercase so external packages cannot reach it. Production code
+	// (NewLocalChecks via the Cobra wiring) leaves it nil.
+	assembleState func(ctx context.Context, client *azdext.AzdClient) (*nextstep.State, []error)
 }
 
 // NewLocalChecks returns the canonical sequence of local doctor checks
-// in execution order. Phase 4.2 covered checks 1-3; Phase 4.3 adds
+// in execution order. Phase 4.2 covered checks 1-3; Phase 4.3 added
 // checks 4-6 (agent service detected, project endpoint set, agent.yaml
-// valid).
+// valid). Phase 5 C9 appends check 7 (manual env vars set) — local
+// check #9 in the design's numbered table (renumbered here because
+// remote checks 7-8 are gated behind --local-only until the runner
+// refactor lands in C10).
 func NewLocalChecks(deps Dependencies) []Check {
 	return []Check{
 		newCheckGRPCAndVersion(deps),
@@ -53,6 +66,7 @@ func NewLocalChecks(deps Dependencies) []Check {
 		newCheckAgentServiceDetected(deps),
 		newCheckProjectEndpointSet(deps),
 		newCheckAgentYAMLValid(deps),
+		newCheckManualEnvVars(deps),
 	}
 }
 

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

@@ -443,7 +443,7 @@ func TestNewLocalChecks_OrderAndIDs(t *testing.T) {
 	t.Parallel()
 
 	checks := NewLocalChecks(Dependencies{})
-	require.Len(t, checks, 6)
+	require.Len(t, checks, 7)
 
 	want := []struct {
 		id     string
@@ -456,6 +456,7 @@ func TestNewLocalChecks_OrderAndIDs(t *testing.T) {
 		{"local.agent-service-detected", "agent service in azure.yaml", false},
 		{"local.project-endpoint-set", "AZURE_AI_PROJECT_ENDPOINT set", false},
 		{"local.agent-yaml-valid", "agent.yaml valid (per service)", false},
+		{"local.manual-env-vars", "manual env vars set", false},
 	}
 	for i, w := range want {
 		require.Equal(t, w.id, checks[i].ID, "index %d", i)

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

@@ -0,0 +1,144 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package doctor
+
+import (
+	"context"
+	"fmt"
+	"slices"
+	"strings"
+
+	"azureaiagent/internal/cmd/nextstep"
+
+	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
+)
+
+// newCheckManualEnvVars produces Check `local.manual-env-vars` — the
+// "manual config values not set" diagnostic.
+//
+// "Manual" env vars are values referenced by `${...}` syntax inside an
+// agent.yaml whose names are NOT declared as outputs of the project's
+// infrastructure (Bicep / Terraform). They are operator-supplied:
+// third-party API keys, model deployment names, hand-rolled connection
+// strings. They have to be set in the active azd environment before
+// `azd ai agent run` (local) or `azd deploy` (Azure) can resolve the
+// agent.yaml — otherwise the running agent sees the literal `${KEY}`
+// string and almost certainly fails on first use.
+//
+// The classification of "manual" vs "infra" lives in nextstep's
+// AssembleState (the same pipeline that drives the `Next:` renderer's
+// per-state guidance). This check forwards the result so the doctor
+// surfaces the same signal users see at the end of `azd ai agent init`
+// — no second source of truth, no drift.
+//
+// Source-of-truth: issue Azure/azure-dev#7975 "Example output (project
+// ready, but manual config values missing)" lines 117-127. The doctor
+// reports the gap; the post-init `Next:` block (resolver.go, manual-vars
+// branch) tells the user what to type.
+//
+// Skip cascade — this check skips when any of the following hold:
+//
+//   - deps.AzdClient is nil (gRPC channel unavailable). Check
+//     `local.grpc-extension` will already have failed with the actionable
+//     error.
+//   - `local.agent-yaml-valid` failed or was skipped. A broken agent.yaml
+//     produces an empty MissingManualVars (the classifier can't extract
+//     references it can't parse), which would mislead the user into
+//     thinking nothing was missing. This guard transitively covers the
+//     azure-yaml → agent-service-detected → agent-yaml-valid arm of the
+//     local-check chain (each step's own skip-cascade propagates here).
+//   - `local.environment-selected` failed or was skipped.
+//     `nextstep.AssembleState` early-exits its `detectMissingVars` block
+//     when no env is selected (state.go: `if project != nil && envName != ""`).
+//     Without this guard the check would silently produce a Pass
+//     ("no manual env vars are missing") in a state where it never even
+//     looked at any env values — the exact false-Pass the doctor exists
+//     to prevent. `environment-selected` is a sibling chain off
+//     `azure-yaml` (not upstream of `agent-yaml-valid`), so the previous
+//     guard does not cover it transitively.
+//
+// On Fail the check lists every missing var in the Message (callers can
+// also iterate `Details["missingManualVars"]` for the structured payload).
+// The Suggestion picks the first missing var as a paste-ready example
+// rather than concatenating one `azd env set` line per var: the formatter
+// renders Suggestion as a single line, and a paragraph of newlines would
+// break the indentation. Users see the full list in the Message and one
+// concrete command to copy-paste.
+func newCheckManualEnvVars(deps Dependencies) Check {
+	return Check{
+		ID:   "local.manual-env-vars",
+		Name: "manual env vars set",
+		Fn: func(ctx context.Context, _ Options, prior []Result) Result {
+			if deps.AzdClient == nil {
+				return Result{Status: StatusSkip, Message: "skipped: azd extension not reachable"}
+			}
+			if priorBlocked(prior, "local.agent-yaml-valid") {
+				return Result{Status: StatusSkip, Message: "skipped: agent.yaml check failed or skipped"}
+			}
+			if priorBlocked(prior, "local.environment-selected") {
+				// Without an azd env, AssembleState's detectMissingVars
+				// block is skipped (state.go:258), so MissingManualVars
+				// would be empty and the check would falsely Pass.
+				return Result{
+					Status:  StatusSkip,
+					Message: "skipped: no azd environment selected (cannot resolve agent.yaml variables)",
+				}
+			}
+
+			assembler := deps.assembleState
+			if assembler == nil {
+				assembler = func(c context.Context, client *azdext.AzdClient) (*nextstep.State, []error) {
+					return nextstep.AssembleState(c, client)
+				}
+			}
+			state, errs := assembler(ctx, deps.AzdClient)
+			if state == nil {
+				// AssembleState always returns a non-nil State even when errs
+				// is non-empty — but defend against a future contract change
+				// so this check can't be the one to panic-dereference.
+				cause := "unknown error"
+				if len(errs) > 0 {
+					cause = errs[0].Error()
+				}
+				return Result{
+					Status:     StatusFail,
+					Message:    fmt.Sprintf("failed to assemble agent state: %s", cause),
+					Suggestion: "Re-run `azd ai agent doctor`; the state assembly returned nil unexpectedly.",
+				}
+			}
+
+			missing := slices.Clone(state.MissingManualVars)
+			slices.Sort(missing)
+
+			if len(missing) == 0 {
+				return Result{
+					Status:  StatusPass,
+					Message: "no manual env vars are missing",
+				}
+			}
+
+			// Single-line Suggestion: pin a paste-ready command for the
+			// first (sorted) missing var, plus a clause pointing at the
+			// rest only when there ARE additional entries. When exactly
+			// one var is missing the bare command is the right
+			// instruction — adding "and likewise for the others" implies
+			// the user missed something they didn't.
+			suggestion := fmt.Sprintf("Run `azd env set %s <value>`.", missing[0])
+			if len(missing) > 1 {
+				suggestion += " Repeat for each of the other variables listed above."
+			}
+
+			return Result{
+				Status: StatusFail,
+				Message: fmt.Sprintf(
+					"%d manual env var(s) referenced by agent.yaml are not set in the azd environment: %s",
+					len(missing), strings.Join(missing, ", ")),
+				Suggestion: suggestion,
+				Details: map[string]any{
+					"missingManualVars": missing,
+				},
+			}
+		},
+	}
+}