[Agents Extension] Post deploy print update#7974
Conversation
…voke Signed-off-by: trangevi <trangevi@microsoft.com>
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
Pull request overview
Updates the Azure AI Agents extension deploy output to display invocation-ready agent endpoints (per protocol) instead of the agent version URL, aligning post-deploy guidance with how users actually call the agent.
Changes:
- Introduces per-protocol invocation endpoint construction (
responses/invocations) and uses it for deploy artifacts and env var registration. - Updates
Endpoints()to return protocol-specific endpoint env vars rather than a single version endpoint. - Adds unit tests covering protocol path mapping, endpoint construction, and deploy artifact labeling/note behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go |
Builds and surfaces per-protocol invocation endpoints; updates env var registration and endpoint reporting. |
cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go |
Adds tests validating protocol filtering/mapping and deploy artifact endpoint formatting. |
jongio
left a comment
There was a problem hiding this comment.
Clean approach to switching from version URLs to per-protocol invocation URLs. A few things:
- CI blocker: gofmt is failing - mixed tabs/spaces in the
deployHostedAgentarea (see inline). Endpoints()lost its validation that previously gave users a helpful error when env vars were not set. Now silently returns empty (see inline).- The
deployHostedAgentdoc comment was removed - worth restoring. - No test coverage for the updated
Endpoints()orregisterAgentEnvironmentVariablesmethods. The new helper functions have good tests, but the integration points that call them do not.
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #7974
Nice work on the per-protocol endpoint refactor, Travis! The approach is clean and the test coverage for the new helpers is solid. A few items to consider:
P1 — Should Fix
1. Protocol list duplication between Endpoints() and protocolPath()
Endpoints() hardcodes ["RESPONSES", "INVOCATIONS"] (line ~238) while protocolPath() maintains its own switch (line ~777). These must stay in sync manually — adding a new protocol requires changes in both places with no compiler safety net.
Consider extracting a shared constant or deriving the env var suffix from protocol constants so there's a single source of truth.
2. Stale AGENT_{KEY}_ENDPOINT env var not cleaned on upgrade
The old single-endpoint env var (AGENT_{KEY}_ENDPOINT) will persist in existing azd environments after re-deploy, alongside the new per-protocol vars (AGENT_{KEY}_RESPONSES_ENDPOINT, etc.). Downstream consumers (hooks, scripts) reading the old var will get a stale URL.
Consider deleting the legacy key in registerAgentEnvironmentVariables().
3. Endpoints() returns empty slice silently
If neither AGENT_{KEY}_RESPONSES_ENDPOINT nor AGENT_{KEY}_INVOCATIONS_ENDPOINT is set, Endpoints() returns nil, nil. Callers can't distinguish "no protocols configured" from "something went wrong." Consider adding a debug-level log when no endpoints are found after deploy.
P2 — Consider Fixing
- Missing unit test for
registerAgentEnvironmentVariableswith per-protocol env vars — would catch regressions in key formatting. - Missing edge case test for
nilprotocols slice (vs empty slice) inTestAgentInvocationEndpoints. - Deleted Godoc comment on
deployHostedAgent(line ~603) — looks accidental; consider restoring.
Signed-off-by: trangevi <trangevi@microsoft.com>
jongio
left a comment
There was a problem hiding this comment.
Addresses all my previous feedback. The displayableProtocols extraction is cleaner than what I suggested - single source of truth for both Endpoints() and protocolPath(). Legacy env var cleanup and default protocol fallback are good additions. CI is green.
wbreza
left a comment
There was a problem hiding this comment.
Code Review — PR #7974 (Re-review)
All previously raised items have been addressed cleanly:
- ✅ Protocol list duplication → shared displayableProtocols constant introduced
- ✅ Stale env var cleanup → legacy AGENT_{KEY}_ENDPOINT now explicitly cleared
- ✅ Silent empty endpoints → proper �xterrors.Dependency error with user guidance
- ✅ Test coverage → TestRegisterAgentEnvironmentVariables, nil edge cases added
- ✅ Godoc comment → restored and improved on deployHostedAgent
Nice turnaround, Travis! LGTM.
* Update end of deploy handling to print the urls which a user would invoke Signed-off-by: trangevi <trangevi@microsoft.com> * Pr comments Signed-off-by: trangevi <trangevi@microsoft.com> --------- Signed-off-by: trangevi <trangevi@microsoft.com> Co-authored-by: therealjohn <1501196+therealjohn@users.noreply.github.com>
Fixes #7942
Update the print to show the endpoint a user would use to invoke the agent.
