feat: add --protocol flag to azd ai agent invoke#7679
Conversation
Add 86 new unit tests across 5 previously untested or undertested packages in the azure.ai.agents extension, raising total test count from 183 to 269. Coverage improvements: - agent_yaml: 23.1% -> 53.8% (map.go YAML-to-API mapping fully tested) - registry_api: 0% -> 28.8% (tool conversion, parameter conversion, merge) - agent_api: 0% -> tested (JSON round-trip for all model types) - cmd: 23.0% -> 23.6% (copyDirectory, copyFile, buildAgentEndpoint) New test files: - agent_yaml/map_test.go: 44 tests for YAML-to-API transform functions - registry_api/helpers_test.go: 35 tests for pure conversion helpers - agent_api/models_test.go: 24 JSON serialization round-trip tests - cmd/init_copy_test.go: directory/file copy logic tests - cmd/agent_context_test.go: endpoint construction test - agent_yaml/testdata_test.go: fixture-based parsing + regression tests New testdata fixtures (7 YAML files): - 3 valid agents (minimal prompt, full prompt, hosted) - 1 MCP tools agent - 3 invalid manifests (no kind, no model, empty template) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the generic ptr[T](v T) *T helper function with Go 1.26's built-in new(val) pattern in models_test.go and helpers_test.go, consistent with map_test.go and AGENTS.md conventions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a --protocol/-p flag to 'azd ai agent invoke' so developers can explicitly choose which protocol to use (responses or invocations) when their agent.yaml declares multiple protocols. Without the flag, the existing auto-detection from agent.yaml is preserved (first listed protocol, defaulting to responses). The flag is validated against the supported protocol values and works for both local (--local) and remote invocation paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use 0600 for WriteFile and 0750 for MkdirAll (gosec G306/G301) - Add nolint:gosec for test helper ReadFile (G304) - Remove trailing blank line (gofmt) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an explicit --protocol / -p flag to azd ai agent invoke to let callers choose between responses and invocations when invoking an agent, rather than relying solely on auto-detection. The PR also introduces a substantial set of new unit tests and YAML fixtures across the agents extension (beyond the invoke command changes described in the PR text).
Changes:
- Add
--protocol/-pflag, validation, and protocol resolution precedence logic toazd ai agent invoke. - Add tests covering explicit protocol resolution and protocol flag validation.
- Add extensive new tests/fixtures for agent API/YAML mapping and registry manifest conversion.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go | Adds the --protocol flag, validates values, and resolves protocol selection before invoking local/remote flows. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_test.go | Adds unit tests for explicit protocol resolution and flag validation behavior. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/agent_context_test.go | Adds unit tests for Foundry endpoint construction helper. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/init_copy_test.go | Adds unit tests for directory/file copy helpers used by init flows. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/registry_api/helpers_test.go | Adds tests for converting registry API tool/agent definitions into agent YAML types. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/map_test.go | Adds extensive unit tests for mapping between agent YAML structs and agent API models. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/models_test.go | Adds round-trip JSON marshal/unmarshal tests for many agent API models. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata_test.go | Adds tests asserting behavior of YAML fixtures (parsing/validation expectations). |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/hosted-agent.yaml | Adds a hosted agent fixture including both responses and invocations protocol records. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/prompt-agent-minimal.yaml | Adds a minimal prompt-agent YAML fixture. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/prompt-agent-full.yaml | Adds a “full” prompt-agent YAML fixture for testing. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/mcp-tools-agent.yaml | Adds a prompt-agent YAML fixture including MCP and code interpreter tools. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/invalid-no-model.yaml | Adds an invalid fixture (missing model) used by validation tests. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/invalid-no-kind.yaml | Adds an invalid fixture (missing kind) used by validation tests. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/testdata/invalid-empty-template.yaml | Adds an invalid fixture (empty template) used by validation tests. |
…ures Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/78e2f379-b686-492f-ac4f-cd0b0045680d Co-authored-by: glharper <64209257+glharper@users.noreply.github.com>
…lpers_test.go Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/78e2f379-b686-492f-ac4f-cd0b0045680d Co-authored-by: glharper <64209257+glharper@users.noreply.github.com>
There was a problem hiding this comment.
The protocol flag feature is clean and well-tested. A few things to address:
-
Silent fallback on explicit flag (HIGH) - when the user explicitly passes --protocol invocations for remote invoke and vnext isn't enabled, Run() silently uses
esponsesRemote() instead. An explicit flag choice should be honored or produce a clear error - not silently ignored. See inline comment. -
PR scope - ~90% of this diff is new test coverage and YAML fixtures for existing code (models, map, helpers, testdata, init_copy, agent_context) that's unrelated to the protocol feature. Consider splitting: (1) protocol flag, (2) helpers.go pointer fix, (3) new test coverage. Smaller PRs are easier to review and bisect.
-
prompt-agent-full.yaml uses publisher and maxTokens which don't match the Go schema (provider and maxOutputTokens). These parse but won't populate the structs. See inline comment.
Positive notes:
- helpers.go pointer fix is correct and important - each property now gets its own pointer
esolveProtocol() cleanly unifies local/remote with explicit flag priority
- Validation catches bad protocol values early with helpful suggestions
- Test structure is solid: parallel, table-driven, good edge cases
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…x testdata field names - Return a clear validation error when --protocol invocations is explicitly requested for remote invoke but vnext is not enabled, instead of silently falling back to responses. - Fix prompt-agent-full.yaml: publisher -> provider, maxTokens -> maxOutputTokens to match the Model/ModelOptions Go struct fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
jongio
left a comment
There was a problem hiding this comment.
Both issues from my earlier review are addressed - the silent fallback is now a clear validation error, and the YAML fixture fields match the Go schema.
Two minor observations (not blocking):
-
The refactored
Run()also changes auto-detect behavior: ifagent.yamlresolves toinvocationsremotely and vnext is off, users now get an error instead of a silent fallback toresponses. This is actually better (fail-fast vs silent degradation), but worth noting in case anyone's relying on the old behavior. -
Consider adding a test for the vnext gating path (explicit
--protocol invocations+ remote + vnext disabled should error). The current tests validate flag parsing but don't cover this decision branch.
Clean feature, good tests, nice refactor of resolveProtocol().
fixes #7492
Summary
Adds a
--protocol/-pflag toazd ai agent invokeso developers can explicitly select which protocol to use when theiragent.yamldeclares multiple protocols.Usage
Changes
invoke.go: Addedprotocolfield toinvokeFlags, registered--protocol/-pflag, added validation against supported values (responses,invocations), and refactoredRun()to use a newresolveProtocol()method that checks the explicit flag first before falling back to agent.yaml auto-detection.invoke_test.go: AddedTestResolveProtocol_ExplicitFlag(verifies explicit flag bypasses agent.yaml resolution) andTestProtocolFlagValidation(verifies valid values pass and invalid values produce clear errors).Behavior
--protocolflagagent.yaml(first listed protocol, defaultresponses) — unchangedresponsesinvocations