Skip to content

Commit 4e293f7

Browse files
Antriksh JainCopilot
andcommitted
fix(azure.ai.agents): align doctor types to design spec
Cross-checked the committed Phase 4.1 doctor types (`internal/cmd/doctor/ types.go`) against the JSON envelope contract in the design spec (`cli/azd/docs/design/azd-ai-agent-nextsteps.md`, lines 357-383). Found nine drift deltas. Per the user's "design spec is a guide, not a contract — keep what's better" direction, this commit adopts the design's unintentional-drift fixes and documents the deliberate improvements that diverge from the spec. Adopted from the design spec - `CurrentSchemaVersion`: "1" → "1.0" (standard semver-ish form). - New field `Result.Links []string` (`json:"links,omitempty"`) for TSG / docs URLs surfaced by Phase 5 remote checks. - New field `Result.DurationMs int64` (`json:"durationMs,omitempty"`) populated by the runner via `time.Since(start).Milliseconds()`. Observability for slow checks. - `Report.Summary` JSON tag → `json:"-"`. Still computed in-memory and consumed by `Report.ExitCode()`; not part of the wire format. Consumers can compute it from `checks[]` if needed. - Namespaced check IDs (e.g., `local.azure-yaml`, `remote.rbac`) called out in doc comments. Phase 4.2 onward emits IDs in this form. Kept as deliberate improvements over the design spec - JSON tags `name`/`message`/`suggestion` retained (design uses `title`/`detail`/`fix`). `name` is shorter and universal; `message` is primary text vs `detail`'s "supplementary" connotation; `suggestion` is broader than `fix` (covers warnings, not just blocking failures). - `Result.Details map[string]any` (`json:"details,omitempty"`) kept as the design-spec extension for Phase 5 RBAC payloads (role lists, scope ARNs, etc.). Doc comment now explicitly identifies this as a documented extension. Runner - `runner.go` now wraps every `check.Fn(...)` call with `start := time.Now()` / `result.DurationMs = time.Since(start). Milliseconds()`. Single integration point, applies uniformly to all checks present and future. Tests - `TestRunner_Run_DurationMsIsPopulated` — 5ms sleep inside a check function, asserts `DurationMs >= 1`. All 16 existing tests still pass. Cspell - Added `nextsteps` override for `internal/cmd/doctor/types.go`. Doc comments now reference the design doc filename (`azd-ai-agent-nextsteps.md`), which trips the existing `nextstep` (singular) entry. No new review pass on this commit — same trivial align-to-design shape as 4.1.1 (precedent: doc/contract cleanups land pre-validated). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 20f83ae commit 4e293f7

4 files changed

Lines changed: 56 additions & 18 deletions

File tree

cli/azd/.vscode/cspell.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,9 @@ overrides:
409409
- filename: extensions/azure.ai.agents/internal/cmd/init_locations.go
410410
words:
411411
- swedencentral
412+
- filename: "**/extensions/azure.ai.agents/internal/cmd/doctor/types.go"
413+
words:
414+
- nextsteps
412415
- filename: docs/code-coverage-guide.md
413416
words:
414417
- covdata

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33

44
package doctor
55

6-
import "context"
6+
import (
7+
"context"
8+
"time"
9+
)
710

811
// CheckFunc is the signature every check satisfies. Checks are invoked
912
// sequentially by the Runner; each receives the immutable Options and the
@@ -90,7 +93,9 @@ func (r *Runner) Run(ctx context.Context, opts Options) Report {
9093
continue
9194
}
9295

96+
start := time.Now()
9397
result := check.Fn(ctx, opts, report.Checks)
98+
result.DurationMs = time.Since(start).Milliseconds()
9499
// Pin the ID + Name at the runner — the design's table is the
95100
// source of truth, and individual check functions should not be
96101
// able to drift from it.

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package doctor
66
import (
77
"context"
88
"testing"
9+
"time"
910

1011
"github.com/stretchr/testify/require"
1112
)
@@ -322,3 +323,19 @@ func TestRunner_Run_UnredactedFlipsRedacted(t *testing.T) {
322323

323324
require.False(t, report.Redacted, "Unredacted true should flip Redacted to false")
324325
}
326+
327+
func TestRunner_Run_DurationMsIsPopulated(t *testing.T) {
328+
t.Parallel()
329+
330+
runner := &Runner{Checks: []Check{{ID: "1", Name: "x", Fn: func(_ context.Context, _ Options, _ []Result) Result {
331+
// Sleep long enough that even a millisecond-resolution clock observes a tick.
332+
time.Sleep(5 * time.Millisecond)
333+
return Result{Status: StatusPass, Message: "ok"}
334+
}}}}
335+
336+
report := runner.Run(t.Context(), Options{})
337+
338+
require.Len(t, report.Checks, 1)
339+
require.GreaterOrEqual(t, report.Checks[0].DurationMs, int64(1),
340+
"runner must time each check and stamp DurationMs (got %d)", report.Checks[0].DurationMs)
341+
}

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

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@
1919
// can be unit-tested without a process-level shim.
2020
package doctor
2121

22-
// CurrentSchemaVersion is the version stamped onto the JSON envelope. Bump
23-
// when the JSON shape changes in a non-additive way; additive changes
24-
// (new optional fields, new status values that consumers can ignore) do
25-
// not require a bump. Consumers should treat unknown statuses as "pass"
26-
// for the purposes of summary aggregation only when this version equals
27-
// the one they were built against.
28-
const CurrentSchemaVersion = "1"
22+
// CurrentSchemaVersion is the version stamped onto the JSON envelope. The
23+
// value matches the design spec (`docs/design/azd-ai-agent-nextsteps.md`,
24+
// "Exit codes & JSON output" section). Bump on non-additive shape changes;
25+
// additive changes (new optional fields, new status values) do not require
26+
// a bump.
27+
const CurrentSchemaVersion = "1.0"
2928

3029
// Status is the outcome of a single check. The set is closed; runners and
3130
// formatters branch exhaustively on these four values.
@@ -49,21 +48,30 @@ const (
4948

5049
// Result captures the outcome of one check.
5150
//
52-
// ID is a stable identifier (the design pins these to "1".."12"). Name is
53-
// a short human-readable title for the text formatter; Message is the
54-
// one-line summary that always renders. Details and Suggestion are
55-
// optional — Details is a structured map for machine consumers (the JSON
56-
// formatter emits it as an object; the text formatter renders each
57-
// key-value pair on an indented line), Suggestion is a single actionable
58-
// command or instruction (the text formatter renders it on its own line
59-
// prefixed with "→ ").
51+
// ID is a stable namespaced identifier (`local.azure-yaml`,
52+
// `remote.rbac`, etc.). Name is a short human-readable title; Message is
53+
// the one-line summary that always renders. Suggestion is a single
54+
// actionable command or instruction (the text formatter renders it after
55+
// the message, indented). Links is an optional slice of URLs (TSG pages,
56+
// learn.microsoft.com docs) that the formatter renders below the
57+
// suggestion. DurationMs is populated by the Runner per check.
58+
//
59+
// JSON tags are extension-owned: the wire shape includes `links` and
60+
// `durationMs` (matching the design spec at
61+
// `docs/design/azd-ai-agent-nextsteps.md`) plus a `details` extension
62+
// field (omitted from the design's example but required for Phase 5
63+
// remote checks that surface structured payload — role lists, scope
64+
// ARNs). `details` is `omitempty`, so consumers built against the
65+
// design's schema ignore the extra field and remain compatible.
6066
type Result struct {
6167
ID string `json:"id"`
6268
Name string `json:"name"`
6369
Status Status `json:"status"`
6470
Message string `json:"message,omitempty"`
65-
Details map[string]any `json:"details,omitempty"`
6671
Suggestion string `json:"suggestion,omitempty"`
72+
Links []string `json:"links,omitempty"`
73+
DurationMs int64 `json:"durationMs,omitempty"`
74+
Details map[string]any `json:"details,omitempty"`
6775
}
6876

6977
// Summary is the aggregate count of results by status. Computed by the
@@ -81,12 +89,17 @@ type Summary struct {
8189
// checks are wired. Redacted is the inverse of the --unredacted flag and
8290
// indicates whether the formatter scrubbed identifiers in user-facing
8391
// strings.
92+
//
93+
// Summary is computed by the Runner for ExitCode and the text formatter,
94+
// but is excluded from the JSON envelope (consumers iterate Checks if they
95+
// need totals). Excluding it keeps the wire shape aligned with the design
96+
// spec.
8497
type Report struct {
8598
SchemaVersion string `json:"schemaVersion"`
8699
Remote bool `json:"remote"`
87100
Redacted bool `json:"redacted"`
88101
Checks []Result `json:"checks"`
89-
Summary Summary `json:"summary"`
102+
Summary Summary `json:"-"`
90103
}
91104

92105
// Options are the runtime flags that influence the runner. LocalOnly

0 commit comments

Comments
 (0)