feat(skills): make description optional during skills gen (#3584)

## Description

Once description becomes optional, skills generation can take in
description directly from `Groups`.

Note: group description takes precedence when present, `--description`
is used otherwise, and both may be empty.

Based on [#3605](https://github.com/googleapis/mcp-toolbox/pull/3605)

---------

Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
This commit is contained in:
Twisha Bansal
2026-07-25 00:25:31 +05:30
committed by GitHub
parent e75ec3b5c8
commit d0a8f14cbe
4 changed files with 120 additions and 24 deletions
+41 -18
View File
@@ -32,6 +32,12 @@ import (
"github.com/spf13/cobra"
)
// skillContent holds the tools and description for a single generated skill.
type skillContent struct {
tools map[string]tools.Tool
description string
}
// skillsCmd is the command for generating skills.
type skillsCmd struct {
*cobra.Command
@@ -60,7 +66,7 @@ func NewCommand(opts *internal.ToolboxOptions) *cobra.Command {
flags := cmd.Flags()
internal.ConfigFileFlags(cmd.Command, flags, opts)
flags.StringVar(&cmd.name, "name", "", "Name of the generated skill.")
flags.StringVar(&cmd.description, "description", "", "Description of the generated skill")
flags.StringVar(&cmd.description, "description", "", "Description of the generated skill. Used as a fallback when a group does not define its own description.")
flags.StringVar(&cmd.toolset, "toolset", "", "Name of the toolset to convert into a skill. If not provided, all tools will be included.")
flags.StringVar(&cmd.outputDir, "output-dir", "skills", "Directory to output generated skills")
flags.StringVar(&cmd.licenseHeader, "license-header", "", "Optional license header to prepend to generated node scripts.")
@@ -68,7 +74,6 @@ func NewCommand(opts *internal.ToolboxOptions) *cobra.Command {
flags.StringVar(&cmd.invocationMode, "invocation-mode", "npx", "Invocation mode for the generated scripts: 'binary' or 'npx'")
flags.StringVar(&cmd.toolboxVersion, "toolbox-version", opts.VersionNum, "Version of @toolbox-sdk/server to use for npx approach")
_ = cmd.MarkFlagRequired("name")
_ = cmd.MarkFlagRequired("description")
return cmd.Command
}
@@ -100,28 +105,29 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error {
opts.Logger.InfoContext(ctx, "Generating skillagent skills...")
// Group the collected tools by toolset they belong to
skillsToTools, err := cmd.collectTools(ctx, opts)
// Collect the tools and description for each skill to generate.
skillsToContents, err := cmd.collectContents(ctx, opts)
if err != nil {
errMsg := fmt.Errorf("error collecting skill tools: %w", err)
errMsg := fmt.Errorf("error collecting skill contents: %w", err)
opts.Logger.ErrorContext(ctx, errMsg.Error())
return errMsg
}
if len(skillsToTools) == 0 {
if len(skillsToContents) == 0 {
opts.Logger.InfoContext(ctx, "No tools found to generate.")
return nil
}
// Iterate over keys to ensure deterministic order
var skillNames []string
for name := range skillsToTools {
for name := range skillsToContents {
skillNames = append(skillNames, name)
}
sort.Strings(skillNames)
for _, skillName := range skillNames {
allTools := skillsToTools[skillName]
content := skillsToContents[skillName]
allTools := content.tools
if len(allTools) == 0 {
opts.Logger.InfoContext(ctx, fmt.Sprintf("No tools found for skill '%s', skipping.", skillName))
continue
@@ -210,7 +216,7 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error {
}
// Generate SKILL.md
skillContent, err := generateSkillMarkdown(skillName, cmd.description, cmd.additionalNotes, allTools, parser.EnvVars)
skillContent, err := generateSkillMarkdown(skillName, content.description, cmd.additionalNotes, allTools, parser.EnvVars)
if err != nil {
errMsg := fmt.Errorf("error generating SKILL.md content: %w", err)
opts.Logger.ErrorContext(ctx, errMsg.Error())
@@ -229,7 +235,7 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error {
return nil
}
func (c *skillsCmd) collectTools(ctx context.Context, opts *internal.ToolboxOptions) (map[string]map[string]tools.Tool, error) {
func (c *skillsCmd) collectContents(ctx context.Context, opts *internal.ToolboxOptions) (map[string]skillContent, error) {
// Initialize tools and groups only; skills generation does not need live
// sources, auth services, or embedding models.
toolsMap, groupsMap, err := server.InitializeOfflineConfigs(ctx, opts.Cfg)
@@ -237,9 +243,16 @@ func (c *skillsCmd) collectTools(ctx context.Context, opts *internal.ToolboxOpti
return nil, fmt.Errorf("failed to initialize resources: %w", err)
}
return c.buildSkillContents(toolsMap, groupsMap)
}
// buildSkillContents maps each skill name to the tools and description it should
// be generated with. In group mode, a group's own description takes precedence
// over the --description flag, which acts as a fallback.
func (c *skillsCmd) buildSkillContents(toolsMap map[string]tools.Tool, groupsMap map[string]group.Group) (map[string]skillContent, error) {
primitiveMgr := primitives.NewPrimitiveManager(nil, nil, nil, toolsMap, nil, groupsMap)
skillsToTools := make(map[string]map[string]tools.Tool)
skillsToContents := make(map[string]skillContent)
getToolsFromGroup := func(g group.Group) map[string]tools.Tool {
groupTools := make(map[string]tools.Tool)
@@ -257,14 +270,15 @@ func (c *skillsCmd) collectTools(ctx context.Context, opts *internal.ToolboxOpti
return nil, fmt.Errorf("toolset %q not found", c.toolset)
}
skillsToTools[c.name] = getToolsFromGroup(g)
return skillsToTools, nil
skillsToContents[c.name] = skillContent{tools: getToolsFromGroup(g), description: c.description}
return skillsToContents, nil
}
if len(groupsMap) <= 1 {
// Default to all tools if no named group found
skillsToTools[c.name] = toolsMap
return skillsToTools, nil
// Default to all tools if no named group found. The default nameless
// group's description (if any) takes precedence over the flag.
skillsToContents[c.name] = skillContent{tools: toolsMap, description: c.descriptionFor(groupsMap[""])}
return skillsToContents, nil
}
// One skill per group
@@ -273,10 +287,19 @@ func (c *skillsCmd) collectTools(ctx context.Context, opts *internal.ToolboxOpti
continue
}
skillName := fmt.Sprintf("%s-%s", c.name, gName)
skillsToTools[skillName] = getToolsFromGroup(g)
skillsToContents[skillName] = skillContent{tools: getToolsFromGroup(g), description: c.descriptionFor(g)}
}
return skillsToTools, nil
return skillsToContents, nil
}
// descriptionFor returns the group's own description when set, falling back to
// the --description flag otherwise.
func (c *skillsCmd) descriptionFor(g group.Group) string {
if g.Description != "" {
return g.Description
}
return c.description
}
func copyFile(src, dst string) error {
+77 -4
View File
@@ -18,11 +18,14 @@ import (
"bytes"
"os"
"path/filepath"
"reflect"
"strings"
"testing"
"github.com/googleapis/mcp-toolbox/cmd/internal"
"github.com/googleapis/mcp-toolbox/internal/group"
_ "github.com/googleapis/mcp-toolbox/internal/sources/sqlite"
"github.com/googleapis/mcp-toolbox/internal/tools"
_ "github.com/googleapis/mcp-toolbox/internal/tools/sqlite/sqlitesql"
"github.com/spf13/cobra"
)
@@ -350,10 +353,6 @@ func TestGenerateSkill_MissingArguments(t *testing.T) {
name: "missing name",
args: []string{"skills-generate", "--config", toolsFilePath, "--description", "test"},
},
{
name: "missing description",
args: []string{"skills-generate", "--config", toolsFilePath, "--name", "test"},
},
}
for _, tt := range tests {
@@ -366,6 +365,80 @@ func TestGenerateSkill_MissingArguments(t *testing.T) {
}
}
func TestBuildSkillContents(t *testing.T) {
tests := []struct {
name string
cmd *skillsCmd
toolsMap map[string]tools.Tool
groupsMap map[string]group.Group
want map[string]skillContent
}{
{
// len(groupsMap) > 1 (default group plus named groups) triggers group mode.
name: "group mode: group description takes precedence over flag, flag is fallback",
cmd: &skillsCmd{name: "my-skill", description: "flag fallback"},
groupsMap: map[string]group.Group{
"": group.NewGroup(group.GroupConfig{Name: ""}),
"with-desc": group.NewGroup(
group.GroupConfig{Name: "with-desc", Description: "group's own description"}),
"no-desc": group.NewGroup(
group.GroupConfig{Name: "no-desc"}),
},
// The default nameless group is skipped, so it produces no skill.
want: map[string]skillContent{
"my-skill-with-desc": {tools: map[string]tools.Tool{}, description: "group's own description"},
"my-skill-no-desc": {tools: map[string]tools.Tool{}, description: "flag fallback"},
},
},
{
name: "toolset mode: uses flag description, ignores group description",
cmd: &skillsCmd{name: "my-skill", description: "flag desc", toolset: "my-toolset"},
groupsMap: map[string]group.Group{
"": group.NewGroup(group.GroupConfig{Name: ""}),
"my-toolset": group.NewGroup(
group.GroupConfig{Name: "my-toolset", Description: "ignored in toolset mode"}),
},
want: map[string]skillContent{
"my-skill": {tools: map[string]tools.Tool{}, description: "flag desc"},
},
},
{
name: "all-tools mode: falls back to flag when default group has no description",
cmd: &skillsCmd{name: "my-skill", description: "flag desc"},
toolsMap: map[string]tools.Tool{},
groupsMap: map[string]group.Group{
"": group.NewGroup(group.GroupConfig{Name: ""}),
},
want: map[string]skillContent{
"my-skill": {tools: map[string]tools.Tool{}, description: "flag desc"},
},
},
{
name: "all-tools mode: default group description takes precedence over flag",
cmd: &skillsCmd{name: "my-skill", description: "flag desc"},
toolsMap: map[string]tools.Tool{},
groupsMap: map[string]group.Group{
"": group.NewGroup(group.GroupConfig{Name: "", Description: "default group description"}),
},
want: map[string]skillContent{
"my-skill": {tools: map[string]tools.Tool{}, description: "default group description"},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := tt.cmd.buildSkillContents(tt.toolsMap, tt.groupsMap)
if err != nil {
t.Fatalf("buildSkillContents failed: %v", err)
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("buildSkillContents() = %#v, want %#v", got, tt.want)
}
})
}
}
func TestGenerateSkill_FlagValidation(t *testing.T) {
tests := []struct {
name string
@@ -34,7 +34,7 @@ toolbox <tool-source> skills-generate \
- `<tool-source>`: Can be `--config`, `--configs`, `--config-folder`, and `--prebuilt`. See the [CLI Reference](../../../reference/cli.md) for details.
- `--name`: Name of the generated skill. When multiple toolsets are generated because `--toolset` is omitted, this name acts as a prefix for each skill folder (e.g., `<name>-<toolset>`).
- `--description`: Description of the generated skill.
- `--description`: (Optional) Description of the generated skill.
- `--toolset`: (Optional) Name of the toolset to convert into a skill. If not provided, one skill will be generated for every custom toolset defined. If no custom toolsets are defined, it defaults to a single skill containing all tools.
- `--output-dir`: (Optional) Directory to output generated skills (default: "skills").
- `--license-header`: (Optional) Optional license header to prepend to generated node scripts.
+1 -1
View File
@@ -75,7 +75,7 @@ toolbox skills-generate --name <name> --description <description> --toolset <too
**Flags:**
- `--name`: Name of the generated skill. When multiple toolsets are generated because `--toolset` is omitted, this name acts as a prefix for each skill folder (e.g., `<name>-<toolset>`).
- `--description`: Description of the generated skill.
- `--description`: (Optional) Description of the generated skill.
- `--toolset`: (Optional) Name of the toolset to convert into a skill. If not provided, one skill will be generated for every custom toolset defined. If no custom toolsets are defined, it defaults to a single skill containing all tools.
- `--output-dir`: (Optional) Directory to output generated skills (default: "skills").
- `--license-header`: (Optional) Optional license header to prepend to generated node scripts.