feat(agents): add azd ai agent toolbox direct commands#8203
Conversation
hund030
requested review from
JeffreyCA,
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
therealjohn,
trangevi,
trrwilson,
vhvb1989,
wbreza and
weikanglim
as code owners
📋 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
This PR adds a new azd ai agent toolbox command group to the azure.ai.agents extension to manage Foundry toolboxes as versioned, connection-backed tool collections, including a local “pending toolbox” flow for create prior to the first connection add.
Changes:
- Introduces
toolboxCRUD-ish verbs (create,update,delete,show,list) plustoolbox connection add|remove|listunder the agent extension command tree. - Adds a per-endpoint pending-toolbox config store (bucketed by a hashed endpoint) and wires it into
create,show,list, and promotion onconnection add. - Extends the Foundry toolbox/projects Azure clients with toolbox pagination/version operations and a connection lookup without credentials, plus unit tests for the new command branches and helper functions.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/pkg/azure/foundry_toolsets_client.go | Adds shared JSON request helper, toolbox URL builder, cursor-pagination walker, and new toolbox/version operations. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/azure/foundry_projects_client.go | Adds GetConnection (no credentials) for toolbox connection resolution. |
| cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go | Adds toolbox-specific error codes and Azure service operation names for toolbox flows. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/root.go | Registers the new toolbox command group in the extension root command. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox.go | Adds toolbox parent command, cross-cutting flags, name/output validation, endpoint resolution, and 404 detection helper. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_client.go | Defines a toolboxClient interface to allow unit tests to stub the toolbox API. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_shared.go | Adds shared helpers for JSON emission, toolbox-not-found mapping, and tool connection ID walking. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_context.go | Adds toolbox/projects client constructors and endpoint parsing helpers. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_create.go | Implements toolbox create as a local pending record (no initial POST). |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_update.go | Implements toolbox update (PATCH default version only). |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_delete.go | Implements toolbox delete including guarded per-version delete semantics and pending-record clearing. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_show.go | Implements toolbox show, including MCP endpoint computation and pending-record rendering. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_list.go | Implements toolbox list, merging live toolboxes with local pending entries. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_connection.go | Adds toolbox connection add and tool-entry construction logic based on connection category. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_connection_actions.go | Implements toolbox connection remove and toolbox connection list. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_connection_resolver.go | Adds a resolver that fetches a project connection (no credentials) and maps it into toolbox-ready shape. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/pending_toolboxes.go | Implements per-endpoint pending-toolbox storage and the pendingToolboxStore abstraction. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_test_helpers_test.go | Adds mock toolbox client, stub connection resolver, and in-memory pending store for tests. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_helpers_test.go | Adds unit tests for helper utilities (validation, tool entry building, filtering, URL building, hashing). |
| cli/azd/extensions/azure.ai.agents/internal/cmd/toolbox_commands_test.go | Adds unit tests covering command branch behavior and key error cases. |
| 'connection add' against the same toolbox name publishes v1 and clears the | ||
| pending record.`, | ||
| Args: cobra.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
Return an error with suggestion so its clear what to specify. All it says now is:
ERROR: accepts 1 arg(s), received 0
I'd expect it show more like the --help comamnd does:
Usage:
agent toolbox create <name> [flags]
Foundry requires a non-empty tool list on the first POST, so 'create' does not
contact the service. Instead it records a local pending entry. The first
'connection add' against the same toolbox name publishes v1 and clears the
pending record.
There was a problem hiding this comment.
That might be outside of extension control, I think that's coming from Args: cobra.ExactArgs(1) being specified
| } | ||
| fmt.Printf( | ||
| "Registered toolbox %s (pending tools). "+ | ||
| "Run 'azd ai agent toolbox connection add %s <connection>' to publish v1.\n", |
There was a problem hiding this comment.
Improve the formatting for this so its not all on one line - there might be an existing pattern for showing next step guidance and coloring, etc.
If not, then just make each sentence on a new line.
|
|
||
| cmd.Flags().StringVar( | ||
| &flags.index, "index", "", | ||
| "Index name (required when the connection's category is CognitiveSearch).", |
There was a problem hiding this comment.
What is CognitiveSearch referring to? Need to check if that is the correct branding/service name now.
| return nil, exterrors.Validation( | ||
| exterrors.CodeUnsupportedConnectionCategory, | ||
| fmt.Sprintf( | ||
| "connection %q has category %q; v1 supports RemoteTool and CognitiveSearch only", |
There was a problem hiding this comment.
I don't expect this limitation.
| } | ||
| if promoted { | ||
| fmt.Printf( | ||
| "Published toolbox %s version %s with connection %s.\n", |
There was a problem hiding this comment.
Add the toolbox endpoint to the output on a new line so its easily copied by the dev after its published.
A better suggestion, if in an AZD project already, would be to recommend setting an env var to use like:
azd env set <toolboxname>_MCP_ENDPOINT=<endpoint>
But fill in the values for the placeholders.
| 'connection add' against the same toolbox name publishes v1 and clears the | ||
| pending record.`, | ||
| Args: cobra.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { |
There was a problem hiding this comment.
That might be outside of extension control, I think that's coming from Args: cobra.ExactArgs(1) being specified
| CognitiveSearch → azure_ai_search tool (requires --index) | ||
| Other categories are rejected. | ||
|
|
||
| If the toolbox has a local pending record (from 'toolbox create'), v1 is |
There was a problem hiding this comment.
This means that if I want to create a toolbox with 3 tools, I'm going to end up on v3 before I'm done. This feels incredibly painful from a user perspective, as I'm not actually iterating on desired changes, I'm being forced into it. Is there a way this can be done without all these added, unwanted, versions?
| if _, err := store.Clear(ctx, endpoint, toolboxName); err != nil { | ||
| return exterrors.Internal( | ||
| exterrors.CodePendingToolboxStoreFailed, | ||
| fmt.Sprintf("failed to clear pending toolbox record: %s", err), |
There was a problem hiding this comment.
What is the implication here? Is this something the user can recover from? At the very least we should probably indicate that this is a client side only failure, and that the toolbox version was still created correctly.
| return exterrors.ServiceFromAzure(err, exterrors.OpGetToolbox) | ||
| } | ||
|
|
||
| current, err := client.GetToolboxVersion(ctx, toolboxName, tb.DefaultVersion) |
There was a problem hiding this comment.
Do we want a way to add a connection to the non-default toolbox version?
| @@ -0,0 +1,281 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
Is there a reason that the add command is in one file, and the other commands are in a different file? I'd suggest either a separate file per command, like the toolbox base commands, or everything in one. Could separate out the helper methods if desired to help clean things up.
| return exterrors.Validation( | ||
| exterrors.CodeMissingForceFlag, | ||
| "--no-prompt requires --force on destructive operations", | ||
| "add --force to confirm the deletion non-interactively", |
There was a problem hiding this comment.
Wouldn't this only apply if we're trying to delete the default?
| } | ||
|
|
||
| // Drop pending records that already exist live-side to avoid duplicates. | ||
| liveNames := map[string]struct{}{} |
There was a problem hiding this comment.
If the toolbox has been published to the project, we shouldn't have a pending entry for it any more, correct? Is this necessary?
| extCtx = ensureExtensionContext(extCtx) | ||
| cmd := &cobra.Command{ | ||
| Use: "list", | ||
| Short: "List toolboxes on the project, plus any local pending records.", |
There was a problem hiding this comment.
Does this list all versions, or just the defaults?
| Short: "Show a toolbox version, including its computed MCP endpoint.", | ||
| Long: `Show a toolbox. | ||
|
|
||
| By default shows the default version. Use --version to inspect a specific |
There was a problem hiding this comment.
How can a user know how many versions a toolbox has?
| Long: `Update a toolbox. | ||
|
|
||
| Only --default-version is mutable through PATCH today (§ 4.1). To edit the | ||
| description or the tool list, publish a new version with 'connection add' or |
There was a problem hiding this comment.
connection add doesn't let a user change the description, does it?
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Technical findings from code-level analysis. I'm not restating the design/UX feedback from @therealjohn and @trangevi; their reviews cover the broader picture.
Three findings, one medium priority:
-
[MED] Non-atomic version promotion
connection addandconnection removeboth callCreateToolboxVersionthenSetDefaultVersionas two separate API calls. If the second fails, there's an orphaned version that isn't the default, and the error doesn't include the created version number, so recovery viatoolbox update --default-versionrequires the user to first figure out which version got created. Same pattern intoolbox_connection_actions.goaround line 120. -
[LOW] Silent pagination truncation
listPagedFromClientreturns partial results without error when the server responds withhas_more=truebut provides no usable cursor (lines 161-164 offoundry_toolsets_client.go). Unlikely in practice, but alog.Printfwarning here would make debugging much easier if it ever happens. -
[LOW] No length cap on toolbox/tool names
toolboxNamePatternis^[A-Za-z0-9_-]+$with no min/max bounds. The existing agent name validation inparse.goenforces 1-63 chars. Worth adding a length cap so users get a clear local error rather than a less helpful service-side 400.
|
|
||
| if _, err := client.SetDefaultVersion(ctx, toolboxName, created.Version); err != nil { | ||
| return exterrors.ServiceFromAzure(err, exterrors.OpSetDefaultVersion) | ||
| } |
There was a problem hiding this comment.
If SetDefaultVersion fails here, we've already POSTed a new version to the service. That version exists server-side but isn't the default, and the error doesn't tell the user which version was created.
Two options: (a) include created.Version in the error so users can recover with toolbox update --default-version <v>, or (b) attempt a best-effort DeleteToolboxVersion rollback (with a log warning if that fails too).
Same pattern exists in connection remove (toolbox_connection_actions.go around line 120).
| if last == "" { | ||
| // HasMore=true with no cursor: bail rather than spin. | ||
| return out, nil | ||
| } |
There was a problem hiding this comment.
When HasMore is true but there's no usable cursor, this silently returns partial results. Callers like ListToolboxes and ListToolboxVersions won't know they got incomplete data. A log.Printf warning here would help debugging, something like pagination: has_more=true but no cursor for %s; returning %d items.
| } | ||
|
|
||
| // toolboxNamePattern is the validation regex for toolbox and tool names per § 4.2. | ||
| var toolboxNamePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) |
There was a problem hiding this comment.
This matches any length. The agent name regex in parse.go enforces 1-63 chars. Adding a length cap here would catch overly long names locally instead of letting them hit the server and come back as a less helpful 400.
Summary
Adds the
azd ai agent toolboxcommand group to theazure.ai.agentsextension so users can manage versioned, connection-backed tool collections without leaving their terminal. Closes #8143. Implements the design spec from #8160.Changes
internal/cmd/toolbox*.go: six verbs (create,update,delete,show,list,connection {add,remove,list}) and the parent command.createrecords a local pending entry; the firstconnection addPOSTs v1 and clears it.connection addresolvesRemoteToolconnections tomcptool entries andCognitiveSearch(with--index) toazure_ai_searchentries; later mutations fetch the default version, edittools[], POST a new version, then PATCHdefault_version. Reuses the centralresolveProjectEndpointcascade and theResolvedEndpointsource enum.internal/cmd/pending_toolboxes.go: per-endpoint pending-toolbox store underextensions.ai-agents.pending-toolboxes.<sha256(endpoint)[:16]>(§ 7); exposed via apendingToolboxStoreinterface so tests can substitute.internal/pkg/azure/foundry_toolsets_client.go: extended withListToolboxes,GetToolboxVersion,ListToolboxVersions,DeleteToolboxVersion,SetDefaultVersion, capped paginator, andEndpoint(). Pipeline helpers (doJSON,toolboxURL, genericlistPagedFromClient[T]) collapse the per-method boilerplate.internal/pkg/azure/foundry_projects_client.go: addsGetConnection(ctx, name)(no credentials) for the connection resolver.internal/exterrors/codes.go: new codes (CodeToolboxNotFound,CodeDefaultVersionDelete,CodeOnlyVersionDelete,CodeUnsupportedConnectionCategory,CodeMissingIndex,CodeUnsupportedIndexFlag,CodeDuplicateConnection,CodeConnectionNotFound,CodeConnectionMissingTarget,CodeConnectionNotInToolbox,CodeLastToolRemoval,CodeMissingForceFlag,CodeInvalidToolboxName,CodeMissingUpdateField,CodePendingToolboxStoreFailed,CodeLastToolRemoval) andOp*constants forServiceFromAzure.toolboxClientandconnectionResolver, plus the shared helpers (buildToolEntry,filterOutConnection,duplicateConnectionInTools,buildToolboxMcpURL,endpointBucketKey).