Add --call-id flag to send x-agent-foundry-call-id header on local invokes#8780
Conversation
Co-authored-by: ankitbko <3169316+ankitbko@users.noreply.github.com>
…vokes Co-authored-by: ankitbko <3169316+ankitbko@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
There was a problem hiding this comment.
Pull request overview
This PR updates the azure.ai.agents azd extension invocation flows by adding a local-only call ID header for azd ai agent invoke, while also refactoring the extension’s “isolation” header/flag story to a single --user-identity flag used across commands.
Changes:
- Add
--call-idtoazd ai agent invokeand send it asx-agent-foundry-call-idon--localinvokes (both responses + invocations protocols). - Replace the prior isolation-key flags (
--user-isolation-key,--chat-isolation-key, and--isolation-key) with a single--user-identityflag, mapping tox-agent-user-id(local) andx-ms-user-identity(remote). - Update tests, help text, changelog, and Fig completion snapshots to reflect the new flags/headers.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/operations.go | Replaces isolation header constants/options with user-identity headers and adds the call-id header constant. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/operations_test.go | Updates request-option/header assertions to the new user-identity header model. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/user_identity.go | Introduces shared --user-identity flag plumbing and helpers for local/remote header application; adds local call-id helper. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/invoke.go | Adds --call-id and applies local headers only on local invoke paths; switches remote header application to user identity. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_test.go | Updates tests to the new user-identity option model and propagation checks. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/invoke_call_id_test.go | Adds coverage to verify x-agent-foundry-call-id behavior for local invokes. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/session.go | Removes old isolation/session-ownership flags and migrates session commands to --user-identity. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/session_test.go | Updates session command flag tests to reflect the new --user-identity flag surface. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/files.go | Migrates file commands from isolation flags to --user-identity. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/files_test.go | Updates file command flag tests for --user-identity. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/monitor.go | Migrates monitor command from isolation flags to --user-identity. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/monitor_test.go | Updates monitor command flag tests for --user-identity. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/isolation_headers.go | Removes the old isolation flags implementation. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/update.go | Updates warning text to reference x-ms-user-identity. |
| cli/azd/extensions/azure.ai.agents/CHANGELOG.md | Documents the new --call-id flag and the breaking --user-identity flag consolidation. |
| cli/azd/cmd/testdata/TestFigSpec.ts | Updates completions snapshot to remove old isolation flags and add --user-identity (but needs --call-id added as well). |
Comments suppressed due to low confidence (1)
cli/azd/cmd/testdata/TestFigSpec.ts:1046
- [azd-code-reviewer] The Fig completion snapshot for
azd ai agent invokedoesn’t include the new--call-idflag, soTestFigSpec(and related snapshot-based checks) will likely fail and users won’t see the new option in completions.
{
name: ['--timeout', '-t'],
description: 'Request timeout in seconds (0 for no timeout)',
args: [
{
name: 'timeout',
},
],
},
{
name: ['--user-identity'],
description: 'User identity header value (sent as x-agent-user-id for local invocations and x-ms-user-identity for remote requests)',
args: [
{
name: 'user-identity',
},
],
},
{
Co-authored-by: ankitbko <3169316+ankitbko@users.noreply.github.com>
Resolved. I merged latest |
jongio
left a comment
There was a problem hiding this comment.
The isolation-key simplification and new --call-id flag both look correct. Local vs. remote header routing is clean, and the SessionRequestOptions consolidation removes real complexity.
One thing to address: the --call-id flag is registered on the invoke command but doesn't appear in the FigSpec snapshot (cli/azd/cmd/testdata/TestFigSpec.ts). The snapshot has the isolation-key removals and user-identity renames, but no call-id addition. Run UPDATE_SNAPSHOTS=true go test ./cmd -run 'TestFigSpec|TestUsage' to regenerate.
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
therealjohn
left a comment
There was a problem hiding this comment.
@trangevi note the breaking changes here.
user-isolation-key -> user-identity
chat-isolation-key -> removed
call-id -> added
Isolation Key concept has changed now in Foundry.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: ankitbko <3169316+ankitbko@users.noreply.github.com>
Co-authored-by: ankitbko <3169316+ankitbko@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
Test coverage fix looks good. The �ssertLocalHeader helper is clean, error assertions and request counting are solid additions.
Still outstanding from my earlier review: the FigSpec snapshot (cli/azd/cmd/testdata/TestFigSpec.ts) is missing the --call-id entry for invoke. Run UPDATE_SNAPSHOTS=true go test ./cmd -run 'TestFigSpec|TestUsage' to regenerate. This will fail the core azd pipeline when it runs.
jongio
left a comment
There was a problem hiding this comment.
The --call-id flag is registered on the invoke command but is not in this snapshot. The TestFigSpec test generates the spec from actual registered flags and compares to this file, so CI will fail until it is regenerated.
Run from cli/azd:
UPDATE_SNAPSHOTS=true go test ./cmd -run "TestFigSpec|TestUsage"
I flagged this in my previous two reviews and it hasn't been addressed yet, so requesting changes to block until the snapshot is updated.
Dismissing: this review was incorrectly posted as changes-requested due to an automation bug. The findings remain valid as comments.
jongio
left a comment
There was a problem hiding this comment.
The isolation-key consolidation into --user-identity and the new --call-id local-only header are both cleanly implemented. Header routing (local vs remote) is correct, the applyLocalCallIDHeader/applyLocalUserIdentityHeader split makes the scope obvious, and the test coverage (including the error assertions and request counting) is solid.
One item I'd like to confirm: the --call-id flag is registered on the invoke command but I don't see a corresponding entry added to the FigSpec snapshot (cli/azd/cmd/testdata/TestFigSpec.ts). The old isolation-key removals and user-identity renames are reflected there, but no --call-id addition. If the core TestFigSpec test generates from a build that includes extensions, this will fail when the core pipeline runs. If extensions aren't included in FigSpec generation, this is fine as-is.
Could you confirm whether the snapshot needs the --call-id entry? If so:
``
UPDATE_SNAPSHOTS=true go test ./cmd -run 'TestFigSpec|TestUsage'
jongio
left a comment
There was a problem hiding this comment.
Incremental review (1 new commit since my last review: c8fd7df added figspec regeneration).
My prior feedback has been addressed:
- Test coverage for
applyLocalUserIdentityHeaderwas added in9e497b5 - FigSpec snapshot was regenerated (correctly does not include extension-only flags)
The refactoring from multi-flag isolation keys to a single --user-identity flag is clean. The new --call-id flag is properly scoped to local-only paths. Breaking change is documented in the CHANGELOG.
One minor suggestion below on update.go.
Co-authored-by: ankitbko <3169316+ankitbko@users.noreply.github.com>
|
/check-enforcer override |
azd ai agent invokehad no way to pass a call ID to a locally running agent. This adds a--call-idflag that sends the value as thex-agent-foundry-call-idheader for local invocations only; it is never sent on remote Foundry requests.Changes
AgentFoundryCallIDHeader = "x-agent-foundry-call-id"inagent_api/operations.go.--call-idtoinvoke, threaded throughinvokeFlags.applyLocalCallIDHeaderhelper sets the header on both local paths (responsesLocal,invocationsLocal). Remote paths are untouched, so the header stays local-only.--call-idis set and absent when omitted.Closes #8796