From 6a568b807aced4754f965f92e5a4c23bc078ca80 Mon Sep 17 00:00:00 2001 From: trangevi Date: Tue, 14 Apr 2026 14:48:27 -0700 Subject: [PATCH 1/2] Better handling for protocol flag Signed-off-by: trangevi --- .../azure.ai.agents/internal/cmd/helpers.go | 64 ++++++++++++++++--- .../azure.ai.agents/internal/cmd/invoke.go | 39 +++++------ .../internal/cmd/invoke_test.go | 5 +- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index 0242734ccee..c1181ccefa3 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -675,28 +675,76 @@ func resolveStartupCommandForInit( // resolveAgentProtocol loads the agent.yaml manifest for the service and returns the // protocol that the agent implements (e.g. "responses", "invocations"). -// Defaults to "responses" when the manifest cannot be loaded or has no protocols. +// Returns an error when the protocol cannot be determined, with a contextual +// suggestion guiding the user to fix the underlying issue. func resolveAgentProtocol( ctx context.Context, azdClient *azdext.AzdClient, name string, noPrompt bool, -) agent_api.AgentProtocol { +) (agent_api.AgentProtocol, error) { svc, project, err := resolveAgentService(ctx, azdClient, name, noPrompt) if err != nil { - return agent_api.AgentProtocolResponses + return "", exterrors.Validation( + exterrors.CodeInvalidParameter, + fmt.Sprintf( + "could not resolve agent service in azd project: %s", err, + ), + "run from your project directory and ensure "+ + "azure.yaml contains an azure.ai.agent service", + ) } - agentYamlPath := filepath.Join(project.Path, svc.RelativePath, "agent.yaml") + agentYamlPath := filepath.Join( + project.Path, svc.RelativePath, "agent.yaml", + ) data, err := os.ReadFile(agentYamlPath) //nolint:gosec // G304: path constructed from azd project root if err != nil { - return agent_api.AgentProtocolResponses + return "", exterrors.Validation( + exterrors.CodeInvalidParameter, + fmt.Sprintf( + "could not read agent.yaml at %s: %s", + agentYamlPath, err, + ), + "ensure agent.yaml exists in the azd service directory", + ) } var hosted agent_yaml.ContainerAgent - if err := yaml.Unmarshal(data, &hosted); err == nil && len(hosted.Protocols) > 0 { - return agent_api.AgentProtocol(hosted.Protocols[0].Protocol) + if err := yaml.Unmarshal(data, &hosted); err != nil { + return "", exterrors.Validation( + exterrors.CodeInvalidParameter, + fmt.Sprintf( + "could not parse agent.yaml at %s: %s", + agentYamlPath, err, + ), + "fix the agent.yaml syntax", + ) } - return agent_api.AgentProtocolResponses + switch len(hosted.Protocols) { + case 0: + return "", exterrors.Validation( + exterrors.CodeInvalidParameter, + "agent.yaml does not declare any protocols", + "add a protocols section to agent.yaml", + ) + case 1: + return agent_api.AgentProtocol( + hosted.Protocols[0].Protocol, + ), nil + default: + names := make([]string, len(hosted.Protocols)) + for i, p := range hosted.Protocols { + names[i] = p.Protocol + } + return "", exterrors.Validation( + exterrors.CodeInvalidParameter, + fmt.Sprintf( + "agent.yaml declares multiple protocols: %s", + strings.Join(names, ", "), + ), + "use --protocol to specify which protocol to use", + ) + } } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go index 9b4078fdd99..825d90c7b0e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go @@ -162,7 +162,10 @@ session automatically. Pass --new-session to force a reset.`, } func (a *InvokeAction) Run(ctx context.Context) error { - protocol := a.resolveProtocol(ctx) + protocol, err := a.resolveProtocol(ctx) + if err != nil { + return err + } if a.flags.local { switch protocol { @@ -190,37 +193,27 @@ func (a *InvokeAction) Run(ctx context.Context) error { // resolveProtocol returns the protocol to use for this invocation. // The explicit --protocol flag takes priority; otherwise the protocol // is auto-detected from agent.yaml (local or remote). -func (a *InvokeAction) resolveProtocol(ctx context.Context) agent_api.AgentProtocol { +func (a *InvokeAction) resolveProtocol( + ctx context.Context, +) (agent_api.AgentProtocol, error) { if a.flags.protocol != "" { - return agent_api.AgentProtocol(a.flags.protocol) + return agent_api.AgentProtocol(a.flags.protocol), nil } - if a.flags.local { - return a.resolveLocalProtocol(ctx) - } - return a.resolveRemoteProtocol(ctx) -} - -// resolveRemoteProtocol determines the protocol for remote invocation from agent.yaml. -func (a *InvokeAction) resolveRemoteProtocol(ctx context.Context) agent_api.AgentProtocol { azdClient, err := azdext.NewAzdClient() if err != nil { - return agent_api.AgentProtocolResponses + return "", fmt.Errorf("failed to create azd client: %w", err) } defer azdClient.Close() - return resolveAgentProtocol(ctx, azdClient, a.flags.name, rootFlags.NoPrompt) -} - -// resolveLocalProtocol determines the protocol for local invocation from agent.yaml. -func (a *InvokeAction) resolveLocalProtocol(ctx context.Context) agent_api.AgentProtocol { - azdClient, err := azdext.NewAzdClient() - if err != nil { - return agent_api.AgentProtocolResponses + if a.flags.local { + return resolveAgentProtocol( + ctx, azdClient, "", rootFlags.NoPrompt, + ) } - defer azdClient.Close() - - return resolveAgentProtocol(ctx, azdClient, "", rootFlags.NoPrompt) + return resolveAgentProtocol( + ctx, azdClient, a.flags.name, rootFlags.NoPrompt, + ) } func (a *InvokeAction) httpTimeout() time.Duration { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_test.go index 7947194bbeb..5b3d78851f6 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_test.go @@ -281,7 +281,10 @@ func TestResolveProtocol_ExplicitFlag(t *testing.T) { } // resolveProtocol with an explicit flag should return it directly // without trying to read agent.yaml (which would fail in tests). - got := action.resolveProtocol(t.Context()) + got, err := action.resolveProtocol(t.Context()) + if err != nil { + t.Fatalf("resolveProtocol() unexpected error: %v", err) + } if got != tt.want { t.Errorf("resolveProtocol() = %q, want %q", got, tt.want) } From bcbbc721ff03f981ce245a878f29c1448b33d02e Mon Sep 17 00:00:00 2001 From: trangevi Date: Tue, 14 Apr 2026 15:09:39 -0700 Subject: [PATCH 2/2] Pr comments Signed-off-by: trangevi --- .../azure.ai.agents/internal/cmd/helpers.go | 22 +++- .../internal/cmd/helpers_test.go | 107 ++++++++++++++++++ 2 files changed, 126 insertions(+), 3 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index c1181ccefa3..58a04ec76ae 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -698,6 +698,15 @@ func resolveAgentProtocol( agentYamlPath := filepath.Join( project.Path, svc.RelativePath, "agent.yaml", ) + return protocolFromAgentYaml(agentYamlPath) +} + +// protocolFromAgentYaml reads and parses the agent.yaml file at the given path +// and extracts the protocol. Returns an error with a contextual suggestion when +// the file cannot be read, parsed, or does not declare exactly one protocol. +func protocolFromAgentYaml( + agentYamlPath string, +) (agent_api.AgentProtocol, error) { data, err := os.ReadFile(agentYamlPath) //nolint:gosec // G304: path constructed from azd project root if err != nil { return "", exterrors.Validation( @@ -730,9 +739,16 @@ func resolveAgentProtocol( "add a protocols section to agent.yaml", ) case 1: - return agent_api.AgentProtocol( - hosted.Protocols[0].Protocol, - ), nil + p := strings.TrimSpace(hosted.Protocols[0].Protocol) + if p == "" { + return "", exterrors.Validation( + exterrors.CodeInvalidParameter, + "agent.yaml declares a protocol entry, "+ + "but its protocol field is empty", + "set a non-empty protocol value in agent.yaml", + ) + } + return agent_api.AgentProtocol(p), nil default: names := make([]string, len(hosted.Protocols)) for i, p := range hosted.Protocols { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go index 8e8fddb1b08..bf2ac3451ed 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go @@ -6,6 +6,7 @@ package cmd import ( "os" "path/filepath" + "strings" "testing" ) @@ -178,3 +179,109 @@ func TestToServiceKey(t *testing.T) { }) } } + +func TestProtocolFromAgentYaml(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + yaml string // file contents; empty string means no file + noFile bool // when true, don't create agent.yaml + wantProto string + wantErr bool + errContain string // substring expected in the error message + }{ + { + name: "single protocol responses", + yaml: "protocols:\n - protocol: responses\n version: \"1.0\"\n", + wantProto: "responses", + }, + { + name: "single protocol invocations", + yaml: "protocols:\n - protocol: invocations\n version: \"1.0\"\n", + wantProto: "invocations", + }, + { + name: "no file", + noFile: true, + wantErr: true, + errContain: "could not read agent.yaml", + }, + { + name: "invalid yaml", + yaml: "protocols: [[[invalid", + wantErr: true, + errContain: "could not parse agent.yaml", + }, + { + name: "no protocols field", + yaml: "name: my-agent\n", + wantErr: true, + errContain: "does not declare any protocols", + }, + { + name: "empty protocols list", + yaml: "protocols: []\n", + wantErr: true, + errContain: "does not declare any protocols", + }, + { + name: "single protocol with empty value", + yaml: "protocols:\n - protocol: \"\"\n version: \"1.0\"\n", + wantErr: true, + errContain: "protocol field is empty", + }, + { + name: "single protocol whitespace only", + yaml: "protocols:\n - protocol: \" \"\n version: \"1.0\"\n", + wantErr: true, + errContain: "protocol field is empty", + }, + { + name: "multiple protocols", + yaml: "protocols:\n - protocol: responses\n" + + " version: \"1.0\"\n - protocol: invocations\n" + + " version: \"1.0\"\n", + wantErr: true, + errContain: "declares multiple protocols", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + yamlPath := filepath.Join(dir, "agent.yaml") + + if !tt.noFile { + if err := os.WriteFile( + yamlPath, []byte(tt.yaml), 0600, + ); err != nil { + t.Fatalf("failed to write agent.yaml: %v", err) + } + } + + got, err := protocolFromAgentYaml(yamlPath) + + if tt.wantErr { + if err == nil { + t.Fatalf("expected error containing %q, got nil", + tt.errContain) + } + if !strings.Contains(err.Error(), tt.errContain) { + t.Errorf("error = %q, want substring %q", + err.Error(), tt.errContain) + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(got) != tt.wantProto { + t.Errorf("protocol = %q, want %q", got, tt.wantProto) + } + }) + } +}