Skip to content

Conversation

@NamesMT
Copy link
Contributor

@NamesMT NamesMT commented Jun 7, 2025

Related GitHub Issue

Closes: #4017

Description

I added a new utility: injectVariables(config, vars), and used it

Test Procedure

I wrote some unit tests and used Extension Debug mode to test the real usage, confirming everything works as expected.

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

Additional Notes

Get in Touch

NamesMT / @topnames_mt


Important

Adds injectVariables utility to inject variables into configurations, replacing injectEnv in McpHub.ts, with tests in config.test.ts.

  • New Functionality:
    • Adds injectVariables(config, vars) in config.ts to inject variables into configurations, supporting nested variables and handling null or undefined values.
    • Replaces injectEnv with injectVariables in McpHub.ts to inject environment and magic variables (e.g., workspaceFolder).
  • Testing:
    • Adds unit tests for injectVariables in config.test.ts, covering variable replacement and handling of undefined or empty values.
    • Updates existing tests for injectEnv to reflect changes in variable handling.

This description was created by Ellipsis for 5789a3e15c4a4399cb248a5eb1847b3e59c43988. You can customize this summary. It will automatically update as commits are pushed.

@NamesMT NamesMT requested review from cte, jr and mrubens as code owners June 7, 2025 09:50
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jun 7, 2025
@adamhill
Copy link
Contributor

adamhill commented Jun 7, 2025

Coolio!

Dockerized MCP Servers for Roo Marketplace FTW!!!

Now if we can figure out how to compose multiples of those in the Docker MCP ecosystem... bobs our uncle!

@daniel-lxs daniel-lxs added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 7, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 8, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

Hey @NamesMT, Thank you for your contribution!

I left a minor suggestion but looks good to me!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 8, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 8, 2025
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 8, 2025
@NamesMT
Copy link
Contributor Author

NamesMT commented Jun 8, 2025

Hi @daniel-lxs, please help me check the latest state, I've added InjectableConfigType to deeply describe the valid config parameter and update the tests to be a bit more thorough too.
I also properly use the defined C type for config, previously I forgot to use it and redeclare the type.

@daniel-lxs daniel-lxs added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 9, 2025
NamesMT and others added 5 commits June 12, 2025 12:01
Previously this is intended so that the CLI receives a correct empty path argument, but on a second thought, if the user have added the quotes themselves it might cause error.

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@daniel-lxs daniel-lxs force-pushed the feat/injectVariables branch from 5a1dd25 to 99ebb51 Compare June 12, 2025 17:03
@mrubens mrubens merged commit 84ccf3f into RooCodeInc:main Jun 12, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 12, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 12, 2025
cte pushed a commit that referenced this pull request Jun 24, 2025
…kspaceFolder`) (#4442)

* feat: add `injectVariables()`, support magic variables for MCPs

* fix: fallback for `workspaceFolder` should just be an empty string

Previously this is intended so that the CLI receives a correct empty path argument, but on a second thought, if the user have added the quotes themselves it might cause error.

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

* chore: remove unused import

* chore: better log format

* chore: better describe the accepted config type and more extensive test

---------

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

MCP configuration Environment Variable expansion does not support ${workspaceFolder}

4 participants