Skip to content

fix(skills): prevent empty strings overriding optional env vars in node scripts#2963

Merged
twishabansal merged 15 commits intomainfrom
skill-opt-env
Apr 7, 2026
Merged

fix(skills): prevent empty strings overriding optional env vars in node scripts#2963
twishabansal merged 15 commits intomainfrom
skill-opt-env

Conversation

@twishabansal
Copy link
Copy Markdown
Contributor

@twishabansal twishabansal commented Apr 6, 2026

Description

Fixes an issue where optional environment variables (e.g., ${CLOUD_SQL_POSTGRES_PASSWORD:}) were being passed as empty strings to the underlying toolbox process by the generated Node.js skill scripts. This disrupted fallback logic (like implicit IAM authentication).
The ConfigParser now tracks optional variables during YAML parsing. The generator uses this list to embed logic in the Node wrapper script to delete the environment variable from the spawned process if it evaluates to "".

Note: If a variable is marked as required (e.g., ${PROJECT_ID}) in one source, but optional (e.g., ${PROJECT_ID:}) in another within the same configuration file, the parser strictly enforces the required constraint, ensuring it is not mistakenly omitted when empty. Eg. in the cloud sql postgres source, CLOUD_SQL_POSTGRES_PROJECT is treated as required despite being optional for the admin source.

PR Checklist

Thank you for opening a Pull Request! Before submitting your PR, there are a
few things you can do to make sure it goes smoothly:

  • Make sure you reviewed
    CONTRIBUTING.md
  • Make sure to open an issue as a
    bug/issue
    before writing your code! That way we can discuss the change, evaluate
    designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Make sure to add ! if this involve a breaking change

🛠️ Fixes #<issue_number_goes_here>

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for optional environment variables, ensuring they are omitted from the environment in generated Node.js scripts when empty. A critical issue was found in the script generator where duplicate declarations would cause a syntax error and break npx mode. Additionally, the configuration parser's duplicate check should be refactored to use the more idiomatic slices.Contains function.

Base automatically changed from skill-npx to main April 6, 2026 13:28
twishabansal and others added 2 commits April 6, 2026 19:23
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.
@twishabansal
Copy link
Copy Markdown
Contributor Author

/gemini review

@twishabansal twishabansal marked this pull request as ready for review April 6, 2026 14:02
@twishabansal twishabansal requested a review from a team as a code owner April 6, 2026 14:02
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to track and handle optional environment variables within the configuration parser and generated wrapper scripts. The ConfigParser now distinguishes between required and optional variables based on the presence of default values in the configuration. Additionally, the generated Node.js wrapper scripts are updated to automatically omit optional environment variables if they are set to empty strings. I have no feedback to provide as there were no review comments to assess.

@twishabansal twishabansal assigned helloeve and unassigned Yuan325 Apr 6, 2026
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.
@twishabansal
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces tracking for optional and required environment variables within the ConfigParser. This metadata is utilized during the generation of Node.js wrapper scripts to ensure that optional variables are omitted from the environment if they are empty. The review feedback suggests unexporting the RequiredEnvVars field to improve encapsulation and refactoring the environment variable tracking logic to use more idiomatic functions from the slices package.

twishabansal and others added 3 commits April 6, 2026 22:02
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@twishabansal twishabansal changed the title feat(skills): prevent empty strings overriding optional env vars in node scripts fix(skills): prevent empty strings overriding optional env vars in node scripts Apr 6, 2026
@twishabansal twishabansal added the release candidate Use label to signal PR should be included in the next release. label Apr 6, 2026
@twishabansal twishabansal enabled auto-merge (squash) April 7, 2026 07:04
@twishabansal twishabansal merged commit c52adeb into main Apr 7, 2026
18 checks passed
@twishabansal twishabansal deleted the skill-opt-env branch April 7, 2026 07:13
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

🧨 Preview deployments removed.

Cloudflare Pages environments for pr-2963 have been deleted.

Yuan325 added a commit that referenced this pull request Apr 8, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.32.0](v0.31.0...v0.32.0)
(2026-04-08)


### ⚠ BREAKING CHANGES

* update repo name
([#2968](#2968))

### Features

* Add MCP tool annotations to all remaining tools
([#2221](#2221))
([ea09db9](ea09db9))
* **bigquery:** Add conversational analytics tools for Data Agents
([#2517](#2517))
([2490a4b](2490a4b))
* **embeddingModel:** Add Backend API selection fields
([#2592](#2592))
([912aa9e](912aa9e))
* **skills:** Add Claude Code support to generated scripts
([#2966](#2966))
([a1609e1](a1609e1))
* **skills:** Add codex user agent
([#2973](#2973))
([070e939](070e939))
* **skills:** Tool invocation via npx
([#2916](#2916))
([377dc5b](377dc5b))
* **sources/singlestore:** Add ConnectionParams to SingleStore Config
([#2555](#2555))
([73e2a8c](73e2a8c))
* **tool/dataplex-lookup-context:** Relax project constraint and enforce
location
([#2952](#2952))
([7ebfdf1](7ebfdf1))
* **tools/looker:** Looker agent management from MCP
([#2830](#2830))
([649d4ad](649d4ad))
* **ui:** Update to use `/mcp` endpoint
([#2829](#2829))
([c3059c2](c3059c2))


### Bug Fixes

* **bigquery:** Add impersonateServiceAccount to prebuilt config
([#2770](#2770))
([9c3a748](9c3a748))
* **quickstart:** Robust tool lookup and modernize dependencies in
Python samples
([#2863](#2863))
([4c0845d](4c0845d))
* **skills:** Fix skill generation template
([#2914](#2914))
([a01a15e](a01a15e))
* **skills:** Prevent empty strings overriding optional env vars in node
scripts ([#2963](#2963))
([c52adeb](c52adeb))
* **tests/bigquery:** Implement uuid-based isolation and reliable
resource cleanup
([#2547](#2547))
([479d842](479d842))
* **tests/Bigtable:** Implement uuid-based isolation and reliable
resource cleanup
([#2880](#2880))
([a769f15](a769f15))
* Update error for ConvertConfig function
([#2993](#2993))
([62bdabb](62bdabb))


### Code Refactoring

* Update repo name
([#2968](#2968))
([3aae809](3aae809))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
github-actions bot pushed a commit that referenced this pull request Apr 8, 2026
🤖 I have created a release *beep* *boop*
---

##
[0.32.0](v0.31.0...v0.32.0)
(2026-04-08)

### ⚠ BREAKING CHANGES

* update repo name
([#2968](#2968))

### Features

* Add MCP tool annotations to all remaining tools
([#2221](#2221))
([ea09db9](ea09db9))
* **bigquery:** Add conversational analytics tools for Data Agents
([#2517](#2517))
([2490a4b](2490a4b))
* **embeddingModel:** Add Backend API selection fields
([#2592](#2592))
([912aa9e](912aa9e))
* **skills:** Add Claude Code support to generated scripts
([#2966](#2966))
([a1609e1](a1609e1))
* **skills:** Add codex user agent
([#2973](#2973))
([070e939](070e939))
* **skills:** Tool invocation via npx
([#2916](#2916))
([377dc5b](377dc5b))
* **sources/singlestore:** Add ConnectionParams to SingleStore Config
([#2555](#2555))
([73e2a8c](73e2a8c))
* **tool/dataplex-lookup-context:** Relax project constraint and enforce
location
([#2952](#2952))
([7ebfdf1](7ebfdf1))
* **tools/looker:** Looker agent management from MCP
([#2830](#2830))
([649d4ad](649d4ad))
* **ui:** Update to use `/mcp` endpoint
([#2829](#2829))
([c3059c2](c3059c2))

### Bug Fixes

* **bigquery:** Add impersonateServiceAccount to prebuilt config
([#2770](#2770))
([9c3a748](9c3a748))
* **quickstart:** Robust tool lookup and modernize dependencies in
Python samples
([#2863](#2863))
([4c0845d](4c0845d))
* **skills:** Fix skill generation template
([#2914](#2914))
([a01a15e](a01a15e))
* **skills:** Prevent empty strings overriding optional env vars in node
scripts ([#2963](#2963))
([c52adeb](c52adeb))
* **tests/bigquery:** Implement uuid-based isolation and reliable
resource cleanup
([#2547](#2547))
([479d842](479d842))
* **tests/Bigtable:** Implement uuid-based isolation and reliable
resource cleanup
([#2880](#2880))
([a769f15](a769f15))
* Update error for ConvertConfig function
([#2993](#2993))
([62bdabb](62bdabb))

### Code Refactoring

* Update repo name
([#2968](#2968))
([3aae809](3aae809))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> b9ae1c6
github-actions bot pushed a commit to nowdaysmen-sudo/genai-toolbox that referenced this pull request Apr 8, 2026
🤖 I have created a release *beep* *boop*
---

##
[0.32.0](googleapis/mcp-toolbox@v0.31.0...v0.32.0)
(2026-04-08)

### ⚠ BREAKING CHANGES

* update repo name
([googleapis#2968](googleapis#2968))

### Features

* Add MCP tool annotations to all remaining tools
([googleapis#2221](googleapis#2221))
([ea09db9](googleapis@ea09db9))
* **bigquery:** Add conversational analytics tools for Data Agents
([googleapis#2517](googleapis#2517))
([2490a4b](googleapis@2490a4b))
* **embeddingModel:** Add Backend API selection fields
([googleapis#2592](googleapis#2592))
([912aa9e](googleapis@912aa9e))
* **skills:** Add Claude Code support to generated scripts
([googleapis#2966](googleapis#2966))
([a1609e1](googleapis@a1609e1))
* **skills:** Add codex user agent
([googleapis#2973](googleapis#2973))
([070e939](googleapis@070e939))
* **skills:** Tool invocation via npx
([googleapis#2916](googleapis#2916))
([377dc5b](googleapis@377dc5b))
* **sources/singlestore:** Add ConnectionParams to SingleStore Config
([googleapis#2555](googleapis#2555))
([73e2a8c](googleapis@73e2a8c))
* **tool/dataplex-lookup-context:** Relax project constraint and enforce
location
([googleapis#2952](googleapis#2952))
([7ebfdf1](googleapis@7ebfdf1))
* **tools/looker:** Looker agent management from MCP
([googleapis#2830](googleapis#2830))
([649d4ad](googleapis@649d4ad))
* **ui:** Update to use `/mcp` endpoint
([googleapis#2829](googleapis#2829))
([c3059c2](googleapis@c3059c2))

### Bug Fixes

* **bigquery:** Add impersonateServiceAccount to prebuilt config
([googleapis#2770](googleapis#2770))
([9c3a748](googleapis@9c3a748))
* **quickstart:** Robust tool lookup and modernize dependencies in
Python samples
([googleapis#2863](googleapis#2863))
([4c0845d](googleapis@4c0845d))
* **skills:** Fix skill generation template
([googleapis#2914](googleapis#2914))
([a01a15e](googleapis@a01a15e))
* **skills:** Prevent empty strings overriding optional env vars in node
scripts ([googleapis#2963](googleapis#2963))
([c52adeb](googleapis@c52adeb))
* **tests/bigquery:** Implement uuid-based isolation and reliable
resource cleanup
([googleapis#2547](googleapis#2547))
([479d842](googleapis@479d842))
* **tests/Bigtable:** Implement uuid-based isolation and reliable
resource cleanup
([googleapis#2880](googleapis#2880))
([a769f15](googleapis@a769f15))
* Update error for ConvertConfig function
([googleapis#2993](googleapis#2993))
([62bdabb](googleapis@62bdabb))

### Code Refactoring

* Update repo name
([googleapis#2968](googleapis#2968))
([3aae809](googleapis@3aae809))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> b9ae1c6
github-actions bot pushed a commit to renovate-bot/googleapis-_-genai-toolbox that referenced this pull request Apr 8, 2026
🤖 I have created a release *beep* *boop*
---

##
[0.32.0](googleapis/mcp-toolbox@v0.31.0...v0.32.0)
(2026-04-08)

### ⚠ BREAKING CHANGES

* update repo name
([googleapis#2968](googleapis#2968))

### Features

* Add MCP tool annotations to all remaining tools
([googleapis#2221](googleapis#2221))
([ea09db9](googleapis@ea09db9))
* **bigquery:** Add conversational analytics tools for Data Agents
([googleapis#2517](googleapis#2517))
([2490a4b](googleapis@2490a4b))
* **embeddingModel:** Add Backend API selection fields
([googleapis#2592](googleapis#2592))
([912aa9e](googleapis@912aa9e))
* **skills:** Add Claude Code support to generated scripts
([googleapis#2966](googleapis#2966))
([a1609e1](googleapis@a1609e1))
* **skills:** Add codex user agent
([googleapis#2973](googleapis#2973))
([070e939](googleapis@070e939))
* **skills:** Tool invocation via npx
([googleapis#2916](googleapis#2916))
([377dc5b](googleapis@377dc5b))
* **sources/singlestore:** Add ConnectionParams to SingleStore Config
([googleapis#2555](googleapis#2555))
([73e2a8c](googleapis@73e2a8c))
* **tool/dataplex-lookup-context:** Relax project constraint and enforce
location
([googleapis#2952](googleapis#2952))
([7ebfdf1](googleapis@7ebfdf1))
* **tools/looker:** Looker agent management from MCP
([googleapis#2830](googleapis#2830))
([649d4ad](googleapis@649d4ad))
* **ui:** Update to use `/mcp` endpoint
([googleapis#2829](googleapis#2829))
([c3059c2](googleapis@c3059c2))

### Bug Fixes

* **bigquery:** Add impersonateServiceAccount to prebuilt config
([googleapis#2770](googleapis#2770))
([9c3a748](googleapis@9c3a748))
* **quickstart:** Robust tool lookup and modernize dependencies in
Python samples
([googleapis#2863](googleapis#2863))
([4c0845d](googleapis@4c0845d))
* **skills:** Fix skill generation template
([googleapis#2914](googleapis#2914))
([a01a15e](googleapis@a01a15e))
* **skills:** Prevent empty strings overriding optional env vars in node
scripts ([googleapis#2963](googleapis#2963))
([c52adeb](googleapis@c52adeb))
* **tests/bigquery:** Implement uuid-based isolation and reliable
resource cleanup
([googleapis#2547](googleapis#2547))
([479d842](googleapis@479d842))
* **tests/Bigtable:** Implement uuid-based isolation and reliable
resource cleanup
([googleapis#2880](googleapis#2880))
([a769f15](googleapis@a769f15))
* Update error for ConvertConfig function
([googleapis#2993](googleapis#2993))
([62bdabb](googleapis@62bdabb))

### Code Refactoring

* Update repo name
([googleapis#2968](googleapis#2968))
([3aae809](googleapis@3aae809))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> b9ae1c6
github-actions bot pushed a commit to Jaleel-zhu/genai-toolbox that referenced this pull request Apr 8, 2026
🤖 I have created a release *beep* *boop*
---

##
[0.32.0](googleapis/mcp-toolbox@v0.31.0...v0.32.0)
(2026-04-08)

### ⚠ BREAKING CHANGES

* update repo name
([googleapis#2968](googleapis#2968))

### Features

* Add MCP tool annotations to all remaining tools
([googleapis#2221](googleapis#2221))
([ea09db9](googleapis@ea09db9))
* **bigquery:** Add conversational analytics tools for Data Agents
([googleapis#2517](googleapis#2517))
([2490a4b](googleapis@2490a4b))
* **embeddingModel:** Add Backend API selection fields
([googleapis#2592](googleapis#2592))
([912aa9e](googleapis@912aa9e))
* **skills:** Add Claude Code support to generated scripts
([googleapis#2966](googleapis#2966))
([a1609e1](googleapis@a1609e1))
* **skills:** Add codex user agent
([googleapis#2973](googleapis#2973))
([070e939](googleapis@070e939))
* **skills:** Tool invocation via npx
([googleapis#2916](googleapis#2916))
([377dc5b](googleapis@377dc5b))
* **sources/singlestore:** Add ConnectionParams to SingleStore Config
([googleapis#2555](googleapis#2555))
([73e2a8c](googleapis@73e2a8c))
* **tool/dataplex-lookup-context:** Relax project constraint and enforce
location
([googleapis#2952](googleapis#2952))
([7ebfdf1](googleapis@7ebfdf1))
* **tools/looker:** Looker agent management from MCP
([googleapis#2830](googleapis#2830))
([649d4ad](googleapis@649d4ad))
* **ui:** Update to use `/mcp` endpoint
([googleapis#2829](googleapis#2829))
([c3059c2](googleapis@c3059c2))

### Bug Fixes

* **bigquery:** Add impersonateServiceAccount to prebuilt config
([googleapis#2770](googleapis#2770))
([9c3a748](googleapis@9c3a748))
* **quickstart:** Robust tool lookup and modernize dependencies in
Python samples
([googleapis#2863](googleapis#2863))
([4c0845d](googleapis@4c0845d))
* **skills:** Fix skill generation template
([googleapis#2914](googleapis#2914))
([a01a15e](googleapis@a01a15e))
* **skills:** Prevent empty strings overriding optional env vars in node
scripts ([googleapis#2963](googleapis#2963))
([c52adeb](googleapis@c52adeb))
* **tests/bigquery:** Implement uuid-based isolation and reliable
resource cleanup
([googleapis#2547](googleapis#2547))
([479d842](googleapis@479d842))
* **tests/Bigtable:** Implement uuid-based isolation and reliable
resource cleanup
([googleapis#2880](googleapis#2880))
([a769f15](googleapis@a769f15))
* Update error for ConvertConfig function
([googleapis#2993](googleapis#2993))
([62bdabb](googleapis@62bdabb))

### Code Refactoring

* Update repo name
([googleapis#2968](googleapis#2968))
([3aae809](googleapis@3aae809))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> b9ae1c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release candidate Use label to signal PR should be included in the next release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants