Fix panic in agent.yaml parsing and enable CI tests for azure.ai.agents#6683
Fix panic in agent.yaml parsing and enable CI tests for azure.ai.agents#6683rajeshkamal5050 merged 12 commits intomainfrom
Conversation
- Update ExtractAgentDefinition to handle both manifest and standalone formats - Add nil check for template field before type assertion - Support flat YAML structure with agent definition at root level - Add comprehensive tests for both YAML formats and edge cases Co-authored-by: vhvb1989 <[email protected]>
- Add support for both 'template' and 'agent' fields in manifest format - Add test for 'agent' field format used in sample files - Now supports three formats: template field, agent field, and standalone Co-authored-by: vhvb1989 <[email protected]>
- Add error handling for yaml.Marshal calls - Replace custom contains function with strings.Contains from stdlib - Remove redundant helper functions Co-authored-by: vhvb1989 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR fixes a panic in azd ai agent init that occurred when parsing agent.yaml files without a template field. The panic was caused by attempting to convert a nil interface{} to a map without proper nil checking.
Changes:
- Updated
ExtractAgentDefinitionto handle three YAML formats: manifest withtemplatefield, manifest withagentfield (alias), and standalone agent definition - Added comprehensive test coverage for different YAML parsing scenarios
- Implemented proper nil checks and type assertions with error messages
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse.go | Added logic to handle multiple YAML formats with proper nil checks and type validation to prevent panic |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_test.go | Added comprehensive test suite covering all three YAML formats and edge cases |
Comments suppressed due to low confidence (1)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_test.go:267
- The custom
containsandstringContainshelper functions are unnecessary. The standard library'sstrings.Containsfunction should be used instead, which is already imported and used extensively throughout the codebase (e.g., in init.go, helpers.go, and parser.go). Replace calls to the customcontainsfunction withstrings.Containsdirectly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_test.go
Show resolved
Hide resolved
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse.go
Outdated
Show resolved
Hide resolved
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_test.go
Show resolved
Hide resolved
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_test.go
Show resolved
Hide resolved
Update protobuf from 5.29.5 to 6.33.5 in microsoft.azd.extensions - Addresses JSON recursion depth bypass vulnerability - Affected versions: <= 6.33.4 - Patched version: 6.33.5 Co-authored-by: vhvb1989 <[email protected]>
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse.go
Outdated
Show resolved
Hide resolved
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse.go
Show resolved
Hide resolved
|
@copilot looks like you weren't able to apply this feedback - #6683 (comment) can you try again? |
- Return error when neither 'template' nor 'agent' field exists - Add check for empty template/agent maps - Update tests to expect errors for YAMLs without wrapper fields - Add test for precedence when both fields present - Add test for empty/null agent field handling Per feedback: the rest of the code assumes manifest format, so supporting standalone format would lead to undefined behavior. Now returns a clear error message with link to AgentSchema documentation. Co-authored-by: rajeshkamal5050 <[email protected]>
Applied the feedback in commit 859d1e8. The code now returns a proper error message when neither |
...azd/extensions/microsoft.azd.extensions/internal/resources/languages/python/requirements.txt
Outdated
Show resolved
Hide resolved
Per review feedback, the 'agent' field is not valid in AgentManifest format. - Remove agent field handling from ExtractAgentDefinition - Update error messages to only mention 'template' field - Remove tests for agent field, BothTemplateAndAgent precedence - Update sample files to use 'template' instead of 'agent' - Rename EmptyAgentField test to EmptyTemplateField The AgentManifest struct defines yaml:"template" as the only valid field. Valid manifest examples: https://github.com/microsoft-foundry/foundry-samples Co-authored-by: rajeshkamal5050 <[email protected]>
|
@vhvb1989 @JeffreyCA can we do error with suggestion from extensions? So we could also suggest users to look at the foundry-samples repo link for valid agent yamls |
Yes would be great if we could reuse the enhanced ErrorWithSuggestion changes in #6700. Currently it's in |
Thanks @JeffreyCA got it. Lets may be fix the panic right now with error message and pointers to agentschema and the foundry-samples repo links. We can probably enhance using suggestions on a later PR. |
|
@copilot can we clean up these changes and emit an appropriate error message pointing to https://microsoft.github.io/AgentSchema/reference/agentmanifest and https://github.com/microsoft-foundry/foundry-samples for valid sample AgentManifests? References: |
Per review feedback, update error messages to point to: - AgentManifest reference: https://microsoft.github.io/AgentSchema/reference/agentmanifest - Sample manifests: https://github.com/microsoft-foundry/foundry-samples This provides users with both the format specification and working examples when they encounter YAML format errors. Co-authored-by: JeffreyCA <[email protected]>
Updated error messages in commit dc2875d to include links to both the AgentManifest reference documentation (https://microsoft.github.io/AgentSchema/reference/agentmanifest) and the foundry-samples repository for working examples. |
Add ci-test.ps1 script and remove SkipTests flag to enable unit tests in the CI pipeline for azure.ai.agents extension. Changes: - Create ci-test.ps1 script matching other extensions (uses gotestsum or go test) - Remove SkipTests: true from release-ext-azure-ai-agents.yml pipeline This aligns with PR #6589 to enable unit test validation in the CI pipeline. Co-authored-by: JeffreyCA <[email protected]>
…#6695 Co-authored-by: JeffreyCA <[email protected]>
This PR fixes a panic that occurred when parsing
agent.yamlfiles that don't have atemplatefield, and enables CI tests for the azure.ai.agents extension.Changes
Bug Fixes
templatefield. Now it returns a clear error message with documentation links."YAML content does not conform to AgentManifest format: YAML content does not conform to AgentManifest format: ..."to appear.agent:totemplate:in test samples to match the correct AgentManifest schema.Improvements
ci-test.ps1to run unit tests for the azure.ai.agents extension in the CI pipeline.Testing
Original prompt
azd ai agent initpanics during agent.yaml parsing with interface conversion error #6679✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.