azure.ai.agents: reject hosted-only agent fields on non-hosted kinds - #9691
azure.ai.agents: reject hosted-only agent fields on non-hosted kinds#9691yzxcj797 wants to merge 3 commits into
Conversation
codeConfiguration, policies, protocols, agentEndpoint and sessionConfiguration are dropped by load/deploy conversion for every non-hosted agent kind, so a manifest carrying them on e.g. a prompt-voice agent validated fine while silently having no effect. Reject them up front: - ValidateAgentDefinition reports each present hosted-only field for non-hosted kinds (presence is checked on the raw template keys so empty or partial values are also reported); - the azure.ai.agent.json schema gains the matching kind gate, in the style of the existing prompt-voice model rule. Fixes Azure#9623
|
Azure Pipelines: Successfully started running 1 pipeline(s). 6 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Thank you for your contribution yzxcj797! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull request overview
Restricts hosted-only agent fields to hosted agents in runtime and schema validation.
Changes:
- Adds schema and runtime restrictions.
- Adds prompt-voice and hosted validation tests.
- Updates an existing workflow test fixture.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
schemas/azure.ai.agent.json |
Adds conditional field restrictions. |
agent_yaml/parse.go |
Adds runtime hosted-only field validation. |
agent_yaml/parse_voice_test.go |
Adds validation tests. |
agent_yaml/parse_test.go |
Updates workflow test expectations. |
Suppressed comments (1)
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse.go:673
- This list only recognizes the inline camelCase spellings. The supported on-disk
agent.yamlcontract usescode_configuration,agent_endpoint, andsession_configuration(yaml.go:355,357,359), andvoiceAgentFromDefinitionFilepasses those raw bytes here. Those three spellings therefore still validate and are silently dropped for prompt-voice agents.
"codeConfiguration",
"policies",
"protocols",
"agentEndpoint",
"sessionConfiguration",
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "if": { | ||
| "not": { | ||
| "properties": { | ||
| "kind": { "const": "hosted" } | ||
| }, | ||
| "required": ["kind"] | ||
| } | ||
| }, |
| errors = append(errors, | ||
| validateNoHostedOnlyFields(templateBytes, agentDef.Kind)...) |
| { "required": ["codeConfiguration"] }, | ||
| { "required": ["policies"] }, | ||
| { "required": ["protocols"] }, | ||
| { "required": ["agentEndpoint"] }, | ||
| { "required": ["sessionConfiguration"] } |
…chema gate Per review: - the schema conditional now requires kind, so kind-less service configuration is untouched by the hosted-only gate; - LoadAndValidateAgentManifest validates the RAW template too: the typed remarshal drops hosted-only fields before ValidateAgentDefinition can see them; - the inline voice deployment path (VoiceAgentFromResolvedService) validates the raw resolved struct before toVoiceAgent drops the fields; - table-driven schema cases cover each hosted-only field on prompt-voice plus hosted and kind-less controls.
|
All three review points addressed in 626290c:
Full extension suite: 21 packages |
| package project | ||
|
|
||
| import ( | ||
| "google.golang.org/protobuf/encoding/protojson" |
| var hostedOnlyAgentFields = []string{ | ||
| "codeConfiguration", | ||
| "policies", | ||
| "protocols", | ||
| "agentEndpoint", | ||
| "sessionConfiguration", | ||
| } |
…rt order Per review: - standalone agent.yaml spells three of the hosted-only fields in snake_case (code_configuration, agent_endpoint, session_configuration, per the ContainerAgent yaml tags), so the raw-key check now covers both spellings; - the protojson import moves into the third-party group so the file is gofmt-clean.
|
Both second-round points addressed:
|
|
yzxcj797 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse_voice_test.go:177
- This hosted-kind success test supplies
agentEndpointas a scalar, butAgentEndpointis a struct (yaml.go:804-808). Unmarshalling the hosted definition therefore fails before the new allow-list behavior is tested. Use an object value so this case can actually pass.
agentEndpoint: https://example.com
cli/azd/extensions/azure.ai.agents/internal/project/doc_examples_test.go:822
- These five rejection cases can all pass because
kind: prompt-voicealready requiresmodel, which none of them supplies; thecodeConfigurationcase also uses a value that independently violates its requiredruntimeandentryPointfields. Add a valid model to every case and makecodeConfigurationvalid so each error is attributable to the new hosted-only conditional.
name: "session configuration rejected for prompt-voice",
mutate: func(value *fixture) {
value.value["kind"] = "prompt-voice"
value.value["sessionConfiguration"] = map[string]any{"idleTimeoutSeconds": 300}
},
cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go:1077
- No test exercises hosted-only field rejection through
VoiceAgentFromResolvedService; the existing round-trip test contains only valid voice fields. Add a service-config test with a raw hosted-only property and assert the validation error so the protojson/raw-key integration is covered, not only the helper itself.
// toVoiceAgent drops hosted-only fields; check the raw definition so
// they are rejected instead of silently ignored (#9623).
if rawBytes, err := protojson.Marshal(resolved); err == nil {
if err := agent_yaml.ValidateHostedOnlyFields(rawBytes); err != nil {
cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_yaml/parse.go:53
- The direct validator tests do not exercise this post-conversion check. Add a
LoadAndValidateAgentManifesttest with a prompt-voice template and a hosted-only raw key; otherwise this branch could be removed or broken while all new runtime tests still pass, reintroducing the silent drop in the manifest-loading path.
// The remarshal above reflects the typed conversion, which drops
// hosted-only fields before ValidateAgentDefinition can see them, so
// check the raw template too (#9623).
var rawManifest map[string]any
Fixes #9623.
Summary
In the
azure.ai.agentsextension,codeConfiguration,policies,protocols,agentEndpointandsessionConfigurationare hosted-agent-only:AgentDefinitionInline.toVoiceAgent()drops them for voice agents, yet both the schema andValidateAgentDefinitionaccepted them for every kind — so akind: prompt-voicemanifest carrying them validated fine while silently having no effect.Two aligned changes:
ValidateAgentDefinitionnow reports each present hosted-only field for non-hosted kinds ('<field>' is only supported for 'hosted' agents and is ignored for kind '<kind>'). Presence is checked on the raw template keys, so empty or partial values — which would unmarshal to zero fields — are reported too.azure.ai.agent.jsongains the matching draft-07 conditional (kind != hosted⇒ rejectanyOfof the fiverequiredfields), in the style of the existing prompt-voicemodelrule.Testing
TestValidateAgentDefinition_HostedOnlyFieldsRejectedOnPromptVoice(each of the five fields rejected on aprompt-voiceagent) andTestValidateAgentDefinition_HostedFieldsStillAllowedOnHosted(hosted kind unaffected).policieson aworkflowagent expecting success — withpoliciesnow hosted-only, a non-hosted agent free of the block stays valid, which is that case's premise.go test ./...in the extension passes; differential: with the runtime check reverted, the new rejection test fails withgot: <nil>.