Support invocations_ws in agent init#9009
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds invocations_ws support and improves non-interactive agent initialization.
Changes:
- Adds protocol support for code and image deployments.
- Adds Azure context flags and region fallback updates.
- Enhances unified sample adoption and model configuration.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
schemas/azure.ai.agent.json |
Documents invocations_ws. |
internal/pkg/agents/agent_yaml/map_test.go |
Tests API protocol serialization. |
internal/cmd/init.go |
Adds flags and image protocol synthesis. |
internal/cmd/init_test.go |
Tests image protocol handling. |
internal/cmd/init_locations.go |
Merges live and embedded regions. |
internal/cmd/init_locations_test.go |
Tests region merging. |
internal/cmd/init_from_code.go |
Adds protocol and Azure context support. |
internal/cmd/init_from_code_test.go |
Tests protocol selection. |
internal/cmd/init_foundry_resources_helpers.go |
Applies Azure context flags and adjusts model resolution. |
internal/cmd/init_adopt.go |
Applies agent and model overrides during adoption. |
internal/cmd/hosted-agent-regions.json |
Adds westus2. |
cspell.yaml |
Adds VOICELIVE spelling. |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Scope here is clean and the invocations_ws wiring lines up: knownProtocols entry, the image-path protocolRecordsForImageManifest/resolveKnownProtocols helpers (default, dedup, activity rejection, unknown-protocol error), and the schema/help/region updates. Test coverage across the flag, interactive, image-manifest, and code-deploy serialization paths is thorough, and the default protocol staying responses is locked down by its own test.
One non-blocking follow-up, already noted by the bot: an invocations_ws-only project still gets an �zd ai agent invoke --local next-step hint even though that protocol isn't invocable. Pre-existing next-step behavior, fine to handle separately.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Scope here is clean and the invocations_ws wiring lines up: knownProtocols entry, the image-path protocolRecordsForImageManifest/resolveKnownProtocols helpers (default, dedup, activity rejection, unknown-protocol error), and the schema/help/region updates. Test coverage across the flag, interactive, image-manifest, and code-deploy serialization paths is thorough, and the default protocol staying responses is locked down by its own test.
One non-blocking follow-up, already noted by the bot: an invocations_ws-only project still gets an "azd ai agent invoke --local" next-step hint even though that protocol is not invocable. Pre-existing next-step behavior, fine to handle separately.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
On the new commit: loadServiceProtocol now early-returns on invocations_ws, so when a service declares both invocations_ws and responses the classification depends on their order. With invocations_ws first it is treated as websocket-only and drops the invoke hint; with responses first it is treated as responses. The function's own doc comment says responses wins when multiple protocols are declared, and invocations is deliberately handled as flag-and-continue to hold that invariant. Treating invocations_ws the same way (using it only when no azd-invocable protocol is declared) keeps the classification order-independent. A reversed-order test case would lock it in. This is already flagged inline; noting it is still worth addressing before merge.
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
The order-dependence I flagged in loadServiceProtocol is resolved: invocations_ws now sets a flag instead of early-returning, so precedence stays responses > invocations > invocations_ws no matter what order the manifest declares them. The reversed-order test cases lock that in.
serviceSupportsAzdInvoke is applied consistently across ResolveAfterRun, ResolveAfterDeploy, and appendInvokeLocalSecondary, and the run/deploy suppression paths (single websocket agent and mixed multi-agent) have coverage.
…erviceProtocol Fixes review comment r3601867100 on PR #9009. The original code returned ProtocolInvocationsWS immediately when encountered in the protocols list, making the selection order-dependent. If invocations_ws appeared before invocations in the list, the function would return early without checking for the higher-priority invocations protocol. The fix uses the same flag-based approach as invocations: track sawInvocationsWS and only return ProtocolInvocationsWS if neither responses nor invocations was found. Priority order is now: responses > invocations > invocations_ws (order-independent). Also includes the full PR 9009 changes: - Add invocations_ws to knownProtocols in init_from_code.go - Support --protocol invocations_ws in synthesizeImageManifestFile - Add invocations_ws to schema description - Add westus2 to hosted-agent-regions.json - Add VOICELIVE to cspell dictionary - Add tests for all new behavior Co-authored-by: v1212 <49907914+v1212@users.noreply.github.com>
| } | ||
|
|
||
| func serviceSupportsAzdInvoke(svc *ServiceState) bool { | ||
| return svc == nil || svc.Protocol != ProtocolInvocationsWS |
There was a problem hiding this comment.
How does this handle agents with multiple protocols?
| if slices.Contains(flagProtocols, "activity") { | ||
| return nil, exterrors.Validation( | ||
| exterrors.CodeInvalidAgentManifest, | ||
| "--protocol activity is not supported with --image", |
There was a problem hiding this comment.
Why is the activity protocol not supported here?
There was a problem hiding this comment.
This is also confusing because if the failure on line 699 in resolveKnownProtocols is raised, it will include the activity protocol in that list of known protocols
|
|
||
| cmd.Flags().StringSliceVar(&flags.protocols, "protocol", nil, | ||
| "Protocols supported by the agent (e.g., 'responses', 'invocations'). Can be specified multiple times.") | ||
| "Protocols supported by the agent (e.g., 'responses', 'invocations', 'invocations_ws'). "+ |
There was a problem hiding this comment.
Similar question, is there a reason the activity protocol isn't listed? Should this list be dynamic, so we don't miss more?
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Circling back after the latest review activity. The two invocations_ws findings from the bot both check out, and they trace to one root cause: serviceSupportsAzdInvoke returns true for a nil service, so the "no specific service" case defaults to invocable.
-
Run flow prints a dangling header for websocket agents. emitNextAfterBind (run.go:307) unconditionally prints "Agent ready. In another terminal, try:" and then calls ResolveAfterRun, which now early-returns nil for invocations_ws (resolver.go:336). printNextIfTerminal no-ops on the empty slice, so an invocations_ws local run ends with that header and no command under it. run.go isn't touched in this PR, but the early-return added here is what surfaces it. Resolving the suggestions first and skipping the header when the list is empty would fix it.
-
Multi-agent, all-websocket still suggests invoke --local. In appendInvokeLocalSecondary, multiple services leave svc nil, so serviceSupportsAzdInvoke(nil) is true and the invoke --local hint is appended even when every service is invocations_ws. Gating on "at least one service is invocable" for the multi-service case covers it.
Both are on the unchanged HEAD I approved earlier, so I'm flagging them here rather than letting that approval imply the websocket paths are fully covered. trangevi's activity-protocol questions on init.go are separate and worth a direct reply from the author.
Summary
invocations_wsas a selectable protocol forazd ai agent initwhile keeping the default protocol asresponses.--protocol invocations_wsfor bring-your-own-image--imageinit.invocations_wsin agent help/schema descriptions.protocol_versions.Sample used for validation
Official sample:
https://github.com/microsoft-foundry/foundry-samples/tree/main/samples/python/hosted-agents/bring-your-own/invocations_ws/hello-world
Validation
go test ./internal/cmd/...fromcli/azd/extensions/azure.ai.agentsgo test ./...fromcli/azd/extensions/azure.ai.agentsgo build ./...fromcli/azd/extensions/azure.ai.agentscspell lint "extensions/**/*.go" "extensions/**/*.md" --config ./.vscode/cspell.yaml --no-progressfromcli/azdFixes #9173