Improve environment detection for WSO2 Integrator in BallerinaExtension#1532
Improve environment detection for WSO2 Integrator in BallerinaExtension#1532kanushka merged 1 commit intowso2:release/bi-1.8.xfrom
Conversation
📝 WalkthroughWalkthroughIntroduces a pre-check in version detection to prefer BALLERINA_HOME from WSO2 Integrator environment when WSO2_INTEGRATOR_RUNTIME is set. The change short-circuits environment synchronization for this specific runtime scenario while preserving the original fallback path for other cases. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-extension/src/core/extension.ts (1)
1697-1700: Use explicit sentinel value forWSO2_INTEGRATOR_RUNTIMEto prevent accidental activation from non-standard environment valuesThe truthiness check at line 1697 means
process.env.WSO2_INTEGRATOR_RUNTIMEwill activate the WSO2 path for any non-empty string, including"false","0", or"no". Clarify intent by comparing explicitly:process.env.WSO2_INTEGRATOR_RUNTIME === "true".Regarding
BALLERINA_HOME: while the code validates it's a directory and includes abin/subdirectory (lines 1745–1748), confirming with WSO2 Integrator that this variable always points to the distribution root (not a sub-directory) prevents reliance on that downstream validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-extension/src/core/extension.ts` around lines 1697 - 1700, Change the environment check to use an explicit sentinel value instead of relying on truthiness: replace the condition using process.env.WSO2_INTEGRATOR_RUNTIME with a strict comparison process.env.WSO2_INTEGRATOR_RUNTIME === "true" so the WSO2 path only activates for that explicit value; continue to read and assign BALLERINA_HOME into ballerinaHome and set overrideBallerinaHome as before, but keep the existing directory/bin validation for BALLERINA_HOME (and add a short TODO/confirm that BALLERINA_HOME is expected to be the distribution root, not a subdirectory, if you want to avoid relying solely on downstream validation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workspaces/ballerina/ballerina-extension/src/core/extension.ts`:
- Around line 1697-1700: The current check for
process.env.WSO2_INTEGRATOR_RUNTIME unconditionally replaces the caller-supplied
ballerinaHome and flips overrideBallerinaHome, which can silently ignore an
explicit VS Code setting; modify the logic in init()/getBallerinaVersion call
site so you only set ballerinaHome = process.env.BALLERINA_HOME and
overrideBallerinaHome = true when ballerinaHome is falsy (i.e., guard with
!ballerinaHome), and if you intentionally want the env to trump a
user-configured path instead, add a clear debug/log line indicating "Overriding
configured ballerinaHome with WSO2_INTEGRATOR_RUNTIME value" before changing
ballerinaHome to make the precedence visible.
---
Nitpick comments:
In `@workspaces/ballerina/ballerina-extension/src/core/extension.ts`:
- Around line 1697-1700: Change the environment check to use an explicit
sentinel value instead of relying on truthiness: replace the condition using
process.env.WSO2_INTEGRATOR_RUNTIME with a strict comparison
process.env.WSO2_INTEGRATOR_RUNTIME === "true" so the WSO2 path only activates
for that explicit value; continue to read and assign BALLERINA_HOME into
ballerinaHome and set overrideBallerinaHome as before, but keep the existing
directory/bin validation for BALLERINA_HOME (and add a short TODO/confirm that
BALLERINA_HOME is expected to be the distribution root, not a subdirectory, if
you want to avoid relying solely on downstream validation).
| if (process.env.WSO2_INTEGRATOR_RUNTIME && process.env.BALLERINA_HOME) { | ||
| debug(`[VERSION] Detected WSO2 Integrator environment with BALLERINA_HOME: ${process.env.BALLERINA_HOME}`); | ||
| ballerinaHome = process.env.BALLERINA_HOME; | ||
| overrideBallerinaHome = true; |
There was a problem hiding this comment.
WSO2 env vars unconditionally override an explicitly caller-supplied ballerinaHome
When init() has already set this.ballerinaHome to a user-configured path (because OVERRIDE_BALLERINA_HOME is true in VS Code settings), it passes that non-empty value as ballerinaHome to getBallerinaVersion. The new condition at line 1697 unconditionally replaces that value with process.env.BALLERINA_HOME — silently ignoring the user's explicit VS Code configuration — and flips overrideBallerinaHome without any diagnostic message beyond the single debug line. A user who has a VS Code-configured home and WSO2_INTEGRATOR_RUNTIME set in their global environment would get the WSO2 path with no indication of why their setting was ignored.
Guarding the override behind !ballerinaHome makes the intended precedence explicit (env var wins only when no home was already resolved):
💡 Proposed fix
- if (process.env.WSO2_INTEGRATOR_RUNTIME && process.env.BALLERINA_HOME) {
+ if (process.env.WSO2_INTEGRATOR_RUNTIME && process.env.BALLERINA_HOME && !ballerinaHome) {
debug(`[VERSION] Detected WSO2 Integrator environment with BALLERINA_HOME: ${process.env.BALLERINA_HOME}`);
ballerinaHome = process.env.BALLERINA_HOME;
overrideBallerinaHome = true;Alternatively, if WSO2 Integrator is intentionally designed to always win over user configuration, add a debug/log statement so the override is visible:
if (process.env.WSO2_INTEGRATOR_RUNTIME && process.env.BALLERINA_HOME) {
debug(`[VERSION] Detected WSO2 Integrator environment with BALLERINA_HOME: ${process.env.BALLERINA_HOME}`);
+ if (ballerinaHome && ballerinaHome !== process.env.BALLERINA_HOME) {
+ debug(`[VERSION] WSO2 Integrator BALLERINA_HOME overrides configured home: '${ballerinaHome}' → '${process.env.BALLERINA_HOME}'`);
+ }
ballerinaHome = process.env.BALLERINA_HOME;
overrideBallerinaHome = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@workspaces/ballerina/ballerina-extension/src/core/extension.ts` around lines
1697 - 1700, The current check for process.env.WSO2_INTEGRATOR_RUNTIME
unconditionally replaces the caller-supplied ballerinaHome and flips
overrideBallerinaHome, which can silently ignore an explicit VS Code setting;
modify the logic in init()/getBallerinaVersion call site so you only set
ballerinaHome = process.env.BALLERINA_HOME and overrideBallerinaHome = true when
ballerinaHome is falsy (i.e., guard with !ballerinaHome), and if you
intentionally want the env to trump a user-configured path instead, add a clear
debug/log line indicating "Overriding configured ballerinaHome with
WSO2_INTEGRATOR_RUNTIME value" before changing ballerinaHome to make the
precedence visible.
Purpose
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit