[BI] Fix support for Ballerina installation paths containing spaces#1551
Conversation
📝 WalkthroughWalkthroughAdds a new quoteShellPath utility and applies it across the extension to quote executable paths when building shell command strings, updating command construction in the core extension, test runner, debugger, project runner, testing runner, and migration tool. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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
🤖 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/utils/config.ts`:
- Around line 76-82: The quoteShellPath function currently double-quotes
already-quoted paths and does not escape embedded double-quotes; update
quoteShellPath to first normalize the input by trimming whitespace and removing
a single pair of surrounding quotes if present, then escape any internal
double-quote characters (e.g., replace " with \"), and finally wrap the
normalized/escaped path in a single pair of double quotes before returning;
ensure this logic is implemented inside the quoteShellPath function so existing
quoted inputs are not double-quoted and embedded quotes are escaped.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
workspaces/ballerina/ballerina-extension/src/core/extension.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/test-runner.tsworkspaces/ballerina/ballerina-extension/src/features/debugger/config-provider.tsworkspaces/ballerina/ballerina-extension/src/features/project/cmds/cmd-runner.tsworkspaces/ballerina/ballerina-extension/src/features/testing/runner.tsworkspaces/ballerina/ballerina-extension/src/utils/config.tsworkspaces/ballerina/ballerina-extension/src/utils/migrate-integration.ts
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where Ballerina installation paths containing spaces would cause command execution failures. The fix implements a utility function to quote shell paths when they contain spaces.
Changes:
- Added
quoteShellPath()utility function to wrap paths containing spaces in double quotes - Updated all Ballerina command execution points to use the new quoting function
- Modified version detection logic to properly handle quoted paths
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| workspaces/ballerina/ballerina-extension/src/utils/config.ts | Adds quoteShellPath() utility function to handle paths with spaces |
| workspaces/ballerina/ballerina-extension/src/utils/migrate-integration.ts | Applies path quoting to migration tool pull command |
| workspaces/ballerina/ballerina-extension/src/features/testing/runner.ts | Applies path quoting to test execution command |
| workspaces/ballerina/ballerina-extension/src/features/project/cmds/cmd-runner.ts | Applies path quoting to project command execution |
| workspaces/ballerina/ballerina-extension/src/features/debugger/config-provider.ts | Applies path quoting to debugger adapter and run commands |
| workspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/test-runner.ts | Applies path quoting to AI agent test runner command |
| workspaces/ballerina/ballerina-extension/src/core/extension.ts | Refactors version detection to build executable path separately before quoting |
Comments suppressed due to low confidence (1)
workspaces/ballerina/ballerina-extension/src/utils/config.ts:82
- If the
filePathalready contains double quotes, this function will produce incorrectly nested quotes (e.g.,\"path with \"quotes\" \") which will fail shell parsing. The function should either escape existing quotes or check for them before wrapping.
export function quoteShellPath(filePath: string): string {
return filePath.includes(' ') ? `"${filePath}"` : filePath;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
workspaces/ballerina/ballerina-extension/src/utils/config.ts (1)
81-94:⚠️ Potential issue | 🟠 MajorHandle single‑quoted and whitespace‑padded inputs in
quoteShellPath.Right now only double quotes are stripped and only literal spaces are checked. If users set
'C:\Program Files\bal.bat'(or have leading/trailing spaces), the function will wrap those quotes and the command will likely fail. Consider normalizing both quote styles, trimming, and checking any whitespace.🛠️ Suggested fix
export function quoteShellPath(filePath: string): string { - // Strip existing surrounding quotes to normalize - let normalized = filePath; - if (normalized.length >= 2 && normalized.startsWith('"') && normalized.endsWith('"')) { - normalized = normalized.slice(1, -1); - } - - if (!normalized.includes(' ')) { - return normalized; - } - - // Escape any embedded double quotes - const escaped = normalized.replace(/"/g, '\\"'); - return `"${escaped}"`; + const trimmed = filePath.trim(); + let normalized = trimmed; + if ( + normalized.length >= 2 && + ((normalized.startsWith('"') && normalized.endsWith('"')) || + (normalized.startsWith("'") && normalized.endsWith("'"))) + ) { + normalized = normalized.slice(1, -1); + } + + const escaped = normalized.replace(/"/g, '\\"'); + return /\s/.test(escaped) ? `"${escaped}"` : escaped; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/ballerina/ballerina-extension/src/utils/config.ts` around lines 81 - 94, The quoteShellPath function currently only strips surrounding double quotes and checks for literal spaces, so inputs with single quotes or leading/trailing whitespace slip through; update quoteShellPath to first trim() the input, then detect and strip a single pair of surrounding quotes of either type (single or double) into the normalized variable, then check for any whitespace using a regex like /\s/ (not just ' ') and only if whitespace exists escape embedded double quotes (escaped) and wrap with double quotes before returning; reference quoteShellPath, normalized and escaped when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@workspaces/ballerina/ballerina-extension/src/utils/config.ts`:
- Around line 81-94: The quoteShellPath function currently only strips
surrounding double quotes and checks for literal spaces, so inputs with single
quotes or leading/trailing whitespace slip through; update quoteShellPath to
first trim() the input, then detect and strip a single pair of surrounding
quotes of either type (single or double) into the normalized variable, then
check for any whitespace using a regex like /\s/ (not just ' ') and only if
whitespace exists escape embedded double quotes (escaped) and wrap with double
quotes before returning; reference quoteShellPath, normalized and escaped when
making these changes.
Purpose
Fixes wso2/product-ballerina-integrator#2550
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