Skip to content

Commit f2528f1

Browse files
Antriksh JainCopilot
andcommitted
feat(azure.ai.agents): qualify single-agent post-deploy Next: block (P5.1 C6, fix B9)
Fix B9 from issue Azure#7975 (lines 228-242). Pre-C6, `ResolveAfterDeploy` stripped the agent name when there was exactly one agent service in state, emitting: Next: azd ai agent show -- verify the deployed agent is running azd ai agent invoke '<payload>' -- send a sample request to the deployed agent That output had two problems: 1. **`azd ai agent show` (no name)** runs interactive resolution that relies on the user's terminal state. When the artifact note is copy-pasted into a different project (or read days later when the single-agent project may have grown a second agent), the unqualified command picks the wrong target or prompts. The spec example at lines 231-232 shows the qualified form even in the single-agent case for exactly this copy-paste-safety reason. 2. **Description copy was generic** ("verify the deployed agent is running" / "send a sample request to the deployed agent"). Per spec lines 231-241 the descriptions are intentionally tighter and identify which agent each row refers to in the multi-agent layout. C6 changes: - `ResolveAfterDeploy` now always emits service-qualified commands regardless of `len(state.Services)`. The pre-C6 unqualified branch is gone. - Descriptions reshape: * Single agent (`len(state.Services) == 1`): `verify it's running` / `test the deployment` * Multi-agent (`len(state.Services) >= 2`): `verify <name> is running` / `test <name>` - Multi-agent layout now groups all `show` lines first, then all `invoke` lines (was interleaved per service). Matches spec example at lines 238-241. Single-agent output is unchanged in layout — with one service the pass-1/pass-2 split still produces show-then- invoke order. - README hint placement: in the new pass-2 invoke loop the per-agent hint is emitted immediately after that agent's invoke line, so a reader can scan rows top-to-bottom and find each agent's hint in context. Single-agent placement is identical to pre-C6. - `AfterDeployOpts.ForceQualified` is kept as a NO-OP for backward compatibility. Callers (doctor.go line 239 passes it for filtered states) still compile and produce identical output. The doc comment is updated to mark it as a no-op and explain why (the single-agent unqualified heuristic it overrode is gone). Tests: - `TestResolveAfterDeploy` rewritten — all single-agent expectations now assert qualified commands (`azd ai agent show echo` / `azd ai agent invoke echo '...'`) and per-spec descriptions. - New subtest for multi-agent grouped ordering (shows-then-invokes) asserts shows in service-declaration order, then invokes in the same order, each row carrying the per-agent descriptive text. - New subtest for README hint placement in the multi-agent layout asserts the hint follows the invoke line for the service that triggered it, even with the new grouped ordering. - The two `ForceQualified` subtests are kept and rewritten as backward-compat assertions — they now compare against the no-opts baseline and prove the flag is a true no-op. Caller impact: - `doctor.go:235-239` passes `ForceQualified: totalServices > 1`. Output is identical to its old behavior (totalServices==1 used to emit unqualified; now emits qualified, but doctor's filtered-state callsite always wanted qualified anyway — that's literally the comment at the callsite). Spec source: issue Azure#7975 lines 228-242 + the C6 row in the P5.1 commit plan. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a93ff7f commit f2528f1

4 files changed

Lines changed: 147 additions & 70 deletions

File tree

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

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -218,25 +218,16 @@ func resolveDoctorTrailing(ctx context.Context, azdClient *azdext.AzdClient) []n
218218
}
219219

220220
if anyServiceDeployed(state.Services) {
221-
// Capture the total agent-service count BEFORE filtering. The
222-
// resolver's `len(state.Services) == 1` heuristic ordinarily
223-
// keys "should I emit no-arg show/invoke commands?" off the
224-
// total count of agent services in azure.yaml. Once we filter
225-
// to deployed-only, that heuristic breaks: a 2-service project
226-
// with 1 deployed would emit `azd ai agent show` (no name),
227-
// but runtime `resolveAgentService` still sees both services
228-
// in azure.yaml and would either prompt or error. Forcing
229-
// qualified suggestions whenever azure.yaml has multiple
230-
// services preserves copy-paste correctness in the partial-
231-
// deploy case and is a no-op when all services are deployed
232-
// (the resolver naturally qualifies len > 1 anyway).
233-
totalServices := len(state.Services)
234-
filtered := filterDeployedServices(state)
221+
// ResolveAfterDeploy always emits service-qualified
222+
// `azd ai agent show <name>` / `invoke <name> ...` commands
223+
// post-B9 (issue #7975), so it's safe to pass a filtered
224+
// (deployed-only) State directly — the suggestions remain
225+
// copy-paste correct even when azure.yaml has additional
226+
// undeployed services that are absent from the filtered set.
235227
return nextstep.ResolveAfterDeploy(
236-
filtered,
228+
filterDeployedServices(state),
237229
doctorCachedPayload(ctx, azdClient),
238230
doctorReadmeExists(ctx, azdClient),
239-
nextstep.AfterDeployOpts{ForceQualified: totalServices > 1},
240231
)
241232
}
242233

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,3 +326,32 @@ func TestFilterDeployedServices(t *testing.T) {
326326
assert.Len(t, state.Services, 2, "clone must not modify input")
327327
})
328328
}
329+
330+
// TestFilterDeployedServices_ChainedIntoResolveAfterDeploy locks in the
331+
// end-to-end contract for doctor's post-deploy guidance block: when the
332+
// project has multiple agent services but only one is deployed, the
333+
// filtered state flowed through ResolveAfterDeploy must still emit a
334+
// service-qualified command — i.e. the user sees `azd ai agent show
335+
// <name>` rather than `azd ai agent show` (no arg). Pre-B9 this
336+
// invariant was enforced via AfterDeployOpts.ForceQualified at the
337+
// caller; post-B9 the resolver always qualifies. This test would have
338+
// caught a future regression that reintroduces an unqualified branch
339+
// keyed on len(state.Services) == 1.
340+
func TestFilterDeployedServices_ChainedIntoResolveAfterDeploy(t *testing.T) {
341+
t.Parallel()
342+
343+
state := &nextstep.State{
344+
Services: []nextstep.ServiceState{
345+
{Name: "alpha", IsDeployed: true, Protocol: nextstep.ProtocolResponses},
346+
{Name: "beta", IsDeployed: false, Protocol: nextstep.ProtocolResponses},
347+
},
348+
}
349+
350+
out := nextstep.ResolveAfterDeploy(filterDeployedServices(state), nil, nil)
351+
352+
require.Len(t, out, 2, "filtered state has one deployed service → show + invoke")
353+
assert.Equal(t, "azd ai agent show alpha", out[0].Command,
354+
"command must be service-qualified even when filtered list has len==1")
355+
assert.Equal(t, `azd ai agent invoke alpha "Hello!"`, out[1].Command,
356+
"invoke command must also be service-qualified")
357+
}

cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver.go

Lines changed: 59 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -415,37 +415,59 @@ func ResolveAfterShow(state *State, serviceName string) []Suggestion {
415415
}
416416

417417
// AfterDeployOpts configures ResolveAfterDeploy. Optional — the
418-
// zero-value matches the historical post-deploy call site behavior.
418+
// zero-value matches the post-deploy call site behavior.
419419
type AfterDeployOpts struct {
420-
// ForceQualified, when true, makes ResolveAfterDeploy emit
421-
// service-qualified `azd ai agent show <name>` / `invoke <name> ...`
422-
// commands even when len(state.Services) == 1.
420+
// ForceQualified is retained for backward compatibility but is
421+
// effectively a no-op as of issue #7975 fix B9: ResolveAfterDeploy
422+
// now always emits service-qualified
423+
// `azd ai agent show <name>` / `invoke <name> ...` commands
424+
// regardless of how many services are in state.
423425
//
424-
// Use this when the input State has been filtered down from a
425-
// larger multi-agent project (e.g., doctor showing only deployed
426-
// services). The default `len(state.Services) == 1` heuristic
427-
// would otherwise emit no-arg commands that ambiguity-prompt or
428-
// error at runtime because resolveAgentService sees ALL azure.yaml
429-
// services, not just the filtered subset.
426+
// Pre-B9 callers passed ForceQualified=true to override a
427+
// "single-agent → unqualified command" heuristic that no longer
428+
// exists. The flag is preserved so existing callers compile and
429+
// run identically; new callers may simply omit it.
430430
ForceQualified bool
431431
}
432432

433433
// ResolveAfterDeploy produces the Next: block embedded in the post-deploy
434-
// artifact note. The block is rendered per agent service: one
435-
// `azd ai agent show <name>` plus one `azd ai agent invoke '<payload>'`
436-
// line, where the payload is taken from the cached OpenAPI spec when the
437-
// `cachedPayload` lookup yields a non-empty string for the agent.
434+
// artifact note. Issue #7975 fix B9 spec (lines 228-242):
435+
//
436+
// - Single-agent project: emit one `azd ai agent show <name>` line
437+
// followed by one `azd ai agent invoke <name> '<payload>'` line.
438+
// Descriptions are "verify it's running" / "test the deployment".
439+
// - Multi-agent project: emit all `show <name>` lines first (one
440+
// per service, in declaration order), then all `invoke <name>`
441+
// lines. Descriptions include the agent name —
442+
// "verify <name> is running" / "test <name>" — so the user can
443+
// identify which row maps to which agent at a glance.
444+
//
445+
// In both cases commands are always service-qualified (B9). Pre-B9
446+
// behavior would strip the name when len(state.Services) == 1, which
447+
// produced ambiguous `azd ai agent show` lines in artifact notes that
448+
// users couldn't run directly when copy-pasted into a multi-agent
449+
// project later. The qualified form is unambiguous and copy-paste
450+
// safe in either project shape.
438451
//
439452
// cachedPayload is injected by the caller (typically a closure over
440453
// ReadCachedOpenAPISpec + ExtractInvokeExample) so the resolver itself
441-
// stays pure and unit-testable.
454+
// stays pure and unit-testable. The cached sample is used verbatim
455+
// (POSIX-escaped) when present; otherwise the protocol-appropriate
456+
// fallback from defaultInvokePayload is used.
442457
//
443-
// readmeExists, also injected, controls whether the "See <relPath>/README.md
444-
// for a sample payload" line is appended. The resolver does not touch the
445-
// filesystem directly.
458+
// readmeExists, also injected, controls whether the
459+
// "See <relPath>/README.md for a sample payload" line is appended
460+
// for a given service. The hint is emitted only when:
461+
// (1) no cached payload was available for that service,
462+
// (2) the service has a RelativePath, and
463+
// (3) readmeExists reports a README on disk at that path.
464+
// In the multi-agent layout each service's README hint is rendered
465+
// immediately after that service's invoke line so users can scan
466+
// rows top-to-bottom and find each agent's hint in context.
446467
//
447-
// opts is variadic for backward compatibility. Only the first element is
448-
// consulted; additional elements are ignored.
468+
// opts is variadic for backward compatibility but is no longer
469+
// consulted — every field of AfterDeployOpts is now a no-op post-B9.
470+
// See AfterDeployOpts.ForceQualified for the historical context.
449471
func ResolveAfterDeploy(
450472
state *State,
451473
cachedPayload func(serviceName string) string,
@@ -456,27 +478,28 @@ func ResolveAfterDeploy(
456478
return nil
457479
}
458480

459-
var forceQualified bool
460-
if len(opts) > 0 {
461-
forceQualified = opts[0].ForceQualified
462-
}
463-
481+
singleAgent := len(state.Services) == 1
464482
out := make([]Suggestion, 0, len(state.Services)*3)
465-
singleAgent := !forceQualified && len(state.Services) == 1
466483
priority := 10
467484

485+
// Pass 1: all `azd ai agent show <name>` lines, in service order.
468486
for _, svc := range state.Services {
469-
showCmd := "azd ai agent show"
470-
if !singleAgent {
471-
showCmd = fmt.Sprintf("azd ai agent show %s", svc.Name)
487+
desc := fmt.Sprintf("verify %s is running", svc.Name)
488+
if singleAgent {
489+
desc = "verify it's running"
472490
}
473491
out = append(out, Suggestion{
474-
Command: showCmd,
475-
Description: "verify the deployed agent is running",
492+
Command: fmt.Sprintf("azd ai agent show %s", svc.Name),
493+
Description: desc,
476494
Priority: priority,
477495
})
478496
priority++
497+
}
479498

499+
// Pass 2: all `azd ai agent invoke <name> <payload>` lines, each
500+
// followed by its README hint when applicable. Grouping invokes
501+
// after shows matches the spec example output (lines 238-241).
502+
for _, svc := range state.Services {
480503
payload := ""
481504
if cachedPayload != nil {
482505
payload = cachedPayload(svc.Name)
@@ -486,13 +509,13 @@ func ResolveAfterDeploy(
486509
invokeArg = shellEscapeSingleQuoted(payload)
487510
}
488511

489-
invokeCmd := fmt.Sprintf("azd ai agent invoke %s", invokeArg)
490-
if !singleAgent {
491-
invokeCmd = fmt.Sprintf("azd ai agent invoke %s %s", svc.Name, invokeArg)
512+
desc := fmt.Sprintf("test %s", svc.Name)
513+
if singleAgent {
514+
desc = "test the deployment"
492515
}
493516
out = append(out, Suggestion{
494-
Command: invokeCmd,
495-
Description: "send a sample request to the deployed agent",
517+
Command: fmt.Sprintf("azd ai agent invoke %s %s", svc.Name, invokeArg),
518+
Description: desc,
496519
Priority: priority,
497520
})
498521
priority++

cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver_test.go

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -716,28 +716,35 @@ func TestResolveAfterShow_NilState(t *testing.T) {
716716
func TestResolveAfterDeploy(t *testing.T) {
717717
t.Parallel()
718718

719-
t.Run("single agent, cached payload available → 2 lines, no README hint", func(t *testing.T) {
719+
t.Run("single agent, cached payload available → 2 qualified lines, no README hint", func(t *testing.T) {
720720
t.Parallel()
721721
state := &State{Services: []ServiceState{{Name: "echo", RelativePath: "./src/echo"}}}
722722
cached := func(_ string) string { return `{"q":"x"}` }
723723
out := ResolveAfterDeploy(state, cached, nil)
724724
require.Len(t, out, 2)
725-
assert.Equal(t, "azd ai agent show", out[0].Command)
726-
assert.Equal(t, `azd ai agent invoke '{"q":"x"}'`, out[1].Command)
725+
assert.Equal(t, "azd ai agent show echo", out[0].Command)
726+
assert.Equal(t, "verify it's running", out[0].Description)
727+
assert.Equal(t, `azd ai agent invoke echo '{"q":"x"}'`, out[1].Command)
728+
assert.Equal(t, "test the deployment", out[1].Description)
727729
})
728730

729-
t.Run("single agent, no cached payload, README on disk → 3 lines with README pointer", func(t *testing.T) {
731+
t.Run("single agent, no cached payload, README on disk → 3 lines with qualified commands", func(t *testing.T) {
730732
t.Parallel()
731733
state := &State{Services: []ServiceState{{Name: "echo", RelativePath: "./src/echo", Protocol: ProtocolResponses}}}
732734
readme := func(p string) bool { return p == "./src/echo" }
733735
out := ResolveAfterDeploy(state, nil, readme)
734736
require.Len(t, out, 3)
735-
assert.Equal(t, "azd ai agent show", out[0].Command)
736-
assert.Equal(t, `azd ai agent invoke "Hello!"`, out[1].Command)
737+
assert.Equal(t, "azd ai agent show echo", out[0].Command)
738+
assert.Equal(t, "verify it's running", out[0].Description)
739+
assert.Equal(t, `azd ai agent invoke echo "Hello!"`, out[1].Command)
740+
assert.Equal(t, "test the deployment", out[1].Description)
737741
assert.Contains(t, out[2].Command, "src/echo/README.md")
738742
})
739743

740-
t.Run("multi-agent → one show/invoke pair per agent, named", func(t *testing.T) {
744+
t.Run("multi-agent → all shows first, then all invokes, with per-agent descriptions", func(t *testing.T) {
745+
// Spec source: issue #7975 lines 238-241 — multi-agent layout
746+
// groups shows before invokes (not interleaved) and bakes the
747+
// agent name into the description so users can scan vertically.
741748
t.Parallel()
742749
state := &State{Services: []ServiceState{
743750
{Name: "alpha", Protocol: ProtocolInvocations},
@@ -746,9 +753,30 @@ func TestResolveAfterDeploy(t *testing.T) {
746753
out := ResolveAfterDeploy(state, nil, nil)
747754
require.Len(t, out, 4)
748755
assert.Equal(t, "azd ai agent show alpha", out[0].Command)
749-
assert.Equal(t, `azd ai agent invoke alpha '{"message": "Hello!"}'`, out[1].Command)
750-
assert.Equal(t, "azd ai agent show beta", out[2].Command)
756+
assert.Equal(t, "verify alpha is running", out[0].Description)
757+
assert.Equal(t, "azd ai agent show beta", out[1].Command)
758+
assert.Equal(t, "verify beta is running", out[1].Description)
759+
assert.Equal(t, `azd ai agent invoke alpha '{"message": "Hello!"}'`, out[2].Command)
760+
assert.Equal(t, "test alpha", out[2].Description)
751761
assert.Equal(t, `azd ai agent invoke beta "Hello!"`, out[3].Command)
762+
assert.Equal(t, "test beta", out[3].Description)
763+
})
764+
765+
t.Run("multi-agent README hint placement → after the corresponding invoke line", func(t *testing.T) {
766+
t.Parallel()
767+
state := &State{Services: []ServiceState{
768+
{Name: "alpha", RelativePath: "./src/alpha", Protocol: ProtocolResponses},
769+
{Name: "beta", Protocol: ProtocolResponses},
770+
}}
771+
readme := func(p string) bool { return p == "./src/alpha" }
772+
out := ResolveAfterDeploy(state, nil, readme)
773+
// 2 shows + 2 invokes + 1 README hint for alpha = 5 entries.
774+
require.Len(t, out, 5)
775+
assert.Equal(t, "azd ai agent show alpha", out[0].Command)
776+
assert.Equal(t, "azd ai agent show beta", out[1].Command)
777+
assert.Equal(t, `azd ai agent invoke alpha "Hello!"`, out[2].Command)
778+
assert.Contains(t, out[3].Command, "src/alpha/README.md")
779+
assert.Equal(t, `azd ai agent invoke beta "Hello!"`, out[4].Command)
752780
})
753781

754782
t.Run("README hint skipped when cached payload is present", func(t *testing.T) {
@@ -770,35 +798,41 @@ func TestResolveAfterDeploy(t *testing.T) {
770798
assert.Nil(t, ResolveAfterDeploy(nil, nil, nil))
771799
})
772800

773-
t.Run("cached payload containing apostrophe → POSIX-escaped", func(t *testing.T) {
801+
t.Run("cached payload containing apostrophe → POSIX-escaped on qualified invoke", func(t *testing.T) {
774802
t.Parallel()
775803
state := &State{Services: []ServiceState{{Name: "echo", RelativePath: "./src/echo"}}}
776804
cached := func(_ string) string { return `{"q":"don't"}` }
777805
out := ResolveAfterDeploy(state, cached, nil)
778806
require.Len(t, out, 2)
779-
assert.Equal(t, `azd ai agent invoke '{"q":"don'\''t"}'`, out[1].Command)
807+
assert.Equal(t, `azd ai agent invoke echo '{"q":"don'\''t"}'`, out[1].Command)
780808
})
781809

782-
t.Run("ForceQualified=true on len==1 → service-qualified commands", func(t *testing.T) {
810+
t.Run("ForceQualified=true on len==1 → no-op, output identical to default", func(t *testing.T) {
811+
// Backward-compat assertion: B9 makes all output qualified by
812+
// default; ForceQualified is preserved as a no-op for callers
813+
// (e.g., doctor) that still pass it. Result must match the
814+
// "no opts" call exactly.
783815
t.Parallel()
784816
state := &State{Services: []ServiceState{
785817
{Name: "echo", RelativePath: "./src/echo", Protocol: ProtocolInvocations},
786818
}}
787819
out := ResolveAfterDeploy(state, nil, nil, AfterDeployOpts{ForceQualified: true})
820+
baseline := ResolveAfterDeploy(state, nil, nil)
821+
require.Equal(t, baseline, out)
788822
require.Len(t, out, 2)
789823
assert.Equal(t, "azd ai agent show echo", out[0].Command)
790824
assert.Equal(t, `azd ai agent invoke echo '{"message": "Hello!"}'`, out[1].Command)
791825
})
792826

793-
t.Run("ForceQualified=false on len==1 → unqualified (matches default)", func(t *testing.T) {
827+
t.Run("ForceQualified=false on len==1 → no-op, also qualified", func(t *testing.T) {
794828
t.Parallel()
795829
state := &State{Services: []ServiceState{
796830
{Name: "echo", RelativePath: "./src/echo", Protocol: ProtocolInvocations},
797831
}}
798832
out := ResolveAfterDeploy(state, nil, nil, AfterDeployOpts{ForceQualified: false})
799833
require.Len(t, out, 2)
800-
assert.Equal(t, "azd ai agent show", out[0].Command)
801-
assert.Equal(t, `azd ai agent invoke '{"message": "Hello!"}'`, out[1].Command)
834+
assert.Equal(t, "azd ai agent show echo", out[0].Command)
835+
assert.Equal(t, `azd ai agent invoke echo '{"message": "Hello!"}'`, out[1].Command)
802836
})
803837

804838
t.Run("ForceQualified=true with cached payload → qualified invoke uses payload", func(t *testing.T) {
@@ -811,7 +845,7 @@ func TestResolveAfterDeploy(t *testing.T) {
811845
assert.Equal(t, `azd ai agent invoke echo '{"q":"x"}'`, out[1].Command)
812846
})
813847

814-
t.Run("ForceQualified=true on multi-agent → qualified (already-qualified case unaffected)", func(t *testing.T) {
848+
t.Run("ForceQualified=true on multi-agent → identical to default multi-agent layout", func(t *testing.T) {
815849
t.Parallel()
816850
state := &State{Services: []ServiceState{
817851
{Name: "alpha", Protocol: ProtocolInvocations},
@@ -820,8 +854,8 @@ func TestResolveAfterDeploy(t *testing.T) {
820854
out := ResolveAfterDeploy(state, nil, nil, AfterDeployOpts{ForceQualified: true})
821855
require.Len(t, out, 4)
822856
assert.Equal(t, "azd ai agent show alpha", out[0].Command)
823-
assert.Equal(t, `azd ai agent invoke alpha '{"message": "Hello!"}'`, out[1].Command)
824-
assert.Equal(t, "azd ai agent show beta", out[2].Command)
857+
assert.Equal(t, "azd ai agent show beta", out[1].Command)
858+
assert.Equal(t, `azd ai agent invoke alpha '{"message": "Hello!"}'`, out[2].Command)
825859
assert.Equal(t, `azd ai agent invoke beta "Hello!"`, out[3].Command)
826860
})
827861

0 commit comments

Comments
 (0)