Skip to content

Commit c233eee

Browse files
authored
feat(skill): update skill generation logic (googleapis#2646)
This PR refactors and improves the skill generation logic to make it more configurable and also simplifies the generated skill. - Shift from generating individual tool-specific YAML files to a more centralized configuration approach using global toolbox flags. - Add support for --license-header to prepend license information to generated Node.js scripts. - Refactored tool parameter documentation from a JSON schema format to a more readable Markdown table. - Update tool invocation log level to avoid unnecessary outputs. - Make generated skill to be compatible with Gemini CLI's env variable.
1 parent 5271368 commit c233eee

6 files changed

Lines changed: 184 additions & 288 deletions

File tree

cmd/internal/skills/command.go

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"os"
2222
"path/filepath"
2323
"sort"
24+
"strings"
2425

2526
"github.com/googleapis/genai-toolbox/cmd/internal"
2627
"github.com/googleapis/genai-toolbox/internal/server"
@@ -33,10 +34,11 @@ import (
3334
// skillsCmd is the command for generating skills.
3435
type skillsCmd struct {
3536
*cobra.Command
36-
name string
37-
description string
38-
toolset string
39-
outputDir string
37+
name string
38+
description string
39+
toolset string
40+
outputDir string
41+
licenseHeader string
4042
}
4143

4244
// NewCommand creates a new Command.
@@ -54,6 +56,7 @@ func NewCommand(opts *internal.ToolboxOptions) *cobra.Command {
5456
cmd.Flags().StringVar(&cmd.description, "description", "", "Description of the generated skill")
5557
cmd.Flags().StringVar(&cmd.toolset, "toolset", "", "Name of the toolset to convert into a skill. If not provided, all tools will be included.")
5658
cmd.Flags().StringVar(&cmd.outputDir, "output-dir", "skills", "Directory to output generated skills")
59+
cmd.Flags().StringVar(&cmd.licenseHeader, "license-header", "", "Optional license header to prepend to generated node scripts.")
5760

5861
_ = cmd.MarkFlagRequired("name")
5962
_ = cmd.MarkFlagRequired("description")
@@ -123,6 +126,40 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error {
123126
return errMsg
124127
}
125128

129+
var jsConfigArgs []string
130+
if len(opts.PrebuiltConfigs) > 0 {
131+
for _, pc := range opts.PrebuiltConfigs {
132+
jsConfigArgs = append(jsConfigArgs, `"--prebuilt"`, fmt.Sprintf(`"%s"`, pc))
133+
}
134+
}
135+
136+
if opts.ToolsFolder != "" {
137+
folderName := filepath.Base(opts.ToolsFolder)
138+
destFolder := filepath.Join(assetsPath, folderName)
139+
if err := copyDir(opts.ToolsFolder, destFolder); err != nil {
140+
return err
141+
}
142+
jsConfigArgs = append(jsConfigArgs, `"--tools-folder"`, fmt.Sprintf(`path.join(__dirname, "..", "assets", %q)`, folderName))
143+
} else if len(opts.ToolsFiles) > 0 {
144+
for _, f := range opts.ToolsFiles {
145+
baseName := filepath.Base(f)
146+
destPath := filepath.Join(assetsPath, baseName)
147+
if err := copyFile(f, destPath); err != nil {
148+
return err
149+
}
150+
jsConfigArgs = append(jsConfigArgs, `"--tools-files"`, fmt.Sprintf(`path.join(__dirname, "..", "assets", %q)`, baseName))
151+
}
152+
} else if opts.ToolsFile != "" {
153+
baseName := filepath.Base(opts.ToolsFile)
154+
destPath := filepath.Join(assetsPath, baseName)
155+
if err := copyFile(opts.ToolsFile, destPath); err != nil {
156+
return err
157+
}
158+
jsConfigArgs = append(jsConfigArgs, `"--tools-file"`, fmt.Sprintf(`path.join(__dirname, "..", "assets", %q)`, baseName))
159+
}
160+
161+
configArgsStr := strings.Join(jsConfigArgs, ", ")
162+
126163
// Iterate over keys to ensure deterministic order
127164
var toolNames []string
128165
for name := range allTools {
@@ -131,26 +168,8 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error {
131168
sort.Strings(toolNames)
132169

133170
for _, toolName := range toolNames {
134-
// Generate YAML config in asset directory
135-
minimizedContent, err := generateToolConfigYAML(opts.Cfg, toolName)
136-
if err != nil {
137-
errMsg := fmt.Errorf("error generating filtered config for %s: %w", toolName, err)
138-
opts.Logger.ErrorContext(ctx, errMsg.Error())
139-
return errMsg
140-
}
141-
142-
specificToolsFileName := fmt.Sprintf("%s.yaml", toolName)
143-
if minimizedContent != nil {
144-
destPath := filepath.Join(assetsPath, specificToolsFileName)
145-
if err := os.WriteFile(destPath, minimizedContent, 0644); err != nil {
146-
errMsg := fmt.Errorf("error writing filtered config for %s: %w", toolName, err)
147-
opts.Logger.ErrorContext(ctx, errMsg.Error())
148-
return errMsg
149-
}
150-
}
151-
152171
// Generate wrapper script in scripts directory
153-
scriptContent, err := generateScriptContent(toolName, specificToolsFileName)
172+
scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader)
154173
if err != nil {
155174
errMsg := fmt.Errorf("error generating script content for %s: %w", toolName, err)
156175
opts.Logger.ErrorContext(ctx, errMsg.Error())
@@ -213,3 +232,28 @@ func (c *skillsCmd) collectTools(ctx context.Context, opts *internal.ToolboxOpti
213232

214233
return result, nil
215234
}
235+
236+
func copyFile(src, dst string) error {
237+
data, err := os.ReadFile(src)
238+
if err != nil {
239+
return err
240+
}
241+
return os.WriteFile(dst, data, 0644)
242+
}
243+
244+
func copyDir(src, dst string) error {
245+
return filepath.Walk(src, func(path string, info os.FileInfo, err error) error {
246+
if err != nil {
247+
return err
248+
}
249+
relPath, err := filepath.Rel(src, path)
250+
if err != nil {
251+
return err
252+
}
253+
destPath := filepath.Join(dst, relPath)
254+
if info.IsDir() {
255+
return os.MkdirAll(destPath, 0755)
256+
}
257+
return copyFile(path, destPath)
258+
})
259+
}

cmd/internal/skills/command_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ description: hello tool
128128
}
129129

130130
// Check assets
131-
assetPath := filepath.Join(skillPath, "assets", "hello-sqlite.yaml")
131+
assetPath := filepath.Join(skillPath, "assets", "tools.yaml")
132132
if _, err := os.Stat(assetPath); os.IsNotExist(err) {
133133
t.Fatalf("asset file not created: %s", assetPath)
134134
}

cmd/internal/skills/generator.go

Lines changed: 61 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,11 @@
1515
package skills
1616

1717
import (
18-
"bytes"
19-
"encoding/json"
2018
"fmt"
2119
"sort"
2220
"strings"
2321
"text/template"
2422

25-
"github.com/goccy/go-yaml"
26-
"github.com/googleapis/genai-toolbox/internal/server"
27-
"github.com/googleapis/genai-toolbox/internal/sources"
2823
"github.com/googleapis/genai-toolbox/internal/tools"
2924
"github.com/googleapis/genai-toolbox/internal/util/parameters"
3025
)
@@ -39,10 +34,10 @@ description: {{.SkillDescription}}
3934
All scripts can be executed using Node.js. Replace ` + "`" + `<param_name>` + "`" + ` and ` + "`" + `<param_value>` + "`" + ` with actual values.
4035
4136
**Bash:**
42-
` + "`" + `node scripts/<script_name>.js '{"<param_name>": "<param_value>"}'` + "`" + `
37+
` + "`" + `node <skill_dir>/scripts/<script_name>.js '{"<param_name>": "<param_value>"}'` + "`" + `
4338
4439
**PowerShell:**
45-
` + "`" + `node scripts/<script_name>.js '{\"<param_name>\": \"<param_value>\"}'` + "`" + `
40+
` + "`" + `node <skill_dir>/scripts/<script_name>.js '{\"<param_name>\": \"<param_value>\"}'` + "`" + `
4641
4742
## Scripts
4843
@@ -118,29 +113,33 @@ func generateSkillMarkdown(skillName, skillDescription string, toolsMap map[stri
118113
}
119114

120115
const nodeScriptTemplate = `#!/usr/bin/env node
121-
116+
{{if .LicenseHeader}}
117+
{{.LicenseHeader}}
118+
{{end}}
122119
const { spawn, execSync } = require('child_process');
123120
const path = require('path');
124121
const fs = require('fs');
125122
126123
const toolName = "{{.Name}}";
127-
const toolsFileName = "{{.ToolsFileName}}";
124+
const configArgs = [{{.ConfigArgs}}];
128125
129126
function getToolboxPath() {
127+
if (process.env.GEMINI_CLI === '1') {
128+
const localPath = path.resolve(__dirname, '../../../toolbox');
129+
if (fs.existsSync(localPath)) {
130+
return localPath;
131+
}
132+
}
130133
try {
131134
const checkCommand = process.platform === 'win32' ? 'where toolbox' : 'which toolbox';
132135
const globalPath = execSync(checkCommand, { stdio: 'pipe', encoding: 'utf-8' }).trim();
133136
if (globalPath) {
134137
return globalPath.split('\n')[0].trim();
135138
}
139+
throw new Error("Toolbox binary not found");
136140
} catch (e) {
137-
// Ignore error;
138-
}
139-
const localPath = path.resolve(__dirname, '../../../toolbox');
140-
if (fs.existsSync(localPath)) {
141-
return localPath;
141+
throw new Error("Toolbox binary not found");
142142
}
143-
throw new Error("Toolbox binary not found");
144143
}
145144
146145
let toolboxBinary;
@@ -151,15 +150,38 @@ try {
151150
process.exit(1);
152151
}
153152
154-
let configArgs = [];
155-
if (toolsFileName) {
156-
configArgs.push("--tools-file", path.join(__dirname, "..", "assets", toolsFileName));
153+
function getEnv() {
154+
const envPath = path.resolve(__dirname, '../../../.env');
155+
const env = { ...process.env };
156+
if (fs.existsSync(envPath)) {
157+
const envContent = fs.readFileSync(envPath, 'utf-8');
158+
envContent.split('\n').forEach(line => {
159+
const trimmed = line.trim();
160+
if (trimmed && !trimmed.startsWith('#')) {
161+
const splitIdx = trimmed.indexOf('=');
162+
if (splitIdx !== -1) {
163+
const key = trimmed.slice(0, splitIdx).trim();
164+
let value = trimmed.slice(splitIdx + 1).trim();
165+
value = value.replace(/(^['"]|['"]$)/g, '');
166+
if (env[key] === undefined) {
167+
env[key] = value;
168+
}
169+
}
170+
}
171+
});
172+
}
173+
return env;
174+
}
175+
176+
let env = process.env;
177+
if (process.env.GEMINI_CLI === '1') {
178+
env = getEnv();
157179
}
158180
159181
const args = process.argv.slice(2);
160-
const toolboxArgs = [...configArgs, "invoke", toolName, ...args];
182+
const toolboxArgs = ["--log-level", "error", ...configArgs, "invoke", toolName, ...args];
161183
162-
const child = spawn(toolboxBinary, toolboxArgs, { stdio: 'inherit' });
184+
const child = spawn(toolboxBinary, toolboxArgs, { stdio: 'inherit', env });
163185
164186
child.on('close', (code) => {
165187
process.exit(code);
@@ -173,16 +195,18 @@ child.on('error', (err) => {
173195

174196
type scriptData struct {
175197
Name string
176-
ToolsFileName string
198+
ConfigArgs string
199+
LicenseHeader string
177200
}
178201

179202
// generateScriptContent creates the content for a Node.js wrapper script.
180203
// This script invokes the toolbox CLI with the appropriate configuration
181204
// (using a generated tools file) and arguments to execute the specific tool.
182-
func generateScriptContent(name string, toolsFileName string) (string, error) {
205+
func generateScriptContent(name string, configArgs string, licenseHeader string) (string, error) {
183206
data := scriptData{
184207
Name: name,
185-
ToolsFileName: toolsFileName,
208+
ConfigArgs: configArgs,
209+
LicenseHeader: licenseHeader,
186210
}
187211

188212
tmpl, err := template.New("script").Parse(nodeScriptTemplate)
@@ -205,105 +229,31 @@ func formatParameters(params []parameters.ParameterManifest, envVars map[string]
205229
return "", nil
206230
}
207231

208-
properties := make(map[string]interface{})
209-
var required []string
232+
var sb strings.Builder
233+
sb.WriteString("#### Parameters\n\n")
234+
sb.WriteString("| Name | Type | Description | Required | Default |\n")
235+
sb.WriteString("| :--- | :--- | :--- | :--- | :--- |\n")
210236

211237
for _, p := range params {
212-
paramMap := map[string]interface{}{
213-
"type": p.Type,
214-
"description": p.Description,
238+
required := "No"
239+
if p.Required {
240+
required = "Yes"
215241
}
242+
defaultValue := ""
216243
if p.Default != nil {
217-
defaultValue := p.Default
218-
// Check if default value is pre-configured, if so, remove it as the the value will be
219-
// read by the tool at runtime and the agent does not need to be aware of it.
220-
if strVal, ok := defaultValue.(string); ok {
244+
defaultValue = fmt.Sprintf("`%v`", p.Default)
245+
// Check if default value matches any env var
246+
if strVal, ok := p.Default.(string); ok {
221247
for _, envVal := range envVars {
222248
if envVal == strVal {
223-
defaultValue = nil
249+
defaultValue = ""
224250
break
225251
}
226252
}
227253
}
228-
if defaultValue != nil {
229-
paramMap["default"] = defaultValue
230-
}
231-
}
232-
properties[p.Name] = paramMap
233-
if p.Required {
234-
required = append(required, p.Name)
235-
}
236-
}
237-
238-
schema := map[string]interface{}{
239-
"type": "object",
240-
"properties": properties,
241-
}
242-
if len(required) > 0 {
243-
schema["required"] = required
244-
}
245-
246-
schemaJSON, err := json.MarshalIndent(schema, "", " ")
247-
if err != nil {
248-
return "", fmt.Errorf("error generating parameters schema: %w", err)
249-
}
250-
251-
return fmt.Sprintf("#### Parameters\n\n```json\n%s\n```", string(schemaJSON)), nil
252-
}
253-
254-
// generateToolConfigYAML generates the YAML configuration for a single tool and its dependency (source).
255-
// It extracts the relevant tool and source configurations from the server config and formats them
256-
// into a YAML document suitable for inclusion in the skill's assets.
257-
func generateToolConfigYAML(cfg server.ServerConfig, toolName string) ([]byte, error) {
258-
toolCfg, ok := cfg.ToolConfigs[toolName]
259-
if !ok {
260-
return nil, fmt.Errorf("error finding tool config: %s", toolName)
261-
}
262-
263-
var buf bytes.Buffer
264-
encoder := yaml.NewEncoder(&buf)
265-
266-
// Process Tool Config
267-
toolWrapper := struct {
268-
Kind string `yaml:"kind"`
269-
Config tools.ToolConfig `yaml:",inline"`
270-
}{
271-
Kind: "tools",
272-
Config: toolCfg,
273-
}
274-
275-
if err := encoder.Encode(toolWrapper); err != nil {
276-
return nil, fmt.Errorf("error encoding tool config: %w", err)
277-
}
278-
279-
// Process Source Config
280-
var toolMap map[string]interface{}
281-
b, err := yaml.Marshal(toolCfg)
282-
if err != nil {
283-
return nil, fmt.Errorf("error marshaling tool config: %w", err)
284-
}
285-
if err := yaml.Unmarshal(b, &toolMap); err != nil {
286-
return nil, fmt.Errorf("error unmarshaling tool config map: %w", err)
287-
}
288-
289-
if sourceName, ok := toolMap["source"].(string); ok && sourceName != "" {
290-
sourceCfg, ok := cfg.SourceConfigs[sourceName]
291-
if !ok {
292-
return nil, fmt.Errorf("error finding source config: %s", sourceName)
293-
}
294-
295-
sourceWrapper := struct {
296-
Kind string `yaml:"kind"`
297-
Config sources.SourceConfig `yaml:",inline"`
298-
}{
299-
Kind: "sources",
300-
Config: sourceCfg,
301-
}
302-
303-
if err := encoder.Encode(sourceWrapper); err != nil {
304-
return nil, fmt.Errorf("error encoding source config: %w", err)
305254
}
255+
fmt.Fprintf(&sb, "| %s | %s | %s | %s | %s |\n", p.Name, p.Type, p.Description, required, defaultValue)
306256
}
307257

308-
return buf.Bytes(), nil
258+
return sb.String(), nil
309259
}

0 commit comments

Comments
 (0)