-
-
Notifications
You must be signed in to change notification settings - Fork 193
Add env parameter to XcodeBuildMCP launch tools for passing environment variables #203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for this, will look at this shortly |
commit: |
9c58546 to
de741bc
Compare
WalkthroughThe pull request adds optional environment variable support across app launching functionality. It introduces an 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcp/tools/device/launch_app_device.ts (1)
131-131:⚠️ Potential issue | 🟡 MinorEmoji in code string violates coding guidelines.
Line 131 contains a
✅emoji in the response text. As per coding guidelines,**/*.{ts,tsx,js,jsx,json,md}: "No emojis in commits, issues, PR comments, or code".
🧹 Nitpick comments (2)
src/mcp/tools/device/__tests__/launch_app_device.test.ts (1)
139-139:any[]could be replaced with a typed array.Consider using a more specific type (e.g.
{ command: string[] }[]) instead ofany[]for thecallstracking array, consistent with the coding guideline against unnecessaryanytypes. This also applies to existing instances on lines 55 and 102, though those predate this PR.As per coding guidelines,
**/*.{ts,tsx}: "Noanytypes unless absolutely necessary in TypeScript code".src/utils/environment.ts (1)
92-118: Consider extracting a shared helper to reduce duplication.
normalizeTestRunnerEnvandnormalizeSimctlChildEnvare identical aside from the prefix string. A genericnormalizePrefixedEnv(vars, prefix)helper would eliminate the duplication.♻️ Suggested refactor
+function normalizePrefixedEnv(vars: Record<string, string>, prefix: string): Record<string, string> { + const normalized: Record<string, string> = {}; + for (const [key, value] of Object.entries(vars ?? {})) { + if (value == null) continue; + const prefixedKey = key.startsWith(prefix) ? key : `${prefix}${key}`; + normalized[prefixedKey] = value; + } + return normalized; +} + export function normalizeTestRunnerEnv(vars: Record<string, string>): Record<string, string> { - const normalized: Record<string, string> = {}; - for (const [key, value] of Object.entries(vars ?? {})) { - if (value == null) continue; - const prefixedKey = key.startsWith('TEST_RUNNER_') ? key : `TEST_RUNNER_${key}`; - normalized[prefixedKey] = value; - } - return normalized; + return normalizePrefixedEnv(vars, 'TEST_RUNNER_'); } export function normalizeSimctlChildEnv(vars: Record<string, string>): Record<string, string> { - const normalized: Record<string, string> = {}; - for (const [key, value] of Object.entries(vars ?? {})) { - if (value == null) continue; - const prefixedKey = key.startsWith('SIMCTL_CHILD_') ? key : `SIMCTL_CHILD_${key}`; - normalized[prefixedKey] = value; - } - return normalized; + return normalizePrefixedEnv(vars, 'SIMCTL_CHILD_'); }
Summary
envparameter tolaunch_app_sim,launch_app_logs_sim, andlaunch_app_devicetools so environment variables can be passed to launched appsnormalizeSimctlChildEnvutility (mirrors existingnormalizeTestRunnerEnv) that auto-prefixes keys withSIMCTL_CHILD_--environment-variables KEY=VALUEflags todevicectlMotivation
Launching apps against staging/dev environments requires passing environment variables to the app process. Previously this meant falling back to raw
simctlcommands (SIMCTL_CHILD_X=1 xcrun simctl launch ...). Now all three launch tools accept anenvparameter directly.