From f23a41f1b0e1460922972881c50de0ab4a07c5da Mon Sep 17 00:00:00 2001 From: Haoyu Wang Date: Tue, 31 Mar 2026 17:39:13 -0400 Subject: [PATCH 01/12] feat(skill): tool invocation via npx --- cmd/internal/skills/generator.go | 50 +++++++++++++------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 801101b8d576..5f1082a3d5a3 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -124,37 +124,11 @@ const nodeScriptTemplate = `#!/usr/bin/env node const { spawn, execSync } = require('child_process'); const path = require('path'); const fs = require('fs'); +const os = require('os'); const toolName = "{{.Name}}"; const configArgs = [{{.ConfigArgs}}]; -function getToolboxPath() { - if (process.env.GEMINI_CLI === '1') { - const localPath = path.resolve(__dirname, '../../../toolbox'); - if (fs.existsSync(localPath)) { - return localPath; - } - } - try { - const checkCommand = process.platform === 'win32' ? 'where toolbox' : 'which toolbox'; - const globalPath = execSync(checkCommand, { stdio: 'pipe', encoding: 'utf-8' }).trim(); - if (globalPath) { - return globalPath.split('\n')[0].trim(); - } - throw new Error("Toolbox binary not found"); - } catch (e) { - throw new Error("Toolbox binary not found"); - } -} - -let toolboxBinary; -try { - toolboxBinary = getToolboxPath(); -} catch (err) { - console.error("Error:", err.message); - process.exit(1); -} - function getEnv() { const envPath = path.resolve(__dirname, '../../../.env'); const env = { ...process.env }; @@ -186,10 +160,26 @@ if (process.env.GEMINI_CLI === '1') { } const args = process.argv.slice(2); +const npxArgs = ["--yes", "@toolbox-sdk/server", "--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...args]; + +let command = 'npx'; +let spawnArgs = npxArgs; + +// The Windows Dependency-Free Bypass +if (os.platform() === 'win32') { + const nodeDir = path.dirname(process.execPath); + const npxCliJs = path.join(nodeDir, 'node_modules', 'npm', 'bin', 'npx-cli.js'); + + if (fs.existsSync(npxCliJs)) { + command = process.execPath; + spawnArgs = [npxCliJs, ...npxArgs]; + } else { + console.error("Error: Could not find the npx executable to launch."); + process.exit(1); + } +} -const toolboxArgs = ["--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...args]; - -const child = spawn(toolboxBinary, toolboxArgs, { stdio: 'inherit', env }); +const child = spawn(command, spawnArgs, { stdio: 'inherit', env }); child.on('close', (code) => { process.exit(code); From d58ff453bf966cfd44fd43e056573424686b113b Mon Sep 17 00:00:00 2001 From: Haoyu Wang Date: Thu, 2 Apr 2026 10:43:43 -0400 Subject: [PATCH 02/12] make tool invocation approach configurable --- cmd/internal/skills/command.go | 4 +- cmd/internal/skills/generator.go | 51 ++++++++++++++++--- cmd/internal/skills/generator_test.go | 17 ++++++- .../configuration/skills/_index.md | 1 + docs/en/reference/cli.md | 1 + 5 files changed, 63 insertions(+), 11 deletions(-) diff --git a/cmd/internal/skills/command.go b/cmd/internal/skills/command.go index 4de9c21b228b..223f7b64e16e 100644 --- a/cmd/internal/skills/command.go +++ b/cmd/internal/skills/command.go @@ -40,6 +40,7 @@ type skillsCmd struct { outputDir string licenseHeader string additionalNotes string + invocationMode string } // NewCommand creates a new Command. @@ -62,6 +63,7 @@ func NewCommand(opts *internal.ToolboxOptions) *cobra.Command { 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.") flags.StringVar(&cmd.additionalNotes, "additional-notes", "", "Additional notes to add under the Usage section of the generated SKILL.md") + flags.StringVar(&cmd.invocationMode, "invocation-mode", "binary", "Invocation mode for the generated scripts: 'binary' or 'npx'") _ = cmd.MarkFlagRequired("name") _ = cmd.MarkFlagRequired("description") return cmd.Command @@ -187,7 +189,7 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error { for _, toolName := range toolNames { // Generate wrapper script in scripts directory - scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader) + scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader, cmd.invocationMode) if err != nil { errMsg := fmt.Errorf("error generating script content for %s: %w", toolName, err) opts.Logger.ErrorContext(ctx, errMsg.Error()) diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 5f1082a3d5a3..ee5b59c328cb 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -160,12 +160,13 @@ if (process.env.GEMINI_CLI === '1') { } const args = process.argv.slice(2); + +{{if eq .InvocationMode "npx"}} const npxArgs = ["--yes", "@toolbox-sdk/server", "--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...args]; let command = 'npx'; let spawnArgs = npxArgs; -// The Windows Dependency-Free Bypass if (os.platform() === 'win32') { const nodeDir = path.dirname(process.execPath); const npxCliJs = path.join(nodeDir, 'node_modules', 'npm', 'bin', 'npx-cli.js'); @@ -180,6 +181,38 @@ if (os.platform() === 'win32') { } const child = spawn(command, spawnArgs, { stdio: 'inherit', env }); +{{else}} +function getToolboxPath() { + if (process.env.GEMINI_CLI === '1') { + const localPath = path.resolve(__dirname, '../../../toolbox'); + if (fs.existsSync(localPath)) { + return localPath; + } + } + try { + const checkCommand = process.platform === 'win32' ? 'where toolbox' : 'which toolbox'; + const globalPath = execSync(checkCommand, { stdio: 'pipe', encoding: 'utf-8' }).trim(); + if (globalPath) { + return globalPath.split('\n')[0].trim(); + } + throw new Error("Toolbox binary not found"); + } catch (e) { + throw new Error("Toolbox binary not found"); + } +} + +let toolboxBinary; +try { + toolboxBinary = getToolboxPath(); +} catch (err) { + console.error("Error:", err.message); + process.exit(1); +} + +const toolboxArgs = ["--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...args]; + +const child = spawn(toolboxBinary, toolboxArgs, { stdio: 'inherit', env }); +{{end}} child.on('close', (code) => { process.exit(code); @@ -192,19 +225,21 @@ child.on('error', (err) => { ` type scriptData struct { - Name string - ConfigArgs string - LicenseHeader string + Name string + ConfigArgs string + LicenseHeader string + InvocationMode string } // generateScriptContent creates the content for a Node.js wrapper script. // This script invokes the toolbox CLI with the appropriate configuration // (using a generated config) and arguments to execute the specific tool. -func generateScriptContent(name string, configArgs string, licenseHeader string) (string, error) { +func generateScriptContent(name string, configArgs string, licenseHeader string, mode string) (string, error) { data := scriptData{ - Name: name, - ConfigArgs: configArgs, - LicenseHeader: licenseHeader, + Name: name, + ConfigArgs: configArgs, + LicenseHeader: licenseHeader, + InvocationMode: mode, } tmpl, err := template.New("script").Parse(nodeScriptTemplate) diff --git a/cmd/internal/skills/generator_test.go b/cmd/internal/skills/generator_test.go index 68ad0ef907ae..9e123f03f8c8 100644 --- a/cmd/internal/skills/generator_test.go +++ b/cmd/internal/skills/generator_test.go @@ -219,11 +219,13 @@ func TestGenerateScriptContent(t *testing.T) { configArgs string wantContains []string licenseHeader string + mode string }{ { - name: "basic script", + name: "basic script (binary default)", toolName: "test-tool", configArgs: `"--prebuilt", "test"`, + mode: "binary", wantContains: []string{ `const toolName = "test-tool";`, `const configArgs = ["--prebuilt", "test"];`, @@ -244,15 +246,26 @@ func TestGenerateScriptContent(t *testing.T) { toolName: "test-tool", configArgs: `"--prebuilt", "test"`, licenseHeader: "// My License", + mode: "binary", wantContains: []string{ "// My License", }, }, + { + name: "npx mode script", + toolName: "npx-tool", + configArgs: `"--prebuilt", "test"`, + mode: "npx", + wantContains: []string{ + `const toolName = "npx-tool";`, + `const npxArgs = ["--yes", "@toolbox-sdk/server"`, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := generateScriptContent(tt.toolName, tt.configArgs, tt.licenseHeader) + got, err := generateScriptContent(tt.toolName, tt.configArgs, tt.licenseHeader, tt.mode) if err != nil { t.Fatalf("generateScriptContent() error = %v", err) } diff --git a/docs/en/documentation/configuration/skills/_index.md b/docs/en/documentation/configuration/skills/_index.md index aea2635f8dc5..8496fa43ab5c 100644 --- a/docs/en/documentation/configuration/skills/_index.md +++ b/docs/en/documentation/configuration/skills/_index.md @@ -39,6 +39,7 @@ toolbox skills-generate \ - `--output-dir`: (Optional) Directory to output generated skills (default: "skills"). - `--license-header`: (Optional) Optional license header to prepend to generated node scripts. - `--additional-notes`: (Optional) Additional notes to add under the Usage section of the generated SKILL.md. +- `--invocation-mode`: (Optional) Invocation mode for the generated scripts: 'binary' or 'npx' (default: "binary"). {{< notice note >}} **Note:** The `` must follow the Agent Skill [naming convention](https://agentskills.io/specification): it must contain only lowercase alphanumeric characters and hyphens, cannot start or end with a hyphen, and cannot contain consecutive hyphens (e.g., `my-skill`, `data-processing`). diff --git a/docs/en/reference/cli.md b/docs/en/reference/cli.md index f9bf2065ed86..f5c772f74bf6 100644 --- a/docs/en/reference/cli.md +++ b/docs/en/reference/cli.md @@ -73,6 +73,7 @@ toolbox skills-generate --name --description --toolset Date: Thu, 2 Apr 2026 11:59:43 -0400 Subject: [PATCH 03/12] update npx --- cmd/internal/skills/generator.go | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index ee5b59c328cb..6abcffb19db9 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -162,25 +162,13 @@ if (process.env.GEMINI_CLI === '1') { const args = process.argv.slice(2); {{if eq .InvocationMode "npx"}} -const npxArgs = ["--yes", "@toolbox-sdk/server", "--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...args]; +const command = os.platform() === 'win32' ? 'npx.cmd' : 'npx'; -let command = 'npx'; -let spawnArgs = npxArgs; +const processedArgs = os.platform() === 'win32' ? args.map(arg => arg.includes('"') ? '"' + arg.replace(/"/g, '""') + '"' : arg) : args; -if (os.platform() === 'win32') { - const nodeDir = path.dirname(process.execPath); - const npxCliJs = path.join(nodeDir, 'node_modules', 'npm', 'bin', 'npx-cli.js'); +const npxArgs = ["--yes", "@toolbox-sdk/server", "--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...processedArgs]; - if (fs.existsSync(npxCliJs)) { - command = process.execPath; - spawnArgs = [npxCliJs, ...npxArgs]; - } else { - console.error("Error: Could not find the npx executable to launch."); - process.exit(1); - } -} - -const child = spawn(command, spawnArgs, { stdio: 'inherit', env }); +const child = spawn(command, npxArgs, { shell: os.platform() === 'win32', stdio: 'inherit', env }); {{else}} function getToolboxPath() { if (process.env.GEMINI_CLI === '1') { From 959c316ffc9973a6b683bf9d6a113f55857f72bd Mon Sep 17 00:00:00 2001 From: Haoyu Wang Date: Thu, 2 Apr 2026 12:48:39 -0400 Subject: [PATCH 04/12] make toolbox version configurable --- cmd/internal/options.go | 1 + cmd/internal/skills/command.go | 4 +++- cmd/internal/skills/generator.go | 6 ++++-- cmd/internal/skills/generator_test.go | 6 ++++-- cmd/root.go | 1 + docs/en/documentation/configuration/skills/_index.md | 1 + docs/en/reference/cli.md | 1 + 7 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cmd/internal/options.go b/cmd/internal/options.go index 337e2d43912f..abdc67347283 100644 --- a/cmd/internal/options.go +++ b/cmd/internal/options.go @@ -44,6 +44,7 @@ type ToolboxOptions struct { Configs []string ConfigFolder string PrebuiltConfigs []string + VersionNum string } // Option defines a function that modifies the ToolboxOptions struct. diff --git a/cmd/internal/skills/command.go b/cmd/internal/skills/command.go index 223f7b64e16e..085012fdf76e 100644 --- a/cmd/internal/skills/command.go +++ b/cmd/internal/skills/command.go @@ -41,6 +41,7 @@ type skillsCmd struct { licenseHeader string additionalNotes string invocationMode string + toolboxVersion string } // NewCommand creates a new Command. @@ -64,6 +65,7 @@ func NewCommand(opts *internal.ToolboxOptions) *cobra.Command { flags.StringVar(&cmd.licenseHeader, "license-header", "", "Optional license header to prepend to generated node scripts.") flags.StringVar(&cmd.additionalNotes, "additional-notes", "", "Additional notes to add under the Usage section of the generated SKILL.md") flags.StringVar(&cmd.invocationMode, "invocation-mode", "binary", "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 @@ -189,7 +191,7 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error { for _, toolName := range toolNames { // Generate wrapper script in scripts directory - scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader, cmd.invocationMode) + scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader, cmd.invocationMode, cmd.toolboxVersion) if err != nil { errMsg := fmt.Errorf("error generating script content for %s: %w", toolName, err) opts.Logger.ErrorContext(ctx, errMsg.Error()) diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 8ad96365a657..014b7cbddb1e 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -166,7 +166,7 @@ const command = os.platform() === 'win32' ? 'npx.cmd' : 'npx'; const processedArgs = os.platform() === 'win32' ? args.map(arg => arg.includes('"') ? '"' + arg.replace(/"/g, '""') + '"' : arg) : args; -const npxArgs = ["--yes", "@toolbox-sdk/server", "--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...processedArgs]; +const npxArgs = ["--yes", "@toolbox-sdk/server@{{.ToolboxVersion}}", "--log-level", "error", ...configArgs, "invoke", toolName, "--user-agent-metadata", userAgent, ...processedArgs]; const child = spawn(command, npxArgs, { shell: os.platform() === 'win32', stdio: 'inherit', env }); {{else}} @@ -218,17 +218,19 @@ type scriptData struct { ConfigArgs string LicenseHeader string InvocationMode string + ToolboxVersion string } // generateScriptContent creates the content for a Node.js wrapper script. // This script invokes the toolbox CLI with the appropriate configuration // (using a generated config) and arguments to execute the specific tool. -func generateScriptContent(name string, configArgs string, licenseHeader string, mode string) (string, error) { +func generateScriptContent(name string, configArgs string, licenseHeader string, mode string, version string) (string, error) { data := scriptData{ Name: name, ConfigArgs: configArgs, LicenseHeader: licenseHeader, InvocationMode: mode, + ToolboxVersion: version, } tmpl, err := template.New("script").Parse(nodeScriptTemplate) diff --git a/cmd/internal/skills/generator_test.go b/cmd/internal/skills/generator_test.go index 9e123f03f8c8..18cfecf1c9d0 100644 --- a/cmd/internal/skills/generator_test.go +++ b/cmd/internal/skills/generator_test.go @@ -220,6 +220,7 @@ func TestGenerateScriptContent(t *testing.T) { wantContains []string licenseHeader string mode string + version string }{ { name: "basic script (binary default)", @@ -256,16 +257,17 @@ func TestGenerateScriptContent(t *testing.T) { toolName: "npx-tool", configArgs: `"--prebuilt", "test"`, mode: "npx", + version: "0.31.0", wantContains: []string{ `const toolName = "npx-tool";`, - `const npxArgs = ["--yes", "@toolbox-sdk/server"`, + `const npxArgs = ["--yes", "@toolbox-sdk/server@0.31.0"`, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := generateScriptContent(tt.toolName, tt.configArgs, tt.licenseHeader, tt.mode) + got, err := generateScriptContent(tt.toolName, tt.configArgs, tt.licenseHeader, tt.mode, tt.version) if err != nil { t.Fatalf("generateScriptContent() error = %v", err) } diff --git a/cmd/root.go b/cmd/root.go index 85f768cd94f3..41de25b23f40 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -105,6 +105,7 @@ func NewCommand(opts *internal.ToolboxOptions) *cobra.Command { // Set server version opts.Cfg.Version = versionString + opts.VersionNum = strings.TrimSpace(versionNum) // set baseCmd in, out and err the same as cmd. cmd.SetIn(opts.IOStreams.In) diff --git a/docs/en/documentation/configuration/skills/_index.md b/docs/en/documentation/configuration/skills/_index.md index 8496fa43ab5c..2360b4d98635 100644 --- a/docs/en/documentation/configuration/skills/_index.md +++ b/docs/en/documentation/configuration/skills/_index.md @@ -40,6 +40,7 @@ toolbox skills-generate \ - `--license-header`: (Optional) Optional license header to prepend to generated node scripts. - `--additional-notes`: (Optional) Additional notes to add under the Usage section of the generated SKILL.md. - `--invocation-mode`: (Optional) Invocation mode for the generated scripts: 'binary' or 'npx' (default: "binary"). +- `--toolbox-version`: (Optional) Version of @toolbox-sdk/server to use for npx approach (defaults to current toolbox version). {{< notice note >}} **Note:** The `` must follow the Agent Skill [naming convention](https://agentskills.io/specification): it must contain only lowercase alphanumeric characters and hyphens, cannot start or end with a hyphen, and cannot contain consecutive hyphens (e.g., `my-skill`, `data-processing`). diff --git a/docs/en/reference/cli.md b/docs/en/reference/cli.md index f5c772f74bf6..fad7ebaa63f8 100644 --- a/docs/en/reference/cli.md +++ b/docs/en/reference/cli.md @@ -74,6 +74,7 @@ toolbox skills-generate --name --description --toolset Date: Mon, 6 Apr 2026 18:24:16 +0530 Subject: [PATCH 05/12] feat(skills): prevent empty strings overriding optional env vars in wrappers --- cmd/internal/config.go | 19 +++++++++++- cmd/internal/config_test.go | 33 ++++++++++++++++---- cmd/internal/skills/command.go | 2 +- cmd/internal/skills/generator.go | 44 +++++++++++++++++++++++++-- cmd/internal/skills/generator_test.go | 33 +++++++++++++++++++- 5 files changed, 120 insertions(+), 11 deletions(-) diff --git a/cmd/internal/config.go b/cmd/internal/config.go index 2e6c3e4162e7..44db265c72d2 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -41,7 +41,8 @@ type Config struct { } type ConfigParser struct { - EnvVars map[string]string + EnvVars map[string]string + OptionalEnvVars []string } // parseEnv replaces environment variables ${ENV_NAME} with their values. @@ -59,6 +60,22 @@ func (p *ConfigParser) parseEnv(input string) (string, error) { // extract the variable name variableName := parts[1] + + isOptional := len(parts) >= 4 && parts[2] != "" + if isOptional { + // Add to OptionalEnvVars if not already present + foundOptional := false + for _, v := range p.OptionalEnvVars { + if v == variableName { + foundOptional = true + break + } + } + if !foundOptional { + p.OptionalEnvVars = append(p.OptionalEnvVars, variableName) + } + } + if value, found := os.LookupEnv(variableName); found { p.EnvVars[variableName] = value return value diff --git a/cmd/internal/config_test.go b/cmd/internal/config_test.go index 9e0d42785412..330797534695 100644 --- a/cmd/internal/config_test.go +++ b/cmd/internal/config_test.go @@ -38,12 +38,13 @@ import ( func TestParseEnv(t *testing.T) { tcs := []struct { - desc string - env map[string]string - in string - want string - err bool - errString string + desc string + env map[string]string + in string + want string + err bool + errString string + wantOptional []string }{ { desc: "without default without env", @@ -64,11 +65,13 @@ func TestParseEnv(t *testing.T) { desc: "with empty default", in: "${FOO:}", want: "", + wantOptional: []string{"FOO"}, }, { desc: "with default", in: "${FOO:bar}", want: "bar", + wantOptional: []string{"FOO"}, }, { desc: "with default with env", @@ -77,6 +80,16 @@ func TestParseEnv(t *testing.T) { }, in: "${FOO:bar}", want: "hello", + wantOptional: []string{"FOO"}, + }, + { + desc: "multiple variables", + in: "user: ${USER_NAME:}, password: ${PASSWORD:}, ip: ${IP:public}, region: ${REGION}", + env: map[string]string{ + "REGION": "us-central1", + }, + want: "user: , password: , ip: public, region: us-central1", + wantOptional: []string{"USER_NAME", "PASSWORD", "IP"}, }, } for _, tc := range tcs { @@ -99,6 +112,14 @@ func TestParseEnv(t *testing.T) { if tc.want != got { t.Fatalf("unexpected want: got %s, want %s", got, tc.want) } + if len(parser.OptionalEnvVars) != len(tc.wantOptional) { + t.Fatalf("OptionalEnvVars length mismatch: got %d, want %d. Got: %v, Want: %v", len(parser.OptionalEnvVars), len(tc.wantOptional), parser.OptionalEnvVars, tc.wantOptional) + } + for i, v := range parser.OptionalEnvVars { + if v != tc.wantOptional[i] { + t.Errorf("OptionalEnvVars element %d mismatch: got %q, want %q", i, v, tc.wantOptional[i]) + } + } }) } } diff --git a/cmd/internal/skills/command.go b/cmd/internal/skills/command.go index 085012fdf76e..3ae1d1436752 100644 --- a/cmd/internal/skills/command.go +++ b/cmd/internal/skills/command.go @@ -191,7 +191,7 @@ func run(cmd *skillsCmd, opts *internal.ToolboxOptions) error { for _, toolName := range toolNames { // Generate wrapper script in scripts directory - scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader, cmd.invocationMode, cmd.toolboxVersion) + scriptContent, err := generateScriptContent(toolName, configArgsStr, cmd.licenseHeader, cmd.invocationMode, cmd.toolboxVersion, parser.OptionalEnvVars) if err != nil { errMsg := fmt.Errorf("error generating script content for %s: %w", toolName, err) opts.Logger.ErrorContext(ctx, errMsg.Error()) diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 014b7cbddb1e..5f70c992217a 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -128,6 +128,38 @@ const os = require('os'); const toolName = "{{.Name}}"; const configArgs = [{{.ConfigArgs}}]; +{{if .OptionalVars}} +const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [ +{{range .OptionalVars}} '{{.}}', +{{end}}]; +{{end}} +function getToolboxPath() { + if (process.env.GEMINI_CLI === '1') { + const ext = process.platform === 'win32' ? '.exe' : ''; + const localPath = path.resolve(__dirname, '../../../toolbox' + ext); + if (fs.existsSync(localPath)) { + return localPath; + } + } + try { + const checkCommand = process.platform === 'win32' ? 'where toolbox' : 'which toolbox'; + const globalPath = execSync(checkCommand, { stdio: 'pipe', encoding: 'utf-8' }).trim(); + if (globalPath) { + return globalPath.split('\n')[0].trim(); + } + throw new Error("Toolbox binary not found"); + } catch (e) { + throw new Error("Toolbox binary not found"); + } +} + +let toolboxBinary; +try { + toolboxBinary = getToolboxPath(); +} catch (err) { + console.error("Error:", err.message); + process.exit(1); +} function getEnv() { const envPath = path.resolve(__dirname, '../../../.env'); @@ -158,7 +190,13 @@ if (process.env.GEMINI_CLI === '1') { env = getEnv(); userAgent = "skills-geminicli"; } - +{{if .OptionalVars}} +OPTIONAL_VARS_TO_OMIT_IF_EMPTY.forEach(varName => { + if (env[varName] === '') { + delete env[varName]; + } +}); +{{end}} const args = process.argv.slice(2); {{if eq .InvocationMode "npx"}} @@ -219,18 +257,20 @@ type scriptData struct { LicenseHeader string InvocationMode string ToolboxVersion string + OptionalVars []string } // generateScriptContent creates the content for a Node.js wrapper script. // This script invokes the toolbox CLI with the appropriate configuration // (using a generated config) and arguments to execute the specific tool. -func generateScriptContent(name string, configArgs string, licenseHeader string, mode string, version string) (string, error) { +func generateScriptContent(name string, configArgs string, licenseHeader string, mode string, version string, optionalVars []string) (string, error) { data := scriptData{ Name: name, ConfigArgs: configArgs, LicenseHeader: licenseHeader, InvocationMode: mode, ToolboxVersion: version, + OptionalVars: optionalVars, } tmpl, err := template.New("script").Parse(nodeScriptTemplate) diff --git a/cmd/internal/skills/generator_test.go b/cmd/internal/skills/generator_test.go index 18cfecf1c9d0..1e7c6e26be0b 100644 --- a/cmd/internal/skills/generator_test.go +++ b/cmd/internal/skills/generator_test.go @@ -221,6 +221,7 @@ func TestGenerateScriptContent(t *testing.T) { licenseHeader string mode string version string + optionalVars []string }{ { name: "basic script (binary default)", @@ -252,6 +253,36 @@ func TestGenerateScriptContent(t *testing.T) { "// My License", }, }, + { + name: "script with optional vars", + toolName: "test-tool", + configArgs: `"--prebuilt", "test"`, + optionalVars: []string{"OPTIONAL_A", "OPTIONAL_B"}, + wantContains: []string{ + "const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [", + " 'OPTIONAL_A',", + " 'OPTIONAL_B',", + "];", + "OPTIONAL_VARS_TO_OMIT_IF_EMPTY.forEach(varName => {", + " if (env[varName] === '') {", + " delete env[varName];", + }, + }, + { + name: "script with optional vars (with default value syntax)", + toolName: "test-tool", + configArgs: `"--prebuilt", "test"`, + optionalVars: []string{"CLOUD_SQL_POSTGRES_USER", "CLOUD_SQL_POSTGRES_IP_TYPE"}, + wantContains: []string{ + "const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [", + " 'CLOUD_SQL_POSTGRES_USER',", + " 'CLOUD_SQL_POSTGRES_IP_TYPE',", + "];", + "OPTIONAL_VARS_TO_OMIT_IF_EMPTY.forEach(varName => {", + " if (env[varName] === '') {", + " delete env[varName];", + }, + }, { name: "npx mode script", toolName: "npx-tool", @@ -267,7 +298,7 @@ func TestGenerateScriptContent(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := generateScriptContent(tt.toolName, tt.configArgs, tt.licenseHeader, tt.mode, tt.version) + got, err := generateScriptContent(tt.toolName, tt.configArgs, tt.licenseHeader, tt.mode, tt.version, tt.optionalVars) if err != nil { t.Fatalf("generateScriptContent() error = %v", err) } From f6cd3d55ad57b913306fd007b561fc8f9623bb38 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Mon, 6 Apr 2026 18:27:50 +0530 Subject: [PATCH 06/12] revert accidental changes --- cmd/internal/skills/generator.go | 28 --------------------------- cmd/internal/skills/generator_test.go | 15 -------------- 2 files changed, 43 deletions(-) diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 5f70c992217a..3d8b4993002d 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -132,34 +132,6 @@ const configArgs = [{{.ConfigArgs}}]; const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [ {{range .OptionalVars}} '{{.}}', {{end}}]; -{{end}} -function getToolboxPath() { - if (process.env.GEMINI_CLI === '1') { - const ext = process.platform === 'win32' ? '.exe' : ''; - const localPath = path.resolve(__dirname, '../../../toolbox' + ext); - if (fs.existsSync(localPath)) { - return localPath; - } - } - try { - const checkCommand = process.platform === 'win32' ? 'where toolbox' : 'which toolbox'; - const globalPath = execSync(checkCommand, { stdio: 'pipe', encoding: 'utf-8' }).trim(); - if (globalPath) { - return globalPath.split('\n')[0].trim(); - } - throw new Error("Toolbox binary not found"); - } catch (e) { - throw new Error("Toolbox binary not found"); - } -} - -let toolboxBinary; -try { - toolboxBinary = getToolboxPath(); -} catch (err) { - console.error("Error:", err.message); - process.exit(1); -} function getEnv() { const envPath = path.resolve(__dirname, '../../../.env'); diff --git a/cmd/internal/skills/generator_test.go b/cmd/internal/skills/generator_test.go index 1e7c6e26be0b..61751d51a3a8 100644 --- a/cmd/internal/skills/generator_test.go +++ b/cmd/internal/skills/generator_test.go @@ -268,21 +268,6 @@ func TestGenerateScriptContent(t *testing.T) { " delete env[varName];", }, }, - { - name: "script with optional vars (with default value syntax)", - toolName: "test-tool", - configArgs: `"--prebuilt", "test"`, - optionalVars: []string{"CLOUD_SQL_POSTGRES_USER", "CLOUD_SQL_POSTGRES_IP_TYPE"}, - wantContains: []string{ - "const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [", - " 'CLOUD_SQL_POSTGRES_USER',", - " 'CLOUD_SQL_POSTGRES_IP_TYPE',", - "];", - "OPTIONAL_VARS_TO_OMIT_IF_EMPTY.forEach(varName => {", - " if (env[varName] === '') {", - " delete env[varName];", - }, - }, { name: "npx mode script", toolName: "npx-tool", From 29267eb130b8055993970748f4a436b6eb66f208 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Mon, 6 Apr 2026 18:42:21 +0530 Subject: [PATCH 07/12] refactor(config): use slices.Contains for optional env var detection Simplify the logic used to check for duplicate entries in OptionalEnvVars by using the slices.Contains idiom. This improves readability and aligns with standard Go 1.21+ patterns. Also fix template syntax error. --- cmd/internal/config.go | 21 ++++++++++++++------- cmd/internal/config_test.go | 9 +++++++++ cmd/internal/skills/generator.go | 2 +- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/cmd/internal/config.go b/cmd/internal/config.go index 44db265c72d2..13c173e8967a 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -43,6 +43,7 @@ type Config struct { type ConfigParser struct { EnvVars map[string]string OptionalEnvVars []string + RequiredEnvVars []string } // parseEnv replaces environment variables ${ENV_NAME} with their values. @@ -63,17 +64,23 @@ func (p *ConfigParser) parseEnv(input string) (string, error) { isOptional := len(parts) >= 4 && parts[2] != "" if isOptional { - // Add to OptionalEnvVars if not already present - foundOptional := false - for _, v := range p.OptionalEnvVars { + // Add to optional list only if it hasn't been explicitly required + if !slices.Contains(p.RequiredEnvVars, variableName) && !slices.Contains(p.OptionalEnvVars, variableName) { + p.OptionalEnvVars = append(p.OptionalEnvVars, variableName) + } + } else { + // Mark as required + if !slices.Contains(p.RequiredEnvVars, variableName) { + p.RequiredEnvVars = append(p.RequiredEnvVars, variableName) + } + + // Remove from optional list if it's there + for i, v := range p.OptionalEnvVars { if v == variableName { - foundOptional = true + p.OptionalEnvVars = append(p.OptionalEnvVars[:i], p.OptionalEnvVars[i+1:]...) break } } - if !foundOptional { - p.OptionalEnvVars = append(p.OptionalEnvVars, variableName) - } } if value, found := os.LookupEnv(variableName); found { diff --git a/cmd/internal/config_test.go b/cmd/internal/config_test.go index 330797534695..ddfb95e94575 100644 --- a/cmd/internal/config_test.go +++ b/cmd/internal/config_test.go @@ -91,6 +91,15 @@ func TestParseEnv(t *testing.T) { want: "user: , password: , ip: public, region: us-central1", wantOptional: []string{"USER_NAME", "PASSWORD", "IP"}, }, + { + desc: "variable required in one place and optional in another", + in: "project_req: ${PROJECT_ID}, project_opt: ${PROJECT_ID:default}", + env: map[string]string{ + "PROJECT_ID": "my_project", + }, + want: "project_req: my_project, project_opt: my_project", + wantOptional: []string{}, // Because it was marked required at least once + }, } for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 3d8b4993002d..88bcf9ed5db9 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -132,7 +132,7 @@ const configArgs = [{{.ConfigArgs}}]; const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [ {{range .OptionalVars}} '{{.}}', {{end}}]; - +{{end}} function getEnv() { const envPath = path.resolve(__dirname, '../../../.env'); const env = { ...process.env }; From 2a18fd414ce432f6496cd39585a2f41976cf7a7d Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Mon, 6 Apr 2026 18:42:21 +0530 Subject: [PATCH 08/12] refactor(config): use slices.Contains for optional env var detection Simplify the logic used to check for duplicate entries in OptionalEnvVars by using the slices.Contains idiom. This improves readability and aligns with standard Go 1.21+ patterns. Also fix template syntax error. --- cmd/internal/config_test.go | 18 +++++++++--------- cmd/internal/skills/generator.go | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/internal/config_test.go b/cmd/internal/config_test.go index ddfb95e94575..1f1517b9446b 100644 --- a/cmd/internal/config_test.go +++ b/cmd/internal/config_test.go @@ -62,15 +62,15 @@ func TestParseEnv(t *testing.T) { want: "bar", }, { - desc: "with empty default", - in: "${FOO:}", - want: "", + desc: "with empty default", + in: "${FOO:}", + want: "", wantOptional: []string{"FOO"}, }, { - desc: "with default", - in: "${FOO:bar}", - want: "bar", + desc: "with default", + in: "${FOO:bar}", + want: "bar", wantOptional: []string{"FOO"}, }, { @@ -78,8 +78,8 @@ func TestParseEnv(t *testing.T) { env: map[string]string{ "FOO": "hello", }, - in: "${FOO:bar}", - want: "hello", + in: "${FOO:bar}", + want: "hello", wantOptional: []string{"FOO"}, }, { @@ -88,7 +88,7 @@ func TestParseEnv(t *testing.T) { env: map[string]string{ "REGION": "us-central1", }, - want: "user: , password: , ip: public, region: us-central1", + want: "user: , password: , ip: public, region: us-central1", wantOptional: []string{"USER_NAME", "PASSWORD", "IP"}, }, { diff --git a/cmd/internal/skills/generator.go b/cmd/internal/skills/generator.go index 88bcf9ed5db9..7c9a34cee227 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -229,7 +229,7 @@ type scriptData struct { LicenseHeader string InvocationMode string ToolboxVersion string - OptionalVars []string + OptionalVars []string } // generateScriptContent creates the content for a Node.js wrapper script. @@ -242,7 +242,7 @@ func generateScriptContent(name string, configArgs string, licenseHeader string, LicenseHeader: licenseHeader, InvocationMode: mode, ToolboxVersion: version, - OptionalVars: optionalVars, + OptionalVars: optionalVars, } tmpl, err := template.New("script").Parse(nodeScriptTemplate) From 720222e7b4cc43cebef351cee67d360cf7c08136 Mon Sep 17 00:00:00 2001 From: Twisha Bansal <58483338+twishabansal@users.noreply.github.com> Date: Mon, 6 Apr 2026 22:02:37 +0530 Subject: [PATCH 09/12] Update cmd/internal/config.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- cmd/internal/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/internal/config.go b/cmd/internal/config.go index 13c173e8967a..f8784bc05db7 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -43,7 +43,7 @@ type Config struct { type ConfigParser struct { EnvVars map[string]string OptionalEnvVars []string - RequiredEnvVars []string + requiredEnvVars []string } // parseEnv replaces environment variables ${ENV_NAME} with their values. From 8242c3698a3b51842de3e8a1c1ab8c5440eeb542 Mon Sep 17 00:00:00 2001 From: Twisha Bansal Date: Mon, 6 Apr 2026 22:03:24 +0530 Subject: [PATCH 10/12] do not expose requiredEnvVars to public API --- cmd/internal/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/internal/config.go b/cmd/internal/config.go index f8784bc05db7..24a54ad9c2a7 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -65,13 +65,13 @@ func (p *ConfigParser) parseEnv(input string) (string, error) { isOptional := len(parts) >= 4 && parts[2] != "" if isOptional { // Add to optional list only if it hasn't been explicitly required - if !slices.Contains(p.RequiredEnvVars, variableName) && !slices.Contains(p.OptionalEnvVars, variableName) { + if !slices.Contains(p.requiredEnvVars, variableName) && !slices.Contains(p.OptionalEnvVars, variableName) { p.OptionalEnvVars = append(p.OptionalEnvVars, variableName) } } else { // Mark as required - if !slices.Contains(p.RequiredEnvVars, variableName) { - p.RequiredEnvVars = append(p.RequiredEnvVars, variableName) + if !slices.Contains(p.requiredEnvVars, variableName) { + p.requiredEnvVars = append(p.requiredEnvVars, variableName) } // Remove from optional list if it's there From a3a245befd21bac261055645ff0508acede69858 Mon Sep 17 00:00:00 2001 From: Twisha Bansal <58483338+twishabansal@users.noreply.github.com> Date: Mon, 6 Apr 2026 22:09:25 +0530 Subject: [PATCH 11/12] Update config.go --- cmd/internal/config.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cmd/internal/config.go b/cmd/internal/config.go index 24a54ad9c2a7..c7cfc05b6098 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -75,11 +75,8 @@ func (p *ConfigParser) parseEnv(input string) (string, error) { } // Remove from optional list if it's there - for i, v := range p.OptionalEnvVars { - if v == variableName { - p.OptionalEnvVars = append(p.OptionalEnvVars[:i], p.OptionalEnvVars[i+1:]...) - break - } + if i := slices.Index(p.OptionalEnvVars, variableName); i != -1 { + p.OptionalEnvVars = slices.Delete(p.OptionalEnvVars, i, i+1) } } From 8053ca27b7f174fae5e5c49549cce0dc3d24eeac Mon Sep 17 00:00:00 2001 From: Twisha Bansal <58483338+twishabansal@users.noreply.github.com> Date: Mon, 6 Apr 2026 22:09:25 +0530 Subject: [PATCH 12/12] Update config.go --- cmd/internal/config.go | 2 +- cmd/internal/config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/internal/config.go b/cmd/internal/config.go index c7cfc05b6098..cc1d74a2e94d 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -73,7 +73,7 @@ func (p *ConfigParser) parseEnv(input string) (string, error) { if !slices.Contains(p.requiredEnvVars, variableName) { p.requiredEnvVars = append(p.requiredEnvVars, variableName) } - + // Remove from optional list if it's there if i := slices.Index(p.OptionalEnvVars, variableName); i != -1 { p.OptionalEnvVars = slices.Delete(p.OptionalEnvVars, i, i+1) diff --git a/cmd/internal/config_test.go b/cmd/internal/config_test.go index 1f1517b9446b..38b9261358c5 100644 --- a/cmd/internal/config_test.go +++ b/cmd/internal/config_test.go @@ -97,7 +97,7 @@ func TestParseEnv(t *testing.T) { env: map[string]string{ "PROJECT_ID": "my_project", }, - want: "project_req: my_project, project_opt: my_project", + want: "project_req: my_project, project_opt: my_project", wantOptional: []string{}, // Because it was marked required at least once }, }