diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/root.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/root.go index 2daac24407a..dfcffe5fdd2 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/root.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/root.go @@ -16,9 +16,10 @@ func NewRootCommand() *cobra.Command { Long: `Manage Foundry toolboxes. A toolbox is a versioned, named collection of connection-backed tools that -agents reference at run time. Each version is immutable and carries the full -tool list; mutations publish a new version and (after the first one) require -an explicit update to retarget the default.`, +agents reference at run time. Each version is immutable: mutations (connection +add/remove, skill add/remove) create a new version but never change which +version is the default. Use 'azd ai toolbox publish ' +to promote a version.`, }) rootCmd.SilenceUsage = true @@ -42,12 +43,13 @@ an explicit update to retarget the default.`, registerToolboxOutputFlag(rootCmd) rootCmd.AddCommand(newToolboxCreateCommand(extCtx)) - rootCmd.AddCommand(newToolboxUpdateCommand(extCtx)) + rootCmd.AddCommand(newToolboxPublishCommand(extCtx)) rootCmd.AddCommand(newToolboxDeleteCommand(extCtx)) rootCmd.AddCommand(newToolboxShowCommand(extCtx)) rootCmd.AddCommand(newToolboxListCommand(extCtx)) rootCmd.AddCommand(newToolboxVersionCommand(extCtx)) rootCmd.AddCommand(newToolboxConnectionCommand(extCtx)) + rootCmd.AddCommand(newToolboxSkillCommand(extCtx)) rootCmd.AddCommand(newVersionCommand(&extCtx.OutputFormat)) rootCmd.AddCommand(newMetadataCommand(rootCmd)) diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_commands_test.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_commands_test.go index 9e69020e82c..0c5dd080944 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_commands_test.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_commands_test.go @@ -43,7 +43,7 @@ func TestRunToolboxDeleteWith_Branches(t *testing.T) { toolboxDeleteFlags{version: "2", force: true}, toolboxFlags{output: "table"}, ) le := requireLocalError(t, err, exterrors.CodeDefaultVersionDelete) - assert.Contains(t, le.Suggestion, "default-version") + assert.Contains(t, le.Suggestion, "azd ai toolbox publish") assert.Empty(t, client.deleteVersionCalls, "service must not be called") }) @@ -230,7 +230,7 @@ func TestRunConnectionAddWith_AppendsAndPromotesDefault(t *testing.T) { require.NotNil(t, req.Policies, "policies must be carried forward") require.NotNil(t, req.Policies.RaiConfig) assert.Equal(t, "Microsoft.Default", req.Policies.RaiConfig.RaiPolicyName) - require.Len(t, client.setDefaultCalls, 1, "default version must be retargeted") + assert.Empty(t, client.setDefaultCalls, "mutation verbs no longer auto-promote default") } func TestRunConnectionAddWith_ConnectionNotFound(t *testing.T) { @@ -290,7 +290,7 @@ func TestRunConnectionAddWith_FromFileAddsMultipleToolsSingleVersion(t *testing. require.NoError(t, err) require.Len(t, client.createVersionCalls, 1, "single version increment for batch input") assert.Len(t, client.createVersionCalls[0].req.Tools, 3, "existing + 2 additions") - require.Len(t, client.setDefaultCalls, 1) + assert.Empty(t, client.setDefaultCalls) } // Public entry-point validation: empty connection without --from-file. @@ -319,7 +319,9 @@ func TestRunConnectionRemoveWith_LastToolBlocks(t *testing.T) { err := runConnectionRemoveWith( t.Context(), client, resolver, "https://e/", - "tb", "a", connectionRemoveFlags{force: true}, toolboxFlags{output: "table"}, + "tb", []string{"a"}, + connectionRemoveFlags{force: true}, + toolboxFlags{output: "table"}, ) requireLocalError(t, err, exterrors.CodeLastToolRemoval) assert.Empty(t, client.createVersionCalls) @@ -344,7 +346,9 @@ func TestRunConnectionRemoveWith_FilteredAndPromoted(t *testing.T) { err := runConnectionRemoveWith( t.Context(), client, resolver, "https://e/", - "tb", "a", connectionRemoveFlags{force: true}, toolboxFlags{output: "json"}, + "tb", []string{"a"}, + connectionRemoveFlags{force: true}, + toolboxFlags{output: "json"}, ) require.NoError(t, err) require.Len(t, client.createVersionCalls, 1) @@ -352,7 +356,7 @@ func TestRunConnectionRemoveWith_FilteredAndPromoted(t *testing.T) { assert.Len(t, req.Tools, 1) require.NotNil(t, req.Policies, "policies must be carried forward on remove") assert.Equal(t, "Microsoft.Default", req.Policies.RaiConfig.RaiPolicyName) - require.Len(t, client.setDefaultCalls, 1) + assert.Empty(t, client.setDefaultCalls) } func TestRunConnectionRemoveWith_ConnectionNotInToolbox(t *testing.T) { @@ -370,7 +374,9 @@ func TestRunConnectionRemoveWith_ConnectionNotInToolbox(t *testing.T) { err := runConnectionRemoveWith( t.Context(), client, resolver, "https://e/", - "tb", "a", connectionRemoveFlags{force: true}, toolboxFlags{output: "table"}, + "tb", []string{"a"}, + connectionRemoveFlags{force: true}, + toolboxFlags{output: "table"}, ) requireLocalError(t, err, exterrors.CodeConnectionNotInToolbox) } @@ -400,10 +406,9 @@ func TestRunConnectionListWith_EmitsAllShapes(t *testing.T) { require.NoError(t, err) } -func TestRunToolboxUpdate_MissingDefaultVersion(t *testing.T) { - err := runToolboxUpdate( - t.Context(), "tb", - toolboxUpdateFlags{}, +func TestRunToolboxPublish_WhitespaceVersion(t *testing.T) { + err := runToolboxPublish( + t.Context(), "tb", " ", toolboxFlags{output: "table"}, ) requireLocalError(t, err, exterrors.CodeMissingUpdateField) @@ -435,6 +440,77 @@ connections: assert.Len(t, client.createVersionCalls[0].req.Tools, 1) } +func TestRunToolboxCreateWith_SkillsFromFile(t *testing.T) { + client := newMockToolboxClient("https://e/") + resolver := newStubConnectionResolver() + resolver.byName["mcp"] = &projectConnection{ + ID: "/c/mcp", Category: connections.ConnectionTypeRemoteTool, Name: "mcp", + Target: "https://mcp.example.com", + } + + inputPath := t.TempDir() + "/create.yaml" + require.NoError(t, os.WriteFile(inputPath, []byte(` +description: tb with skills +connections: + - name: mcp +skills: + - name: pinned + version: "3" + - name: unpinned +`), 0o600)) + + err := runToolboxCreateWith( + t.Context(), client, resolver, "https://e/", "tb", + toolboxCreateFlags{fromFile: inputPath}, + toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + + skills := client.createVersionCalls[0].req.Skills + require.Len(t, skills, 2) + + byName := map[string]map[string]any{} + for _, s := range skills { + n, _ := s["name"].(string) + byName[n] = s + } + require.Contains(t, byName, "pinned") + require.Contains(t, byName, "unpinned") + assert.Equal(t, "skill_reference", byName["pinned"]["type"]) + assert.Equal(t, "3", byName["pinned"]["version"]) + _, hasVersion := byName["unpinned"]["version"] + assert.False(t, hasVersion, "skill without version must omit the version key") +} + +func TestRunToolboxCreateWith_DuplicateSkillRejected(t *testing.T) { + client := newMockToolboxClient("https://e/") + resolver := newStubConnectionResolver() + resolver.byName["mcp"] = &projectConnection{ + ID: "/c/mcp", Category: connections.ConnectionTypeRemoteTool, Name: "mcp", + Target: "https://mcp.example.com", + } + + inputPath := t.TempDir() + "/create.yaml" + require.NoError(t, os.WriteFile(inputPath, []byte(` +description: tb +connections: + - name: mcp +skills: + - name: dup + - name: dup + version: "2" +`), 0o600)) + + err := runToolboxCreateWith( + t.Context(), client, resolver, "https://e/", "tb", + toolboxCreateFlags{fromFile: inputPath}, + toolboxFlags{output: "json"}, + ) + requireLocalError(t, err, exterrors.CodeDuplicateSkill) + assert.Empty(t, client.createVersionCalls, "no version should be created when local validation fails") +} + func TestRunToolboxCreateWith_AlreadyExists(t *testing.T) { client := newMockToolboxClient("https://e/") client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{Name: "tb", DefaultVersion: "1"}} @@ -733,11 +809,107 @@ func TestRunToolboxVersionListWith_ListVersionsServiceError(t *testing.T) { } func TestRunConnectionRemove_NoPromptWithoutForce(t *testing.T) { - err := runConnectionRemove( - t.Context(), "tb", "conn", + err := runConnectionRemove(t.Context(), "tb", []string{"conn"}, connectionRemoveFlags{force: false}, toolboxFlags{output: "table", noPrompt: true}, newStubConnectionResolver(), ) requireLocalError(t, err, exterrors.CodeMissingForceFlag) } + +// Carry-forward: skills attached to the current default version must survive +// across new versions created by `connection add`. +func TestRunConnectionAddWith_CarriesForwardSkills(t *testing.T) { + skills := []map[string]any{ + {"type": "skill_reference", "name": "alpha", "version": "1"}, + {"type": "skill_reference", "name": "beta"}, + } + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", Description: "first", + Tools: []map[string]any{ + {"type": "mcp", "name": "a", "project_connection_id": "/c/a"}, + }, + Skills: skills, + }} + resolver := newStubConnectionResolver() + resolver.byName["b"] = &projectConnection{ + ID: "/c/b", Category: connections.ConnectionTypeRemoteTool, Name: "b", Target: "https://mcp-b", + } + + err := runConnectionAddWith( + t.Context(), client, resolver, "https://e/", + "tb", "b", connectionAddFlags{}, toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + assert.Equal(t, skills, client.createVersionCalls[0].req.Skills, + "skills must be carried forward verbatim into the new version") +} + +// Carry-forward: skills attached to the current default version must survive +// across new versions created by `connection remove`. +func TestRunConnectionRemoveWith_CarriesForwardSkills(t *testing.T) { + skills := []map[string]any{ + {"type": "skill_reference", "name": "alpha"}, + } + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{ + {"type": "mcp", "name": "a", "project_connection_id": "/c/a"}, + {"type": "mcp", "name": "b", "project_connection_id": "/c/b"}, + }, + Skills: skills, + }} + resolver := newStubConnectionResolver() + resolver.byName["a"] = &projectConnection{ + ID: "/c/a", Category: connections.ConnectionTypeRemoteTool, Name: "a", + } + + err := runConnectionRemoveWith( + t.Context(), client, resolver, "https://e/", + "tb", []string{"a"}, + connectionRemoveFlags{force: true}, + toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + assert.Equal(t, skills, client.createVersionCalls[0].req.Skills, + "skills must be carried forward verbatim into the new version") +} + +// Batch removal via variadic positionals. +func TestRunConnectionRemoveWith_VariadicPositionals(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{ + {"type": "mcp", "name": "a", "project_connection_id": "/c/a"}, + {"type": "mcp", "name": "b", "project_connection_id": "/c/b"}, + {"type": "mcp", "name": "c", "project_connection_id": "/c/c"}, + }, + }} + resolver := newStubConnectionResolver() + resolver.byName["a"] = &projectConnection{ID: "/c/a", Name: "a", Category: connections.ConnectionTypeRemoteTool} + resolver.byName["b"] = &projectConnection{ID: "/c/b", Name: "b", Category: connections.ConnectionTypeRemoteTool} + + err := runConnectionRemoveWith( + t.Context(), client, resolver, "https://e/", + "tb", []string{"a", "b"}, + connectionRemoveFlags{force: true}, toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1, "one new version created for the whole batch") + require.Len(t, client.createVersionCalls[0].req.Tools, 1) + assert.Equal(t, "/c/c", client.createVersionCalls[0].req.Tools[0]["project_connection_id"]) +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection.go index 8575017c315..1dd34b255d8 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection.go @@ -24,8 +24,9 @@ func newToolboxConnectionCommand(extCtx *azdext.ExtensionContext) *cobra.Command Tools are project connections. Supported categories: RemoteTool (MCP), CognitiveSearch (Azure AI Search), RemoteA2A, and GroundingWithCustomSearch. -Each mutation publishes a new immutable version and retargets the toolbox -default.`, +Each mutation creates a new immutable version; the toolbox's default version +is unchanged. Use 'azd ai toolbox publish ' +to promote a version.`, } cmd.AddCommand(newToolboxConnectionAddCommand(extCtx)) cmd.AddCommand(newToolboxConnectionRemoveCommand(extCtx)) diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_add.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_add.go index ee1ec1670a0..da8ddc412f4 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_add.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_add.go @@ -31,7 +31,7 @@ func newToolboxConnectionAddCommand(extCtx *azdext.ExtensionContext) *cobra.Comm cmd := &cobra.Command{ Use: "add [connection]", Short: "Attach one or more connections to a toolbox.", - Long: `Attach one or more tools to a toolbox and publish a new default version. + Long: `Attach one or more tools to a toolbox and create a new version. This command has two modes: @@ -42,16 +42,18 @@ Single-connection mode: Pass the project connection's short name as the positional. --index is required when the connection's category is CognitiveSearch (Azure AI Search). --instance-name is required when the category is GroundingWithCustomSearch. -Only one tool is appended; the new version becomes the default. File mode: azd ai toolbox connection add --from-file Provide a JSON or YAML file with multiple connections. All inputs from a -single invocation publish exactly one new toolbox version, so adding three +single invocation create exactly one new toolbox version, so adding three connections this way produces v(N+1), not v(N+3). +The new version is created but the toolbox's default version is unchanged; +run 'azd ai toolbox publish ' to promote it. + ` + fileShapeBlurb(false) + ` At least one connection must be provided. @@ -237,6 +239,7 @@ func runConnectionAddWith( Description: current.Description, Metadata: current.Metadata, Tools: newTools, + Skills: current.Skills, Policies: current.Policies, } created, err := client.CreateToolboxVersion(ctx, toolboxName, req) @@ -244,20 +247,6 @@ func runConnectionAddWith( return exterrors.ServiceFromAzure(err, exterrors.OpCreateToolboxVersion) } - if _, err := client.SetDefaultVersion(ctx, toolboxName, created.Version); err != nil { - return exterrors.Dependency( - exterrors.CodeSetDefaultVersionFailed, - fmt.Sprintf( - "toolbox %q version %q was created but could not be promoted to default: %s", - toolboxName, created.Version, err, - ), - fmt.Sprintf( - "run `azd ai toolbox update %q --default-version %q` to retarget the default", - toolboxName, created.Version, - ), - ) - } - return emitConnectionAddResult(toolboxName, created.Version, addedConnectionNames, parent.output, endpoint) } @@ -311,10 +300,12 @@ func emitConnectionAddResult( return emitJSON(payload) } - fmt.Printf("Attached connection(s) to toolbox %s (now at version %s).\n", toolboxName, newVersion) + fmt.Printf("Created toolbox %s version %s.\n", toolboxName, newVersion) if len(connectionNames) > 0 { fmt.Printf("Connections: %s\n", strings.Join(connectionNames, ", ")) } fmt.Printf("Endpoint: %s\n", mcpURL) + fmt.Printf("The default version is unchanged; "+ + "run `azd ai toolbox publish %q %q` to promote.\n", toolboxName, newVersion) return nil } diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_remove.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_remove.go index ba19ce4a2a7..bc5b9b7eebf 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_remove.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_connection_remove.go @@ -6,6 +6,7 @@ package cmd import ( "context" "fmt" + "slices" "strings" "azure.ai.toolboxes/internal/exterrors" @@ -26,17 +27,24 @@ func newToolboxConnectionRemoveCommand(extCtx *azdext.ExtensionContext) *cobra.C flags := &connectionRemoveFlags{} cmd := &cobra.Command{ - Use: "remove ", - Short: "Detach a project connection from a toolbox.", - Long: `Detach a project connection from a toolbox. - -Publishes a new default version with the named connection's tool entry -removed. Refuses to leave the toolbox with zero tools (use 'toolbox delete' -instead).`, - Args: cobra.ExactArgs(2), + Use: "remove ...", + Short: "Detach one or more connections from a toolbox.", + Long: `Detach one or more connections from a toolbox and create a new version. + +Pass one or more connection short names as positionals. All removals are +applied atomically: each invocation creates exactly one new toolbox version. + +Refuses to leave the toolbox with zero tools (use 'toolbox delete' instead). + +Examples: + + azd ai toolbox connection remove research my-mcp + azd ai toolbox connection remove research a b c --force +`, + Args: cobra.MinimumNArgs(2), RunE: func(cmd *cobra.Command, args []string) error { return runConnectionRemove( - cmd.Context(), args[0], args[1], + cmd.Context(), args[0], args[1:], *flags, readToolboxFlags(cmd, extCtx), defaultConnectionResolver{}, @@ -52,7 +60,7 @@ instead).`, } func runConnectionRemove( - ctx context.Context, toolboxName, connName string, + ctx context.Context, toolboxName string, connNames []string, verb connectionRemoveFlags, parent toolboxFlags, resolver connectionResolver, ) error { @@ -62,13 +70,22 @@ func runConnectionRemove( if err := validateOutputFormat(parent.output); err != nil { return err } - if strings.TrimSpace(connName) == "" { + if len(connNames) == 0 { return exterrors.Validation( exterrors.CodeInvalidPositionalArg, - " must not be empty", - "pass the short name of a project connection", + "at least one must be provided", + "pass one or more connection short names", ) } + for _, n := range connNames { + if strings.TrimSpace(n) == "" { + return exterrors.Validation( + exterrors.CodeInvalidPositionalArg, + " must not be empty", + "remove empty entries from the argument list", + ) + } + } if parent.noPrompt && !verb.force { return exterrors.Validation( exterrors.CodeMissingForceFlag, @@ -84,47 +101,59 @@ func runConnectionRemove( logResolvedEndpoint("toolbox connection remove", resolved) return runConnectionRemoveWith(ctx, client, resolver, resolved.Endpoint, - toolboxName, connName, verb, parent) + toolboxName, connNames, verb, parent) } func runConnectionRemoveWith( ctx context.Context, client toolboxClient, resolver connectionResolver, - endpoint, toolboxName, connName string, + endpoint, toolboxName string, connNames []string, verb connectionRemoveFlags, parent toolboxFlags, ) error { - conn, err := resolver.resolveConnection(ctx, endpoint, connName) - if err != nil { - return err + // Normalize whitespace so callers that pass `" foo "` match the stored + // entry. Parity with `skill remove`. + names := make([]string, 0, len(connNames)) + for _, n := range connNames { + names = append(names, strings.TrimSpace(n)) } tb, err := client.GetToolbox(ctx, toolboxName) if err != nil { return toolboxNotFoundOrService(err, toolboxName, exterrors.OpGetToolbox) } - current, err := client.GetToolboxVersion(ctx, toolboxName, tb.DefaultVersion) if err != nil { return exterrors.ServiceFromAzure(err, exterrors.OpGetToolboxVersion) } - filtered, removed := filterOutConnection(current.Tools, conn.ID) - if !removed { - return exterrors.Validation( - exterrors.CodeConnectionNotInToolbox, - fmt.Sprintf( - "connection %q is not attached to toolbox %q's current default version", - connName, toolboxName, - ), - fmt.Sprintf("run 'azd ai toolbox connection list %q'", toolboxName), - ) + // Resolve each name and strip from the tools[]. + filtered := slices.Clone(current.Tools) + removedConns := make([]*projectConnection, 0, len(names)) + for _, name := range names { + conn, err := resolver.resolveConnection(ctx, endpoint, name) + if err != nil { + return err + } + var didRemove bool + filtered, didRemove = filterOutConnection(filtered, conn.ID) + if !didRemove { + return exterrors.Validation( + exterrors.CodeConnectionNotInToolbox, + fmt.Sprintf( + "connection %q is not attached to toolbox %q's current default version", + name, toolboxName, + ), + fmt.Sprintf("run 'azd ai toolbox connection list %q'", toolboxName), + ) + } + removedConns = append(removedConns, conn) } if len(filtered) == 0 { return exterrors.Validation( exterrors.CodeLastToolRemoval, fmt.Sprintf( - "removing %q would leave toolbox %q with zero tools", - connName, toolboxName, + "removing the listed connections would leave toolbox %q with zero tools", + toolboxName, ), fmt.Sprintf( "delete the toolbox with `azd ai toolbox delete %q` instead", @@ -135,14 +164,14 @@ func runConnectionRemoveWith( if !verb.force { shouldProceed := true + summary := summarizeConnectionNames(removedConns) err := withAzdClient(func(azdClient *azdext.AzdClient) error { confirmed, err := confirmToolboxDelete( ctx, azdClient, fmt.Sprintf( - "Detach connection %q from toolbox %q (publishes a new version)?", - connName, - toolboxName, + "Detach %s from toolbox %q (creates a new version)?", + summary, toolboxName, ), ) if err != nil { @@ -166,44 +195,70 @@ func runConnectionRemoveWith( Description: current.Description, Metadata: current.Metadata, Tools: filtered, + Skills: current.Skills, Policies: current.Policies, } created, err := client.CreateToolboxVersion(ctx, toolboxName, req) if err != nil { return exterrors.ServiceFromAzure(err, exterrors.OpCreateToolboxVersion) } - if _, err := client.SetDefaultVersion(ctx, toolboxName, created.Version); err != nil { - return exterrors.Dependency( - exterrors.CodeSetDefaultVersionFailed, - fmt.Sprintf( - "toolbox %q version %q was created but could not be promoted to default: %s", - toolboxName, created.Version, err, - ), - fmt.Sprintf( - "run `azd ai toolbox update %q --default-version %q` to retarget the default", - toolboxName, created.Version, - ), - ) - } - return emitConnectionRemoveResult(toolboxName, created.Version, conn, parent.output) + return emitConnectionRemoveResult(toolboxName, created.Version, removedConns, parent.output) +} + +// summarizeConnectionNames renders "connection \"a\"" or "connections [\"a\", \"b\"]". +func summarizeConnectionNames(conns []*projectConnection) string { + if len(conns) == 1 { + return fmt.Sprintf("connection %q", conns[0].Name) + } + quoted := make([]string, 0, len(conns)) + for _, c := range conns { + quoted = append(quoted, fmt.Sprintf("%q", c.Name)) + } + return "connections [" + strings.Join(quoted, ", ") + "]" } func emitConnectionRemoveResult( - toolboxName, newVersion string, conn *projectConnection, output string, + toolboxName, newVersion string, conns []*projectConnection, output string, ) error { if output == "json" { - payload := map[string]any{ - "toolbox": toolboxName, - "version": newVersion, - "connection": conn.Name, - "connection_id": conn.ID, + if len(conns) == 1 { + return emitJSON(map[string]any{ + "toolbox": toolboxName, + "version": newVersion, + "connection": conns[0].Name, + "connection_id": conns[0].ID, + }) + } + rows := make([]map[string]string, 0, len(conns)) + for _, c := range conns { + rows = append(rows, map[string]string{ + "connection": c.Name, + "connection_id": c.ID, + }) } - return emitJSON(payload) + return emitJSON(map[string]any{ + "toolbox": toolboxName, + "version": newVersion, + "connections": rows, + }) } - fmt.Printf( - "Detached connection %s from toolbox %s (now at version %s).\n", - conn.Name, toolboxName, newVersion, - ) + if len(conns) == 1 { + fmt.Printf( + "Created toolbox %s version %s (detached connection %s).\n", + toolboxName, newVersion, conns[0].Name, + ) + } else { + names := make([]string, 0, len(conns)) + for _, c := range conns { + names = append(names, c.Name) + } + fmt.Printf( + "Created toolbox %s version %s (detached connections [%s]).\n", + toolboxName, newVersion, strings.Join(names, ", "), + ) + } + fmt.Printf("The default version is unchanged; "+ + "run `azd ai toolbox publish %q %q` to promote.\n", toolboxName, newVersion) return nil } diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_create.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_create.go index 8f77b85aef9..76d0648d667 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_create.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_create.go @@ -29,8 +29,8 @@ func newToolboxCreateCommand(extCtx *azdext.ExtensionContext) *cobra.Command { cmd := &cobra.Command{ Use: "create --from-file ", - Short: "Create a toolbox and publish its initial version from a file.", - Long: `Create a toolbox and publish its initial version. + Short: "Create a toolbox and its initial version from a file.", + Long: `Create a toolbox and its initial version. The Foundry service requires the initial version to ship with at least one tool entry, so 'create' takes its inputs from a JSON or YAML file via @@ -105,7 +105,8 @@ func runToolboxCreateWith( return exterrors.Validation( exterrors.CodeInvalidToolboxName, fmt.Sprintf("toolbox %q already exists", name), - "run 'azd ai toolbox update' or 'connection add/remove' to change it", + "use 'connection add/remove' or 'skill add/remove' to create a new version, "+ + "then 'azd ai toolbox publish ' to promote it", ) } else if !isAzureNotFound(err) { return exterrors.ServiceFromAzure(err, exterrors.OpGetToolbox) @@ -113,6 +114,7 @@ func runToolboxCreateWith( description := "" entries := []map[string]any{} + skillEntries := []map[string]any{} var policies *azure.ToolboxPolicies if strings.TrimSpace(verb.fromFile) != "" { @@ -126,6 +128,15 @@ func runToolboxCreateWith( return err } entries = append(entries, resolvedEntries...) + for _, s := range input.Skills { + if err := validateSkillName(s.Name); err != nil { + return err + } + skillEntries = append(skillEntries, buildSkillEntry(skillSpec{ + Name: strings.TrimSpace(s.Name), + Version: strings.TrimSpace(s.Version), + })) + } rawEntries, err := validateRawToolEntries(input.Tools) if err != nil { @@ -150,6 +161,9 @@ func runToolboxCreateWith( if err := validateNoDuplicateConnectionIDs(entries); err != nil { return err } + if err := validateNoDuplicateSkills(skillEntries); err != nil { + return err + } if err := validateNoDuplicateToolNames(entries); err != nil { return err } @@ -157,6 +171,7 @@ func runToolboxCreateWith( req := &azure.CreateToolboxVersionRequest{ Description: description, Tools: entries, + Skills: skillEntries, Policies: policies, } created, err := client.CreateToolboxVersion(ctx, name, req) diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_delete.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_delete.go index 3b07df9fbc4..783859a038e 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_delete.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_delete.go @@ -156,7 +156,7 @@ func runDeleteToolboxVersion( "version %q is the default for toolbox %q and other versions exist", verb.version, name, ), - "retarget the default with `azd ai toolbox update --default-version ` first", + "retarget the default with `azd ai toolbox publish ` first", ) } diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_files.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_files.go index bd9d982b7c8..8668a477b3a 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_files.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_files.go @@ -25,16 +25,25 @@ type toolboxConnectionSpec struct { InstanceName string `json:"instance_name,omitempty" yaml:"instance_name,omitempty"` } +// toolboxSkillSpec is one skill reference input for the file shape. Empty +// Version means "use the skill's default version". +type toolboxSkillSpec struct { + Name string `json:"name" yaml:"name"` + Version string `json:"version,omitempty" yaml:"version,omitempty"` +} + // toolboxToolsFile is the file shape for `toolbox connection add --from-file`. -// -// Each connections[] item resolves through the project's connections -// data-plane and is converted into a service tool entry. The toolbox's -// existing description and metadata are carried forward; the file does not -// accept `description` (set at create time only in v1). +// Description and skills are not accepted here; use `skill add`/`skill remove` +// to change skills, and set description at create time. type toolboxToolsFile struct { Connections []toolboxConnectionSpec `json:"connections,omitempty" yaml:"connections,omitempty"` } +// toolboxSkillsFile is the file shape for `toolbox skill add --from-file`. +type toolboxSkillsFile struct { + Skills []toolboxSkillSpec `json:"skills,omitempty" yaml:"skills,omitempty"` +} + // toolboxCreateFile is the file shape for `toolbox create --from-file`. // // connections[] is azd sugar over the project connections data-plane and @@ -46,6 +55,7 @@ type toolboxToolsFile struct { type toolboxCreateFile struct { Description string `json:"description,omitempty" yaml:"description,omitempty"` Connections []toolboxConnectionSpec `json:"connections,omitempty" yaml:"connections,omitempty"` + Skills []toolboxSkillSpec `json:"skills,omitempty" yaml:"skills,omitempty"` Tools []map[string]any `json:"tools,omitempty" yaml:"tools,omitempty"` Policies *toolboxPoliciesSpec `json:"policies,omitempty" yaml:"policies,omitempty"` } @@ -126,16 +136,29 @@ func parseToolboxFile(path string, out any) error { } } -// suggestionForParseError returns a context-aware fix-it hint. The common -// surprise is putting `description` in a `connection add` file (the field -// only applies to `create`); call that out explicitly so the user does not -// have to read the file-shape doc to know why their description was rejected. +// suggestionForParseError returns a context-aware fix-it hint for common +// shape mistakes (e.g. putting `description` or `skills` in a `connection add` +// file). func suggestionForParseError(out any, err error) string { msg := err.Error() - if _, ok := out.(*toolboxToolsFile); ok && strings.Contains(msg, "description") { - return "the 'description' field is only accepted by `toolbox create`; " + - "in v1 a toolbox's description is set at create time and cannot be changed later" + if _, ok := out.(*toolboxToolsFile); ok { + switch { + case strings.Contains(msg, "description"): + return "the 'description' field is only accepted by `toolbox create`; " + + "a toolbox's description is set at create time and cannot be changed later" + case strings.Contains(msg, "skills"): + return "the 'skills' field belongs in a skills file; " + + "use `azd ai toolbox skill add --from-file` instead" + } + } + if _, ok := out.(*toolboxSkillsFile); ok { + switch { + case strings.Contains(msg, "connections"): + return "the 'connections' field belongs in a connections file; " + + "use `azd ai toolbox connection add --from-file` instead" + case strings.Contains(msg, "description"): + return "the 'description' field is only accepted by `toolbox create`" + } } - return "fix the file and retry; see `azd ai toolbox create --help` " + - "or `azd ai toolbox connection add --help` for the supported file shape" + return "fix the file and retry; see the verb's --help for the supported file shape" } diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_help.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_help.go index 4dd3539b2a0..70ff8e990b4 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_help.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_help.go @@ -21,6 +21,10 @@ func fileShapeBlurb(includeDescription bool) string { { "name": "my-bing", "instance_name": "docs-config" }, { "name": "my-a2a" } ], + "skills": [ + { "name": "my-skill", "version": "2" }, + { "name": "qa-skill" } + ], "tools": [ { "type": "web_search", "name": "web" }, { "type": "file_search", "name": "files" } @@ -40,6 +44,10 @@ Equivalent YAML: - name: my-bing instance_name: docs-config - name: my-a2a + skills: + - name: my-skill + version: "2" + - name: qa-skill tools: - type: web_search name: web @@ -59,6 +67,9 @@ Fields: Supported connection categories: RemoteTool (MCP), CognitiveSearch (Azure AI Search), RemoteA2A, GroundingWithCustomSearch. + skills Optional. Existing project skills to attach by reference. + Each entry needs 'name'; 'version' is optional (omit to + follow the skill's default version). tools List of raw Foundry tool entries (OpenAI.Tool shape), forwarded verbatim. Use for connectionless tools (e.g., built-in web_search, file_search, code_interpreter, @@ -70,7 +81,7 @@ Fields: AI content-filter policy applied to this toolbox version (the alias 'name' is also accepted). -At least one of 'connections' or 'tools' must be non-empty. +At least one of 'connections', 'skills', or 'tools' must be non-empty. Project connections must already exist on the Foundry project; this command does not create them. Run 'azd ai agent connection list' to see available @@ -108,8 +119,8 @@ Fields: CognitiveSearch (Azure AI Search), RemoteA2A, GroundingWithCustomSearch. -The toolbox's existing description is carried forward unchanged; use -'azd ai toolbox update' to change it. +The toolbox's existing description is carried forward unchanged; the +description is set at create time and cannot be changed later. Project connections must already exist on the Foundry project; this command does not create them. Run 'azd ai agent connection list' to see available diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_publish.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_publish.go new file mode 100644 index 00000000000..36c1a9d48c0 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_publish.go @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "fmt" + "strings" + + "azure.ai.toolboxes/internal/exterrors" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/spf13/cobra" +) + +// newToolboxPublishCommand returns the `azd ai toolbox publish ` command. +func newToolboxPublishCommand(extCtx *azdext.ExtensionContext) *cobra.Command { + extCtx = ensureExtensionContext(extCtx) + + cmd := &cobra.Command{ + Use: "publish ", + Short: "Set the default version for a toolbox.", + Long: `Set the default version for a toolbox. + +This promotes a previously created version so that consumers referencing the +toolbox without an explicit version will receive it. To create a new version, +use 'connection add', 'connection remove', 'skill add', or 'skill remove'.`, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + return runToolboxPublish(cmd.Context(), args[0], args[1], readToolboxFlags(cmd, extCtx)) + }, + } + + registerToolboxOutputFlag(cmd) + + return cmd +} + +func runToolboxPublish( + ctx context.Context, name string, version string, parent toolboxFlags, +) error { + if err := validateToolboxName(name); err != nil { + return err + } + if err := validateOutputFormat(parent.output); err != nil { + return err + } + + if strings.TrimSpace(version) == "" { + return exterrors.Validation( + exterrors.CodeMissingUpdateField, + "version must not be empty", + "pass the version to promote as the second positional argument", + ) + } + + client, resolved, err := resolveToolboxAndClient(ctx, parent) + if err != nil { + return err + } + logResolvedEndpoint("toolbox publish", resolved) + + result, err := client.SetDefaultVersion(ctx, name, version) + if err != nil { + return toolboxNotFoundOrService(err, name, exterrors.OpSetDefaultVersion) + } + + if parent.output == "json" { + return emitJSON(result) + } + fmt.Printf("Toolbox %s default version set to %s.\n", name, result.DefaultVersion) + return nil +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_show.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_show.go index 01ac0100101..a39e7463fbb 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_show.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_show.go @@ -142,6 +142,7 @@ func emitShowTable( fmt.Fprintf(w, "Description\t%s\n", version.Description) fmt.Fprintf(w, "Endpoint\t%s\n", mcpURL) fmt.Fprintf(w, "Tools\t%d\n", len(version.Tools)) + fmt.Fprintf(w, "Skills\t%d\n", len(version.Skills)) if err := w.Flush(); err != nil { return err } @@ -161,6 +162,25 @@ func emitShowTable( return err } } + + if len(version.Skills) > 0 { + fmt.Println() + tw := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "SKILL\tVERSION\tTYPE") + fmt.Fprintln(tw, "-----\t-------\t----") + // Use extractSkillRows so malformed entries are skipped consistently + // with `skill list`. + for _, r := range extractSkillRows(version.Skills) { + ver := r["version"] + if ver == "" { + ver = "(default)" + } + fmt.Fprintf(tw, "%s\t%s\t%s\n", r["name"], ver, r["type"]) + } + if err := tw.Flush(); err != nil { + return err + } + } return nil } diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill.go new file mode 100644 index 00000000000..c44464b09d5 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill.go @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "fmt" + "regexp" + "slices" + "strings" + + "azure.ai.toolboxes/internal/exterrors" +) + +// skillNamePattern matches the SkillName scalar in the Foundry Skills spec: +// lowercase letters / digits / hyphens, must not start or end with a hyphen, +// max 64 chars. Duplicated from azure.ai.skills' validateSkillName because the +// extensions are separate Go modules; keep both in lockstep if the scalar +// changes. +var skillNamePattern = regexp.MustCompile(`^[a-z0-9]([a-z0-9\-]*[a-z0-9])?$`) + +const skillNameMaxLen = 64 + +// skillSpec is the parsed form of a positional skill argument or a skills[] +// file entry. Empty Version means "use the skill's default version" per the +// ToolboxSkillReference contract. +type skillSpec struct { + Name string + Version string +} + +// parseSkillFlag parses `` or `@`. Version is opaque and +// passed to the service verbatim. +func parseSkillFlag(s string) (skillSpec, error) { + trimmed := strings.TrimSpace(s) + if trimmed == "" { + return skillSpec{}, exterrors.Validation( + exterrors.CodeInvalidSkillSpec, + " must not be empty", + "pass a skill name as [@]", + ) + } + + name := trimmed + version := "" + if before, after, ok := strings.Cut(trimmed, "@"); ok { + name = strings.TrimSpace(before) + version = strings.TrimSpace(after) + if version == "" { + return skillSpec{}, exterrors.Validation( + exterrors.CodeInvalidSkillSpec, + fmt.Sprintf(" %q has an empty version after '@'", trimmed), + "either drop the trailing '@' to use the skill's default version, "+ + "or pass @", + ) + } + } + + if err := validateSkillName(name); err != nil { + return skillSpec{}, err + } + return skillSpec{Name: name, Version: version}, nil +} + +// validateSkillName enforces the SkillName regex + length cap. +func validateSkillName(name string) error { + trimmed := strings.TrimSpace(name) + if trimmed == "" { + return exterrors.Validation( + exterrors.CodeInvalidSkillName, + "skill name must not be empty", + "pass a non-empty skill name", + ) + } + if len(trimmed) > skillNameMaxLen || !skillNamePattern.MatchString(trimmed) { + return exterrors.Validation( + exterrors.CodeInvalidSkillName, + fmt.Sprintf("skill name %q is invalid", trimmed), + "use 1-64 lowercase letters, digits, and hyphens; "+ + "must not start or end with a hyphen", + ) + } + return nil +} + +// buildSkillEntry returns the wire map for a skill_reference entry. +func buildSkillEntry(spec skillSpec) map[string]any { + entry := map[string]any{ + "type": "skill_reference", + "name": spec.Name, + } + if spec.Version != "" { + entry["version"] = spec.Version + } + return entry +} + +// validateNoDuplicateSkills rejects two skills[] entries with the same name. +// The service may also reject this; the local check produces a sharper error. +func validateNoDuplicateSkills(entries []map[string]any) error { + names := make([]string, 0, len(entries)) + for _, e := range entries { + if n, ok := e["name"].(string); ok && n != "" { + names = append(names, n) + } + } + slices.Sort(names) + for i := 1; i < len(names); i++ { + if names[i] == names[i-1] { + return exterrors.Validation( + exterrors.CodeDuplicateSkill, + fmt.Sprintf("skill %q appears more than once in the input", names[i]), + "remove duplicate skills[] entries from the input file", + ) + } + } + return nil +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_add.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_add.go new file mode 100644 index 00000000000..6e8514f7217 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_add.go @@ -0,0 +1,264 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "fmt" + "slices" + "strings" + + "azure.ai.toolboxes/internal/exterrors" + "azure.ai.toolboxes/internal/pkg/azure" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/spf13/cobra" +) + +// skillAddFlags carries the verb-specific flags for `skill add`. +type skillAddFlags struct { + fromFile string +} + +// newToolboxSkillAddCommand returns the `skill add` command. +func newToolboxSkillAddCommand(extCtx *azdext.ExtensionContext) *cobra.Command { + extCtx = ensureExtensionContext(extCtx) + flags := &skillAddFlags{} + + cmd := &cobra.Command{ + Use: "add [skill[@version]]", + Short: "Attach one or more skill references to a toolbox.", + Long: `Attach one or more skill references to a toolbox. + +Pass a single skill as the positional, or many via --from-file. Either way +the invocation creates exactly one new toolbox version. The toolbox's +default version is unchanged; run +'azd ai toolbox publish ' to promote it. + +When the version is omitted, the reference resolves to the skill's default +version at read time. + +Examples: + + azd ai toolbox skill add research my-skill + azd ai toolbox skill add research my-skill@2 + azd ai toolbox skill add research --from-file ./skills.yaml +`, + Args: func(cmd *cobra.Command, args []string) error { + fromFile, _ := cmd.Flags().GetString("from-file") + if strings.TrimSpace(fromFile) != "" { + if len(args) != 1 { + return cobra.ExactArgs(1)(cmd, args) + } + return nil + } + if len(args) != 2 { + return cobra.RangeArgs(2, 2)(cmd, args) + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + rawSkill := "" + if len(args) > 1 { + rawSkill = args[1] + } + return runSkillAdd(cmd.Context(), args[0], rawSkill, *flags, readToolboxFlags(cmd, extCtx)) + }, + } + cmd.Flags().StringVar( + &flags.fromFile, "from-file", "", + "Path to a JSON/YAML file listing skills to attach (skills[] block).", + ) + registerToolboxOutputFlag(cmd) + return cmd +} + +func runSkillAdd( + ctx context.Context, toolboxName, rawSkill string, + verb skillAddFlags, parent toolboxFlags, +) error { + if err := validateToolboxName(toolboxName); err != nil { + return err + } + if err := validateOutputFormat(parent.output); err != nil { + return err + } + hasFile := strings.TrimSpace(verb.fromFile) != "" + hasPos := strings.TrimSpace(rawSkill) != "" + if hasFile && hasPos { + return exterrors.Validation( + exterrors.CodeInvalidPositionalArg, + "do not pass when --from-file is set", + "either pass a single skill positional or use --from-file", + ) + } + if !hasFile && !hasPos { + return exterrors.Validation( + exterrors.CodeInvalidPositionalArg, + " must not be empty", + "pass a skill name or use --from-file", + ) + } + + client, resolved, err := resolveToolboxAndClient(ctx, parent) + if err != nil { + return err + } + logResolvedEndpoint("toolbox skill add", resolved) + + return runSkillAddWith(ctx, client, toolboxName, rawSkill, verb, parent) +} + +// runSkillAddWith is the testable core. +func runSkillAddWith( + ctx context.Context, client toolboxClient, + toolboxName, rawSkill string, + verb skillAddFlags, parent toolboxFlags, +) error { + specs, err := collectSkillSpecs(rawSkill, verb) + if err != nil { + return err + } + + tb, err := client.GetToolbox(ctx, toolboxName) + if err != nil { + return toolboxNotFoundOrService(err, toolboxName, exterrors.OpGetToolbox) + } + current, err := client.GetToolboxVersion(ctx, toolboxName, tb.DefaultVersion) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpGetToolboxVersion) + } + + // Reject duplicates within the input and against the current default. + seen := map[string]struct{}{} + for _, sk := range current.Skills { + if n, ok := sk["name"].(string); ok && n != "" { + seen[n] = struct{}{} + } + } + for _, sp := range specs { + if _, dup := seen[sp.Name]; dup { + return exterrors.Validation( + exterrors.CodeSkillAlreadyAttached, + fmt.Sprintf( + "skill %q is already attached to toolbox %q's current default version "+ + "(or appears more than once in the input)", + sp.Name, toolboxName, + ), + fmt.Sprintf( + "remove the existing reference with `azd ai toolbox skill remove %q %q` first", + toolboxName, sp.Name, + ), + ) + } + seen[sp.Name] = struct{}{} + } + + newSkills := slices.Clone(current.Skills) + for _, sp := range specs { + newSkills = append(newSkills, buildSkillEntry(sp)) + } + + req := &azure.CreateToolboxVersionRequest{ + Description: current.Description, + Metadata: current.Metadata, + Tools: current.Tools, + Skills: newSkills, + } + created, err := client.CreateToolboxVersion(ctx, toolboxName, req) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpCreateToolboxVersion) + } + + return emitSkillAddResult(toolboxName, created.Version, specs, parent.output) +} + +// collectSkillSpecs picks the active input mode and returns the parsed list. +func collectSkillSpecs(rawSkill string, verb skillAddFlags) ([]skillSpec, error) { + if strings.TrimSpace(verb.fromFile) != "" { + var input toolboxSkillsFile + if err := parseToolboxFile(verb.fromFile, &input); err != nil { + return nil, err + } + if len(input.Skills) == 0 { + return nil, exterrors.Validation( + exterrors.CodeInvalidParameter, + "no skills to add", + "provide at least one skill in 'skills[]'", + ) + } + specs := make([]skillSpec, 0, len(input.Skills)) + for _, s := range input.Skills { + if err := validateSkillName(s.Name); err != nil { + return nil, err + } + specs = append(specs, skillSpec{ + Name: strings.TrimSpace(s.Name), + Version: strings.TrimSpace(s.Version), + }) + } + return specs, nil + } + sp, err := parseSkillFlag(rawSkill) + if err != nil { + return nil, err + } + return []skillSpec{sp}, nil +} + +func emitSkillAddResult(toolboxName, newVersion string, specs []skillSpec, output string) error { + if output == "json" { + if len(specs) == 1 { + payload := map[string]any{ + "toolbox": toolboxName, + "version": newVersion, + "skill": specs[0].Name, + } + if specs[0].Version != "" { + payload["skill_version"] = specs[0].Version + } + return emitJSON(payload) + } + rows := make([]map[string]any, 0, len(specs)) + for _, s := range specs { + row := map[string]any{"name": s.Name} + if s.Version != "" { + row["version"] = s.Version + } + rows = append(rows, row) + } + return emitJSON(map[string]any{ + "toolbox": toolboxName, + "version": newVersion, + "skills": rows, + }) + } + + if len(specs) == 1 { + pinned := "" + if specs[0].Version != "" { + pinned = "@" + specs[0].Version + } + fmt.Printf( + "Created toolbox %s version %s (attached skill %s%s).\n", + toolboxName, newVersion, specs[0].Name, pinned, + ) + } else { + names := make([]string, 0, len(specs)) + for _, s := range specs { + entry := s.Name + if s.Version != "" { + entry += "@" + s.Version + } + names = append(names, entry) + } + fmt.Printf( + "Created toolbox %s version %s (attached skills [%s]).\n", + toolboxName, newVersion, strings.Join(names, ", "), + ) + } + fmt.Printf("The default version is unchanged; "+ + "run `azd ai toolbox publish %q %q` to promote.\n", toolboxName, newVersion) + return nil +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_group.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_group.go new file mode 100644 index 00000000000..0e7d814dbd1 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_group.go @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/spf13/cobra" +) + +// newToolboxSkillCommand returns the `azd ai toolbox skill` parent. +func newToolboxSkillCommand(extCtx *azdext.ExtensionContext) *cobra.Command { + extCtx = ensureExtensionContext(extCtx) + cmd := &cobra.Command{ + Use: "skill", + Short: "Manage skill references attached to a toolbox.", + Long: `Manage skill references attached to a toolbox. + +Each add/remove creates a new immutable version; the toolbox's default +version is unchanged. Use 'azd ai toolbox publish ' +to promote a version.`, + } + cmd.AddCommand(newToolboxSkillAddCommand(extCtx)) + cmd.AddCommand(newToolboxSkillRemoveCommand(extCtx)) + cmd.AddCommand(newToolboxSkillListCommand(extCtx)) + return cmd +} + +// findSkillEntry returns the index of the first entry in skills[] whose name +// matches, or -1 if absent. +func findSkillEntry(skills []map[string]any, name string) int { + for i, s := range skills { + if n, ok := s["name"].(string); ok && n == name { + return i + } + } + return -1 +} + +// filterOutSkill returns skills[] with the first matching entry stripped. +func filterOutSkill(skills []map[string]any, name string) (result []map[string]any, removed bool) { + idx := findSkillEntry(skills, name) + if idx < 0 { + return skills, false + } + result = make([]map[string]any, 0, len(skills)-1) + result = append(result, skills[:idx]...) + result = append(result, skills[idx+1:]...) + return result, true +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_list.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_list.go new file mode 100644 index 00000000000..e234cb91d30 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_list.go @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "fmt" + "os" + "text/tabwriter" + + "azure.ai.toolboxes/internal/exterrors" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/spf13/cobra" +) + +// newToolboxSkillListCommand returns the `skill list` command. +func newToolboxSkillListCommand(extCtx *azdext.ExtensionContext) *cobra.Command { + extCtx = ensureExtensionContext(extCtx) + + cmd := &cobra.Command{ + Use: "list ", + Short: "List the skill references attached to a toolbox.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return runSkillList(cmd.Context(), args[0], readToolboxFlags(cmd, extCtx)) + }, + } + registerToolboxOutputFlag(cmd) + return cmd +} + +func runSkillList(ctx context.Context, toolboxName string, parent toolboxFlags) error { + if err := validateToolboxName(toolboxName); err != nil { + return err + } + if err := validateOutputFormat(parent.output); err != nil { + return err + } + + client, resolved, err := resolveToolboxAndClient(ctx, parent) + if err != nil { + return err + } + logResolvedEndpoint("toolbox skill list", resolved) + + return runSkillListWith(ctx, client, toolboxName, parent) +} + +func runSkillListWith( + ctx context.Context, client toolboxClient, toolboxName string, parent toolboxFlags, +) error { + tb, err := client.GetToolbox(ctx, toolboxName) + if err != nil { + return toolboxNotFoundOrService(err, toolboxName, exterrors.OpGetToolbox) + } + version, err := client.GetToolboxVersion(ctx, toolboxName, tb.DefaultVersion) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpGetToolboxVersion) + } + + rows := extractSkillRows(version.Skills) + + if parent.output == "json" { + return emitJSON(map[string]any{"skills": rows}) + } + + w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(w, "NAME\tVERSION\tTYPE") + fmt.Fprintln(w, "----\t-------\t----") + for _, r := range rows { + ver := r["version"] + if ver == "" { + ver = "(default)" + } + fmt.Fprintf(w, "%s\t%s\t%s\n", r["name"], ver, r["type"]) + } + return w.Flush() +} + +// extractSkillRows reduces ToolboxSkill discriminator maps to the fields +// surfaced in `skill list` output. Empty version renders as "(default)" in +// table mode. +func extractSkillRows(skills []map[string]any) []map[string]string { + rows := make([]map[string]string, 0, len(skills)) + for _, s := range skills { + name, _ := s["name"].(string) + if name == "" { + continue + } + skType, _ := s["type"].(string) + ver, _ := s["version"].(string) + rows = append(rows, map[string]string{ + "name": name, + "version": ver, + "type": skType, + }) + } + return rows +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_remove.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_remove.go new file mode 100644 index 00000000000..4ff881f2750 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_remove.go @@ -0,0 +1,219 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "fmt" + "slices" + "strings" + + "azure.ai.toolboxes/internal/exterrors" + "azure.ai.toolboxes/internal/pkg/azure" + + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/spf13/cobra" +) + +// skillRemoveFlags carries the verb-specific flags for `skill remove`. +type skillRemoveFlags struct { + force bool +} + +// newToolboxSkillRemoveCommand returns the `skill remove` command. +func newToolboxSkillRemoveCommand(extCtx *azdext.ExtensionContext) *cobra.Command { + extCtx = ensureExtensionContext(extCtx) + flags := &skillRemoveFlags{} + + cmd := &cobra.Command{ + Use: "remove ...", + Short: "Detach one or more skill references from a toolbox.", + Long: `Detach one or more skill references from a toolbox and create a new version. + +Pass one or more skill short names as positionals. All removals are applied +atomically: each invocation creates exactly one new toolbox version. + +Removing the last skill is allowed. + +Examples: + + azd ai toolbox skill remove research my-skill + azd ai toolbox skill remove research a b c --force +`, + Args: cobra.MinimumNArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + return runSkillRemove( + cmd.Context(), args[0], args[1:], *flags, readToolboxFlags(cmd, extCtx), + ) + }, + } + cmd.Flags().BoolVar( + &flags.force, "force", false, + "Skip confirmation prompts and apply the removal immediately.", + ) + registerToolboxOutputFlag(cmd) + return cmd +} + +func runSkillRemove( + ctx context.Context, toolboxName string, skillNames []string, + verb skillRemoveFlags, parent toolboxFlags, +) error { + if err := validateToolboxName(toolboxName); err != nil { + return err + } + if err := validateOutputFormat(parent.output); err != nil { + return err + } + if len(skillNames) == 0 { + return exterrors.Validation( + exterrors.CodeInvalidPositionalArg, + "at least one must be provided", + "pass one or more skill short names", + ) + } + for _, n := range skillNames { + if err := validateSkillName(n); err != nil { + return err + } + } + if parent.noPrompt && !verb.force { + return exterrors.Validation( + exterrors.CodeMissingForceFlag, + "--no-prompt requires --force for skill removal", + "add --force to confirm the operation non-interactively", + ) + } + + client, resolved, err := resolveToolboxAndClient(ctx, parent) + if err != nil { + return err + } + logResolvedEndpoint("toolbox skill remove", resolved) + + return runSkillRemoveWith(ctx, client, toolboxName, skillNames, verb, parent) +} + +// runSkillRemoveWith is the testable core. +func runSkillRemoveWith( + ctx context.Context, client toolboxClient, + toolboxName string, skillNames []string, + verb skillRemoveFlags, parent toolboxFlags, +) error { + // Normalize whitespace so `" beta "` matches the stored entry. + names := make([]string, 0, len(skillNames)) + for _, n := range skillNames { + names = append(names, strings.TrimSpace(n)) + } + + tb, err := client.GetToolbox(ctx, toolboxName) + if err != nil { + return toolboxNotFoundOrService(err, toolboxName, exterrors.OpGetToolbox) + } + current, err := client.GetToolboxVersion(ctx, toolboxName, tb.DefaultVersion) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpGetToolboxVersion) + } + + filtered := slices.Clone(current.Skills) + for _, name := range names { + var didRemove bool + filtered, didRemove = filterOutSkill(filtered, name) + if !didRemove { + return exterrors.Validation( + exterrors.CodeSkillNotInToolbox, + fmt.Sprintf( + "skill %q is not attached to toolbox %q's current default version", + name, toolboxName, + ), + fmt.Sprintf("run 'azd ai toolbox skill list %q'", toolboxName), + ) + } + } + + if !verb.force { + shouldProceed := true + summary := summarizeSkillNames(names) + err := withAzdClient(func(azdClient *azdext.AzdClient) error { + confirmed, err := confirmToolboxDelete( + ctx, + azdClient, + fmt.Sprintf( + "Detach %s from toolbox %q (creates a new version)?", + summary, toolboxName, + ), + ) + if err != nil { + return err + } + if !confirmed { + shouldProceed = false + fmt.Println("Aborted.") + } + return nil + }) + if err != nil { + return err + } + if !shouldProceed { + return nil + } + } + + req := &azure.CreateToolboxVersionRequest{ + Description: current.Description, + Metadata: current.Metadata, + Tools: current.Tools, + Skills: filtered, + } + created, err := client.CreateToolboxVersion(ctx, toolboxName, req) + if err != nil { + return exterrors.ServiceFromAzure(err, exterrors.OpCreateToolboxVersion) + } + + return emitSkillRemoveResult(toolboxName, created.Version, names, parent.output) +} + +// summarizeSkillNames renders "skill \"a\"" or "skills [\"a\", \"b\"]". +func summarizeSkillNames(names []string) string { + if len(names) == 1 { + return fmt.Sprintf("skill %q", names[0]) + } + quoted := make([]string, 0, len(names)) + for _, n := range names { + quoted = append(quoted, fmt.Sprintf("%q", n)) + } + return "skills [" + strings.Join(quoted, ", ") + "]" +} + +func emitSkillRemoveResult(toolboxName, newVersion string, names []string, output string) error { + if output == "json" { + if len(names) == 1 { + return emitJSON(map[string]any{ + "toolbox": toolboxName, + "version": newVersion, + "skill": names[0], + }) + } + return emitJSON(map[string]any{ + "toolbox": toolboxName, + "version": newVersion, + "skills": names, + }) + } + if len(names) == 1 { + fmt.Printf( + "Created toolbox %s version %s (detached skill %s).\n", + toolboxName, newVersion, names[0], + ) + } else { + fmt.Printf( + "Created toolbox %s version %s (detached skills [%s]).\n", + toolboxName, newVersion, strings.Join(names, ", "), + ) + } + fmt.Printf("The default version is unchanged; "+ + "run `azd ai toolbox publish %q %q` to promote.\n", toolboxName, newVersion) + return nil +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_test.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_test.go new file mode 100644 index 00000000000..19ae9feb348 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_test.go @@ -0,0 +1,159 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "strings" + "testing" + + "azure.ai.toolboxes/internal/exterrors" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseSkillFlag(t *testing.T) { + t.Run("bare name", func(t *testing.T) { + spec, err := parseSkillFlag("my-skill") + require.NoError(t, err) + assert.Equal(t, "my-skill", spec.Name) + assert.Empty(t, spec.Version) + }) + + t.Run("name with version", func(t *testing.T) { + spec, err := parseSkillFlag("my-skill@2") + require.NoError(t, err) + assert.Equal(t, "my-skill", spec.Name) + assert.Equal(t, "2", spec.Version) + }) + + t.Run("version with whitespace trimmed", func(t *testing.T) { + spec, err := parseSkillFlag(" qa-skill@ v1.0.0 ") + require.NoError(t, err) + assert.Equal(t, "qa-skill", spec.Name) + assert.Equal(t, "v1.0.0", spec.Version) + }) + + t.Run("name with inner whitespace before @ trimmed", func(t *testing.T) { + // Regression: parseSkillFlag must not store a trailing space on Name + // after splitting on '@'. Otherwise the wire entry won't match + // duplicate / remove lookups later. + spec, err := parseSkillFlag("my-skill @2") + require.NoError(t, err) + assert.Equal(t, "my-skill", spec.Name) + assert.Equal(t, "2", spec.Version) + }) + + t.Run("empty rejected", func(t *testing.T) { + _, err := parseSkillFlag("") + requireLocalError(t, err, exterrors.CodeInvalidSkillSpec) + }) + + t.Run("whitespace-only rejected", func(t *testing.T) { + _, err := parseSkillFlag(" ") + requireLocalError(t, err, exterrors.CodeInvalidSkillSpec) + }) + + t.Run("trailing @ rejected", func(t *testing.T) { + _, err := parseSkillFlag("my-skill@") + requireLocalError(t, err, exterrors.CodeInvalidSkillSpec) + }) + + t.Run("trailing @whitespace rejected", func(t *testing.T) { + _, err := parseSkillFlag("my-skill@ ") + requireLocalError(t, err, exterrors.CodeInvalidSkillSpec) + }) + + t.Run("uppercase name rejected", func(t *testing.T) { + _, err := parseSkillFlag("MySkill") + requireLocalError(t, err, exterrors.CodeInvalidSkillName) + }) + + t.Run("leading hyphen rejected", func(t *testing.T) { + _, err := parseSkillFlag("-skill") + requireLocalError(t, err, exterrors.CodeInvalidSkillName) + }) + + t.Run("trailing hyphen rejected", func(t *testing.T) { + _, err := parseSkillFlag("skill-") + requireLocalError(t, err, exterrors.CodeInvalidSkillName) + }) + + t.Run("underscore rejected", func(t *testing.T) { + _, err := parseSkillFlag("my_skill") + requireLocalError(t, err, exterrors.CodeInvalidSkillName) + }) + + t.Run("over 64 chars rejected", func(t *testing.T) { + var long strings.Builder + for range 65 { + long.WriteString("a") + } + _, err := parseSkillFlag(long.String()) + requireLocalError(t, err, exterrors.CodeInvalidSkillName) + }) + + t.Run("exactly 64 chars accepted", func(t *testing.T) { + var long strings.Builder + for range 64 { + long.WriteString("a") + } + spec, err := parseSkillFlag(long.String()) + require.NoError(t, err) + assert.Equal(t, long.String(), spec.Name) + }) +} + +func TestBuildSkillEntry(t *testing.T) { + t.Run("with version", func(t *testing.T) { + entry := buildSkillEntry(skillSpec{Name: "my-skill", Version: "2"}) + assert.Equal(t, "skill_reference", entry["type"]) + assert.Equal(t, "my-skill", entry["name"]) + assert.Equal(t, "2", entry["version"]) + }) + + t.Run("without version omits version key", func(t *testing.T) { + entry := buildSkillEntry(skillSpec{Name: "my-skill"}) + assert.Equal(t, "skill_reference", entry["type"]) + assert.Equal(t, "my-skill", entry["name"]) + _, hasVersion := entry["version"] + assert.False(t, hasVersion, "version key must be omitted when empty") + }) +} + +func TestValidateNoDuplicateSkills(t *testing.T) { + t.Run("unique names pass", func(t *testing.T) { + err := validateNoDuplicateSkills([]map[string]any{ + {"type": "skill_reference", "name": "a"}, + {"type": "skill_reference", "name": "b"}, + {"type": "skill_reference", "name": "c"}, + }) + require.NoError(t, err) + }) + + t.Run("duplicate names rejected", func(t *testing.T) { + err := validateNoDuplicateSkills([]map[string]any{ + {"type": "skill_reference", "name": "dup"}, + {"type": "skill_reference", "name": "other"}, + {"type": "skill_reference", "name": "dup"}, + }) + le := requireLocalError(t, err, exterrors.CodeDuplicateSkill) + assert.Contains(t, le.Message, "dup") + }) + + t.Run("duplicates differ in version still rejected", func(t *testing.T) { + // Pinning the same skill to two different versions is also a duplicate + // for our purposes; the service is single-row-per-name. + err := validateNoDuplicateSkills([]map[string]any{ + {"type": "skill_reference", "name": "x", "version": "1"}, + {"type": "skill_reference", "name": "x", "version": "2"}, + }) + requireLocalError(t, err, exterrors.CodeDuplicateSkill) + }) + + t.Run("empty list accepted", func(t *testing.T) { + require.NoError(t, validateNoDuplicateSkills(nil)) + require.NoError(t, validateNoDuplicateSkills([]map[string]any{})) + }) +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_verbs_test.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_verbs_test.go new file mode 100644 index 00000000000..f6170ce4cd2 --- /dev/null +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_skill_verbs_test.go @@ -0,0 +1,336 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "os" + "testing" + + "azure.ai.toolboxes/internal/exterrors" + "azure.ai.toolboxes/internal/pkg/azure" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindSkillEntry(t *testing.T) { + skills := []map[string]any{ + {"type": "skill_reference", "name": "alpha"}, + {"type": "skill_reference", "name": "beta", "version": "2"}, + {"type": "skill_reference", "name": "gamma"}, + } + assert.Equal(t, 0, findSkillEntry(skills, "alpha")) + assert.Equal(t, 1, findSkillEntry(skills, "beta")) + assert.Equal(t, 2, findSkillEntry(skills, "gamma")) + assert.Equal(t, -1, findSkillEntry(skills, "delta")) + assert.Equal(t, -1, findSkillEntry(nil, "any")) +} + +func TestFilterOutSkill(t *testing.T) { + skills := []map[string]any{ + {"type": "skill_reference", "name": "alpha"}, + {"type": "skill_reference", "name": "beta", "version": "2"}, + {"type": "skill_reference", "name": "gamma"}, + } + + got, removed := filterOutSkill(skills, "beta") + require.True(t, removed) + require.Len(t, got, 2) + assert.Equal(t, "alpha", got[0]["name"]) + assert.Equal(t, "gamma", got[1]["name"]) + + got2, removed2 := filterOutSkill(skills, "missing") + assert.False(t, removed2) + assert.Len(t, got2, 3, "unmodified slice returned when name not found") + + // Removing the only entry returns an empty (not nil) slice — exercises the + // "removing last skill is OK" semantic. + single := []map[string]any{{"type": "skill_reference", "name": "only"}} + got3, removed3 := filterOutSkill(single, "only") + assert.True(t, removed3) + assert.Empty(t, got3) +} + +func TestRunSkillAddWith_AppendsAndCarriesForward(t *testing.T) { + existingTools := []map[string]any{ + {"type": "mcp", "name": "a", "project_connection_id": "/c/a"}, + } + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", Description: "first", + Tools: existingTools, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "already-there"}, + }, + }} + + err := runSkillAddWith(t.Context(), client, "tb", "new-skill@3", skillAddFlags{}, toolboxFlags{output: "json"}) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + + req := client.createVersionCalls[0].req + assert.Equal(t, "first", req.Description, "description carried forward") + assert.Equal(t, existingTools, req.Tools, "tools carried forward verbatim") + + require.Len(t, req.Skills, 2, "existing skill + new skill") + assert.Equal(t, "already-there", req.Skills[0]["name"]) + assert.Equal(t, "new-skill", req.Skills[1]["name"]) + assert.Equal(t, "3", req.Skills[1]["version"]) + assert.Equal(t, "skill_reference", req.Skills[1]["type"]) + + assert.Empty(t, client.setDefaultCalls, "mutation verbs no longer auto-promote default") +} + +func TestRunSkillAddWith_NoExistingSkills(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{ + {"type": "mcp", "name": "a", "project_connection_id": "/c/a"}, + }, + // Skills nil — exercises the "first skill on a toolbox without any" path. + }} + + err := runSkillAddWith(t.Context(), client, "tb", "first-skill", skillAddFlags{}, toolboxFlags{output: "json"}) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + require.Len(t, client.createVersionCalls[0].req.Skills, 1) + assert.Equal(t, "first-skill", client.createVersionCalls[0].req.Skills[0]["name"]) + _, hasVersion := client.createVersionCalls[0].req.Skills[0]["version"] + assert.False(t, hasVersion, "version key must be omitted when @ is not provided") +} + +func TestRunSkillAddWith_AlreadyAttached(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a", "project_connection_id": "/c/a"}}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "dup"}, + }, + }} + + err := runSkillAddWith(t.Context(), client, "tb", "dup@2", skillAddFlags{}, toolboxFlags{output: "json"}) + requireLocalError(t, err, exterrors.CodeSkillAlreadyAttached) + assert.Empty(t, client.createVersionCalls, "no version should be created when validation fails") +} + +func TestRunSkillAddWith_InvalidSpec(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a"}}, + }} + + err := runSkillAddWith(t.Context(), client, "tb", "BadName@", skillAddFlags{}, toolboxFlags{output: "json"}) + requireLocalError(t, err, exterrors.CodeInvalidSkillSpec) +} + +func TestRunSkillRemoveWith_FilteredAndPromoted(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a", "project_connection_id": "/c/a"}}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "keep"}, + {"type": "skill_reference", "name": "drop"}, + }, + }} + + err := runSkillRemoveWith(t.Context(), client, "tb", []string{"drop"}, + skillRemoveFlags{force: true}, toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + skills := client.createVersionCalls[0].req.Skills + require.Len(t, skills, 1) + assert.Equal(t, "keep", skills[0]["name"]) + assert.Empty(t, client.setDefaultCalls) +} + +// Removing the only skill is allowed (no last-skill block). +func TestRunSkillRemoveWith_LastSkillAllowed(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a", "project_connection_id": "/c/a"}}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "only"}, + }, + }} + + err := runSkillRemoveWith(t.Context(), client, "tb", []string{"only"}, + skillRemoveFlags{force: true}, toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + assert.Empty(t, client.createVersionCalls[0].req.Skills, "removing the last skill is allowed") +} + +// Regression: skillName with surrounding whitespace must match the stored +// canonical entry rather than producing a misleading "not in toolbox" error. +func TestRunSkillRemoveWith_TrimsSkillName(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a", "project_connection_id": "/c/a"}}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "beta"}, + }, + }} + + err := runSkillRemoveWith(t.Context(), client, "tb", []string{" beta "}, + skillRemoveFlags{force: true}, toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + assert.Empty(t, client.createVersionCalls[0].req.Skills) +} + +func TestRunSkillRemoveWith_NotAttached(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a"}}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "other"}, + }, + }} + + err := runSkillRemoveWith(t.Context(), client, "tb", []string{"missing"}, + skillRemoveFlags{force: true}, toolboxFlags{output: "json"}, + ) + requireLocalError(t, err, exterrors.CodeSkillNotInToolbox) +} + +func TestRunSkillRemove_NoPromptWithoutForce(t *testing.T) { + err := runSkillRemove( + t.Context(), "tb", []string{"any-skill"}, + skillRemoveFlags{force: false}, + toolboxFlags{output: "table", noPrompt: true}, + ) + requireLocalError(t, err, exterrors.CodeMissingForceFlag) +} + +func TestRunSkillListWith_EmitsAllShapes(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "alpha", "version": "2"}, + {"type": "skill_reference", "name": "beta"}, + }, + }} + + rows := extractSkillRows(client.versionResults["tb/1"].obj.Skills) + require.Len(t, rows, 2) + assert.Equal(t, "alpha", rows[0]["name"]) + assert.Equal(t, "2", rows[0]["version"]) + assert.Equal(t, "skill_reference", rows[0]["type"]) + assert.Equal(t, "beta", rows[1]["name"]) + assert.Empty(t, rows[1]["version"], "empty version means 'use the skill's default'") + + err := runSkillListWith(t.Context(), client, "tb", toolboxFlags{output: "json"}) + require.NoError(t, err) +} + +// extractSkillRows must skip malformed entries (defensive against unexpected +// service responses). +func TestExtractSkillRows_SkipsMalformedEntries(t *testing.T) { + skills := []map[string]any{ + {"type": "skill_reference"}, // missing name + {"type": "skill_reference", "name": ""}, // empty name + {"type": "skill_reference", "name": "ok"}, // valid + {"type": "skill_reference", "name": 42}, // wrong type for name + } + rows := extractSkillRows(skills) + require.Len(t, rows, 1) + assert.Equal(t, "ok", rows[0]["name"]) +} + +// Batch removal via variadic positionals. +func TestRunSkillRemoveWith_VariadicPositionals(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a", "project_connection_id": "/c/a"}}, + Skills: []map[string]any{ + {"type": "skill_reference", "name": "alpha"}, + {"type": "skill_reference", "name": "beta"}, + {"type": "skill_reference", "name": "gamma"}, + }, + }} + + err := runSkillRemoveWith(t.Context(), client, "tb", []string{"alpha", "gamma"}, + skillRemoveFlags{force: true}, toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1, "one new version created for the whole batch") + require.Len(t, client.createVersionCalls[0].req.Skills, 1) + assert.Equal(t, "beta", client.createVersionCalls[0].req.Skills[0]["name"]) +} + +// Batch attachment via --from-file. +func TestRunSkillAddWith_FromFile(t *testing.T) { + client := newMockToolboxClient("https://e/") + client.getResults["tb"] = toolboxGetResult{obj: &azure.ToolboxObject{ + Name: "tb", DefaultVersion: "1", + }} + client.versionResults["tb/1"] = toolboxVersionResult{obj: &azure.ToolboxVersionObject{ + Name: "tb", Version: "1", + Tools: []map[string]any{{"type": "mcp", "name": "a"}}, + }} + + inputPath := t.TempDir() + "/skills.yaml" + require.NoError(t, os.WriteFile(inputPath, []byte(` +skills: + - name: alpha + - name: beta + version: "2" +`), 0o600)) + + err := runSkillAddWith(t.Context(), client, "tb", "", + skillAddFlags{fromFile: inputPath}, + toolboxFlags{output: "json"}, + ) + require.NoError(t, err) + require.Len(t, client.createVersionCalls, 1) + require.Len(t, client.createVersionCalls[0].req.Skills, 2) + names := []string{ + client.createVersionCalls[0].req.Skills[0]["name"].(string), + client.createVersionCalls[0].req.Skills[1]["name"].(string), + } + assert.ElementsMatch(t, []string{"alpha", "beta"}, names) +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_update.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_update.go deleted file mode 100644 index 6a30ebf6a41..00000000000 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_update.go +++ /dev/null @@ -1,84 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package cmd - -import ( - "context" - "fmt" - "strings" - - "azure.ai.toolboxes/internal/exterrors" - - "github.com/azure/azure-dev/cli/azd/pkg/azdext" - "github.com/spf13/cobra" -) - -// toolboxUpdateFlags carries the verb-specific flags for `toolbox update`. -type toolboxUpdateFlags struct { - defaultVersion string -} - -// newToolboxUpdateCommand returns the `azd ai toolbox update ` command. -// Only --default-version is supported. -func newToolboxUpdateCommand(extCtx *azdext.ExtensionContext) *cobra.Command { - extCtx = ensureExtensionContext(extCtx) - flags := &toolboxUpdateFlags{} - - cmd := &cobra.Command{ - Use: "update ", - Short: "Update a toolbox (currently: retarget the default version).", - Long: `Update a toolbox. - -Only --default-version is supported today. To change the tool list, publish a -new version with 'connection add' or 'connection remove'.`, - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - return runToolboxUpdate(cmd.Context(), args[0], *flags, readToolboxFlags(cmd, extCtx)) - }, - } - - cmd.Flags().StringVar( - &flags.defaultVersion, "default-version", "", - "Version string to mark as the default for this toolbox.", - ) - registerToolboxOutputFlag(cmd) - - return cmd -} - -func runToolboxUpdate( - ctx context.Context, name string, verb toolboxUpdateFlags, parent toolboxFlags, -) error { - if err := validateToolboxName(name); err != nil { - return err - } - if err := validateOutputFormat(parent.output); err != nil { - return err - } - - if strings.TrimSpace(verb.defaultVersion) == "" { - return exterrors.Validation( - exterrors.CodeMissingUpdateField, - "no fields to update", - "specify --default-version", - ) - } - - client, resolved, err := resolveToolboxAndClient(ctx, parent) - if err != nil { - return err - } - logResolvedEndpoint("toolbox update", resolved) - - result, err := client.SetDefaultVersion(ctx, name, verb.defaultVersion) - if err != nil { - return toolboxNotFoundOrService(err, name, exterrors.OpSetDefaultVersion) - } - - if parent.output == "json" { - return emitJSON(result) - } - fmt.Printf("Toolbox %s default version set to %s.\n", name, result.DefaultVersion) - return nil -} diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_version_list.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_version_list.go index 77be5d914c3..68af58cf641 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_version_list.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/toolbox_version_list.go @@ -30,7 +30,7 @@ func newToolboxVersionListCommand(extCtx *azdext.ExtensionContext) *cobra.Comman Long: `List published versions for a toolbox. Shows one row per published version and marks which one is currently the -default. Use this when choosing a target for 'toolbox update --default-version'.`, +default. Use this when choosing a target for 'toolbox publish'.`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runToolboxVersionList(cmd.Context(), args[0], readToolboxFlags(cmd, extCtx)) @@ -100,13 +100,14 @@ func emitToolboxVersionListJSON(name, defaultVersion string, versions []azure.To items := make([]map[string]any, 0, len(versions)) for _, v := range versions { items = append(items, map[string]any{ - "id": v.ID, - "name": v.Name, - "version": v.Version, - "description": v.Description, - "created_at": v.CreatedAt, - "tools_count": len(v.Tools), - "is_default": v.Version == defaultVersion, + "id": v.ID, + "name": v.Name, + "version": v.Version, + "description": v.Description, + "created_at": v.CreatedAt, + "tools_count": len(v.Tools), + "skills_count": len(v.Skills), + "is_default": v.Version == defaultVersion, }) } @@ -119,8 +120,8 @@ func emitToolboxVersionListJSON(name, defaultVersion string, versions []azure.To func emitToolboxVersionListTable(name, defaultVersion string, versions []azure.ToolboxVersionObject) error { w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) - fmt.Fprintln(w, "VERSION\tDEFAULT\tCREATED\tTOOLS\tDESCRIPTION") - fmt.Fprintln(w, "-------\t-------\t-------\t-----\t-----------") + fmt.Fprintln(w, "VERSION\tDEFAULT\tCREATED\tTOOLS\tSKILLS\tDESCRIPTION") + fmt.Fprintln(w, "-------\t-------\t-------\t-----\t------\t-----------") for _, v := range versions { marker := "" @@ -133,11 +134,12 @@ func emitToolboxVersionListTable(name, defaultVersion string, versions []azure.T } fmt.Fprintf( w, - "%s\t%s\t%s\t%d\t%s\n", + "%s\t%s\t%s\t%d\t%d\t%s\n", v.Version, marker, created, len(v.Tools), + len(v.Skills), v.Description, ) } diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/exterrors/codes.go b/cli/azd/extensions/azure.ai.toolboxes/internal/exterrors/codes.go index aea91ffeeca..b5901104ffd 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/exterrors/codes.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/exterrors/codes.go @@ -40,6 +40,11 @@ const ( CodeUnsupportedIndexFlag = "unsupported_index_flag" CodeMissingInstanceName = "missing_instance_name" CodeUnsupportedInstanceNameFlag = "unsupported_instance_name_flag" + CodeInvalidSkillName = "invalid_skill_name" + CodeInvalidSkillSpec = "invalid_skill_spec" + CodeDuplicateSkill = "duplicate_skill" + CodeSkillNotInToolbox = "skill_not_in_toolbox" + CodeSkillAlreadyAttached = "skill_already_attached" CodeDuplicateConnection = "duplicate_connection" CodeDuplicateToolName = "duplicate_tool_name" CodeMissingToolType = "missing_tool_type" @@ -48,7 +53,6 @@ const ( CodeConnectionMissingTarget = "connection_missing_target" CodeLastToolRemoval = "last_tool_removal" CodePendingToolboxStoreFailed = "pending_toolbox_store_failed" - CodeSetDefaultVersionFailed = "set_default_version_failed" ) // Operation names for [ServiceFromAzure] errors. diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/pkg/azure/foundry_toolsets_client.go b/cli/azd/extensions/azure.ai.toolboxes/internal/pkg/azure/foundry_toolsets_client.go index 597921fd486..127cb57261d 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/pkg/azure/foundry_toolsets_client.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/pkg/azure/foundry_toolsets_client.go @@ -188,7 +188,10 @@ type CreateToolboxVersionRequest struct { Description string `json:"description,omitempty"` Metadata map[string]string `json:"metadata,omitempty"` Tools []map[string]any `json:"tools"` - Policies *ToolboxPolicies `json:"policies,omitempty"` + // Skills holds ToolboxSkill discriminated objects. []map[string]any keeps + // future ToolboxSkill variants flowing through without recompiling. + Skills []map[string]any `json:"skills,omitempty"` + Policies *ToolboxPolicies `json:"policies,omitempty"` } // ToolboxPolicies mirrors the data-plane ToolboxPolicies model: per-version @@ -219,7 +222,9 @@ type ToolboxVersionObject struct { CreatedAt int64 `json:"created_at"` Metadata map[string]string `json:"metadata,omitempty"` Tools []map[string]any `json:"tools"` - Policies *ToolboxPolicies `json:"policies,omitempty"` + // Skills has no omitempty: the service always emits "skills":[] on reads. + Skills []map[string]any `json:"skills"` + Policies *ToolboxPolicies `json:"policies,omitempty"` } // toolboxURL builds the canonical toolboxes URL with the api-version query.