diff --git a/cmd/internal/config.go b/cmd/internal/config.go index 2e6c3e4162e7..cc1d74a2e94d 100644 --- a/cmd/internal/config.go +++ b/cmd/internal/config.go @@ -41,7 +41,9 @@ type Config struct { } type ConfigParser struct { - EnvVars map[string]string + EnvVars map[string]string + OptionalEnvVars []string + requiredEnvVars []string } // parseEnv replaces environment variables ${ENV_NAME} with their values. @@ -59,6 +61,25 @@ 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 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 + if i := slices.Index(p.OptionalEnvVars, variableName); i != -1 { + p.OptionalEnvVars = slices.Delete(p.OptionalEnvVars, i, i+1) + } + } + 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..38b9261358c5 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", @@ -61,22 +62,43 @@ 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"}, }, { desc: "with default with env", env: map[string]string{ "FOO": "hello", }, - in: "${FOO:bar}", - want: "hello", + 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"}, + }, + { + 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 { @@ -99,6 +121,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 739eb55f53bb..c619c3fcebfb 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..7c9a34cee227 100644 --- a/cmd/internal/skills/generator.go +++ b/cmd/internal/skills/generator.go @@ -128,7 +128,11 @@ const os = require('os'); const toolName = "{{.Name}}"; const configArgs = [{{.ConfigArgs}}]; - +{{if .OptionalVars}} +const OPTIONAL_VARS_TO_OMIT_IF_EMPTY = [ +{{range .OptionalVars}} '{{.}}', +{{end}}]; +{{end}} function getEnv() { const envPath = path.resolve(__dirname, '../../../.env'); const env = { ...process.env }; @@ -158,7 +162,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 +229,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..61751d51a3a8 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,21 @@ 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: "npx mode script", toolName: "npx-tool", @@ -267,7 +283,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) }