-
Notifications
You must be signed in to change notification settings - Fork 59
Add WSO2 integrator #901
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
Add WSO2 integrator #901
Conversation
… wso2-integrator
…nsions into wso2-integrator
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/mi/mi-extension/package.json (1)
22-24: Add missing WI extension dependency to extensionDependencies.The MI extension contributes
viewsWelcomeentries to thewso2-integrator.explorerview (lines 228, 233, 238), which is provided by the WI extension (wso2.wso2-integrator). However,extensionDependencies(lines 22–24) does not declare this dependency. This risks undefined view rendering if the WI extension is not installed or hasn't activated yet.Add
"wso2.wso2-integrator"toextensionDependencies:"extensionDependencies": [ - "wso2.wso2-platform" + "wso2.wso2-platform", + "wso2.wso2-integrator" ],
♻️ Duplicate comments (7)
workspaces/mi/mi-extension/package.json (1)
228-241: New viewsWelcome entries targeting wso2-integrator.explorer require dependency fix.Lines 228–241 add three new
viewsWelcomeentries targetingwso2-integrator.explorer. These entries mirror the content and logic of the existing MI-specific entries but with a condition gating them to when WI is active (config.integrator.defaultIntegrator != ''). This separation is architecturally sound and enables per-integrator messaging.However, this change depends on resolving the
extensionDependenciesissue flagged above. Once the WI extension dependency is added, these entries should render correctly.workspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.ts (4)
25-32: Unresolved: Replace "Ballerina" branding with WSO2 Integrator naming.The interface name, output channel label, and configuration namespace still reference "Ballerina" instead of WSO2 Integrator. This issue was previously flagged but remains unaddressed.
41-43: Unresolved: Configuration namespace still uses 'ballerina'.The configuration namespace should be updated to match the WSO2 Integrator extension context (e.g., 'wso2Integrator' or similar). This was flagged in previous reviews but remains unchanged.
45-61: Unresolved: Update comments and consider async logging for performance.The inline comments at lines 45 and 55 still reference "Ballerina output channel" instead of "WSO2 Integrator output channel". Additionally, the synchronous file I/O in
persistDebugLogs(called on lines 52 and 60) could impact performance with frequent logging. Both concerns were raised in previous reviews.
78-78: Log folder path also requires branding update.The log folder path uses
.ballerinadirectory name. When addressing the broader branding issues, ensure this path is also updated to reflect WSO2 Integrator naming (e.g.,.wso2-integratoror similar).workspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.ts (1)
35-63: Critical: Command injection vulnerability via unvalidatedversionparameterThe
versionparameter is not validated before being interpolated into a shell command at line 62. WhilemigrationToolNameis constrained by the whitelist, an attacker could inject shell metacharacters throughversion(e.g.,"1.0.0; rm -rf /") to execute arbitrary commands.Solution 1 (recommended): Use
spawnwith an args array to avoid shell interpolation:-import { exec } from "child_process"; +import { spawn } from "child_process";Then replace the exec call (lines 84-87) with:
- const childProcess = exec(command, { - maxBuffer: 1024 * 1024, - timeout: 300000 // 5 minutes timeout - }); + const childProcess = spawn(ballerinaCmd, ["tool", "pull", `${migrationToolName}:${version}`], { + timeout: 300000 // 5 minutes timeout + });Solution 2 (alternative): Strictly validate
versionbefore constructing the command:if (!version) { const errorMessage = "Migration tool version is required"; return Promise.reject(new Error(errorMessage)); } + +// Validate version format to prevent command injection +const versionRegex = /^[0-9A-Za-z._-]+$/; +if (!versionRegex.test(version)) { + const errorMessage = `Invalid version format: ${version}`; + return Promise.reject(new Error(errorMessage)); +}Note: If switching to
spawn, you'll also need to adjust how stdout/stderr are handled sincespawnreturns a stream-based interface.workspaces/wi/wi-extension/src/stateMachine.ts (1)
218-226: Critical: Ballerina extension exports accessed without activation check.The code accesses
ballerinaExt.exports.ballerinaExtInstance(lines 223-225) without verifying the extension is activated or that exports exist. If the Ballerina extension is installed but not yet activated,exportswill be undefined, causing runtime errors.Add activation and existence checks:
if (projectInfo.isBallerina && ballerinaExt) { ext.log('Detected BI/Ballerina project'); + + // Ensure extension is activated + if (!ballerinaExt.isActive) { + await ballerinaExt.activate(); + } + + // Verify API is available + if (!ballerinaExt.exports?.ballerinaExtInstance) { + ext.logError('Ballerina extension API not available', new Error('Missing ballerinaExtInstance')); + return { projectType: ProjectType.NONE }; + } // Initialize Ballerina extension context extension.context = ext.context; extension.langClient = ballerinaExt.exports.ballerinaExtInstance.langClient; extension.biSupported = ballerinaExt.exports.ballerinaExtInstance.biSupported; extension.isNPSupported = ballerinaExt.exports.ballerinaExtInstance.isNPSupported;
🧹 Nitpick comments (3)
workspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.ts (1)
69-74: Clarify documentation: logs are retained by file count, not days.The comment states "keeping logs for up to 10 days" but the implementation maintains up to 10 log files. While typically one file per day, this isn't guaranteed. Consider updating the documentation to accurately reflect the file-count-based retention policy.
Apply this diff:
/** - * Persist debug logs to a file, keeping logs for up to 10 days. + * Persist debug logs to a file, keeping up to 10 log files. * Each day's logs are stored in a file named with the current date (YYYY-MM-DD.log). - * When more than 10 log files exist, delete the oldest one. + * When more than 10 log files exist, the oldest file is deleted. * Each log entry is appended to the corresponding day's file, prefixed with the current date and time. */workspaces/wi/wi-extension/src/stateMachine.ts (2)
164-176: Consider consolidating duplicate initialization logic.Both the
BI_BALLERINAandMIbranches execute identicalawait context.extensionAPIs.initialize()calls (lines 171, 175). If no project-type-specific initialization is planned, this can be simplified:- // Initialize extension APIs and activate appropriate extensions based on project type - if (context.projectType === ProjectType.BI_BALLERINA) { - // Activate only BI extension for Ballerina projects - ext.log('Initializing BI extension for Ballerina project'); - await context.extensionAPIs.initialize(); - } else if (context.projectType === ProjectType.MI) { - // Activate only MI extension for MI projects - ext.log('Initializing MI extension for MI project'); - await context.extensionAPIs.initialize(); - } + // Initialize extension APIs for known project types + if (context.projectType !== ProjectType.NONE) { + ext.log(`Initializing extension for ${context.projectType} project`); + await context.extensionAPIs.initialize(); + }However, if you anticipate project-type-specific initialization logic in the future, the current structure is acceptable.
243-243: Minor: Unnecessary fallback forextensionAPIs.Line 243 includes a fallback
context.extensionAPIs || new ExtensionAPIs(), butextensionAPIsis a required field inMachineContext(line 43) and always initialized in the state machine context (line 72). The fallback is unreachable.Consider simplifying to:
-const extensionAPIs = context.extensionAPIs || new ExtensionAPIs(); +const extensionAPIs = context.extensionAPIs;This is a very minor nitpick and doesn't affect functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
workspaces/api-designer/api-designer-extension/package.json(1 hunks)workspaces/apk/apk-extension/package.json(1 hunks)workspaces/ballerina/ballerina-extension/package.json(1 hunks)workspaces/bi/bi-extension/package.json(1 hunks)workspaces/common-libs/scripts/package-vsix.js(1 hunks)workspaces/mcp-inspector/mcp-inspector-extension/package.json(1 hunks)workspaces/mi/mi-extension/package.json(2 hunks)workspaces/mi/mi-extension/src/stateMachine.ts(9 hunks)workspaces/wi/wi-extension/package.json(1 hunks)workspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.ts(1 hunks)workspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.ts(1 hunks)workspaces/wi/wi-extension/src/stateMachine.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workspaces/wi/wi-extension/package.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/ballerina/ballerina-extension/package.jsonworkspaces/bi/bi-extension/package.jsonworkspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.tsworkspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.tsworkspaces/wi/wi-extension/src/stateMachine.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/ballerina/ballerina-extension/package.jsonworkspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.tsworkspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.tsworkspaces/wi/wi-extension/src/stateMachine.ts
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/ballerina/ballerina-extension/package.jsonworkspaces/wi/wi-extension/src/stateMachine.ts
🧬 Code graph analysis (4)
workspaces/mi/mi-extension/src/stateMachine.ts (1)
workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)
activateProjectExplorer(40-722)
workspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.ts (1)
workspaces/mcp-inspector/mcp-inspector-extension/src/utils/logger.ts (1)
error(24-26)
workspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.ts (6)
workspaces/wi/wi-core/src/rpc-types/main/rpc-type.ts (2)
pullMigrationTool(43-43)onDownloadProgress(38-38)workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts (1)
pullMigrationTool(295-302)workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts (1)
pullMigrationTool(138-140)workspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.ts (1)
debug(46-53)workspaces/wi/wi-core/src/types/rpc.types.ts (1)
DownloadProgress(134-141)workspaces/wi/wi-rpc-client/src/RpcClient.ts (1)
onDownloadProgress(41-43)
workspaces/wi/wi-extension/src/stateMachine.ts (9)
workspaces/wi/wi-extension/src/extensionAPIs.ts (1)
ExtensionAPIs(27-113)workspaces/wi/wi-extension/src/webviewManager.ts (1)
WebviewManager(30-259)workspaces/wi/wi-extension/src/extensionVariables.ts (1)
ext(59-59)workspaces/wi/wi-extension/src/bi/project-explorer/activate.ts (1)
activateBIProjectExplorer(31-47)workspaces/wi/wi-core/src/constants.ts (1)
CONTEXT_KEYS(49-52)workspaces/wi/wi-extension/src/commands.ts (1)
registerCommands(29-117)workspaces/wi/wi-extension/src/mi/utils.ts (1)
checkIfMiProject(24-44)workspaces/wi/wi-extension/src/bi/utils.ts (2)
ProjectInfo(23-27)fetchProjectInfo(36-57)workspaces/wi/wi-extension/src/bi/biExtentionContext.ts (1)
extension(30-30)
🔇 Additional comments (11)
workspaces/bi/bi-extension/package.json (1)
165-165: LGTM!The migration to the centralized packaging script simplifies maintenance and ensures consistent packaging behavior across extensions.
workspaces/ballerina/ballerina-extension/package.json (1)
1190-1190: LGTM!Consistent migration to the centralized packaging script.
workspaces/mcp-inspector/mcp-inspector-extension/package.json (1)
63-63: LGTM!Consistent with the centralized packaging approach.
workspaces/api-designer/api-designer-extension/package.json (1)
86-86: LGTM!Consistent with the centralized packaging approach.
workspaces/apk/apk-extension/package.json (1)
54-54: LGTM!Correctly integrates the centralized packaging script into this extension's build workflow.
workspaces/mi/mi-extension/package.json (3)
190-204: Visibility predicates added correctly for MI views.The
whenconditions gatingMI.project-explorerandMI.mock-servicesto!config.integrator.defaultIntegratorproperly hide these views when using the unified WI integrator. This aligns with the PR's goal of conditionally showing MI extension activity items.
206-226: Verify when conditions for existing viewsWelcome entries.The existing
viewsWelcomeentries (lines 207–226) now include!config.integrator.defaultIntegratorconditions to hide them when WI is active. These condition updates look syntactically correct and logically sound. However, ensure that:
- The
config.integrator.defaultIntegratorsetting is properly registered and exposed by the WI extension.- The status values (
unknownProject,projectLoaded,disabled,notSetUp) are already defined in the MI context or other extensions.- Short-circuiting logic (e.g.,
condition1 && (condition2 && condition3 && ...)) doesn't inadvertently hide content when conditions are not yet evaluated.
977-977: Centralized packaging script verified and fully functional.All verification checks passed:
- Script exists at the correct relative path:
workspaces/common-libs/scripts/package-vsix.js- Script is compatible: wraps
vsce packageand the extension has all required metadata (name, version, publisher, displayName, vsce dependency @3.4.2)- Extension metadata is properly configured and will be correctly handled by vsce
workspaces/mi/mi-extension/src/stateMachine.ts (2)
105-105: LGTM! Correctly defers welcome screen to WI.The condition
!context.isInWIappropriately prevents MI from showing its welcome screen when running inside the WI extension, avoiding duplicate UIs.
657-658: LGTM! Dynamic tree view selection is implemented correctly.The logic correctly selects the appropriate tree view ID based on the WI embedding context, ensuring MI integrates properly with the unified explorer when running inside WI.
workspaces/wi/wi-extension/src/stateMachine.ts (1)
260-280: LGTM! Configuration change handling is well-implemented.The dynamic configuration listener (lines 265-276) correctly:
- Watches for changes to
wso2-integrator.integrator.defaultIntegrator- Updates the state machine context via
UPDATE_MODEevent- Properly registers disposal with
ext.context.subscriptionsThis enables runtime mode switching without requiring extension restart.
workspaces/wi/wi-extension/src/rpc-managers/main/ballerinaLogger.ts
Outdated
Show resolved
Hide resolved
| const childProcess = exec(command, { | ||
| maxBuffer: 1024 * 1024, | ||
| timeout: 300000 // 5 minutes timeout | ||
| }); |
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.
Major: Timeout handling is broken—the documented 5-minute guarantee does not work
The exec function's timeout option (line 86) kills the child process and reports the timeout via the callback with error.code === "ETIMEDOUT". It does not emit a "timeout" event on the ChildProcess instance. Since no callback is provided to exec, the timeout handler at lines 259-269 will never fire, and callers won't see the intended "Download timed out after 5 minutes" message.
Solution 1 (recommended): Implement an explicit timeout with setTimeout:
return new Promise<void>((resolve, reject) => {
+ let timeoutHandle: NodeJS.Timeout | undefined;
+
// Helper to send notifications to the webview
const sendProgress = (progress: DownloadProgress) => {
RPCLayer._messengers.get("wi-webview")?.sendNotification(
@@ -84,9 +86,21 @@
const childProcess = exec(command, {
- maxBuffer: 1024 * 1024,
- timeout: 300000 // 5 minutes timeout
+ maxBuffer: 1024 * 1024
});
+ // Set up explicit 5-minute timeout
+ timeoutHandle = setTimeout(() => {
+ childProcess.kill();
+ const errorMessage = "Download timed out after 5 minutes";
+ sendProgress({
+ message: errorMessage,
+ success: false,
+ step: -1,
+ });
+ reject(new Error("Migration tool pull timed out after 5 minutes"));
+ }, 300000);
+
let accumulatedStdout = "";Then clear the timeout in the "close" handler:
childProcess.on("close", (code) => {
+ if (timeoutHandle) clearTimeout(timeoutHandle);
debug(`Tool pull command exited with code ${code}`);And remove the now-unnecessary "timeout" event handler (lines 259-269).
Solution 2 (alternative): Use the exec callback to detect timeout errors:
- const childProcess = exec(command, {
- maxBuffer: 1024 * 1024,
- timeout: 300000 // 5 minutes timeout
- });
+ const childProcess = exec(command, {
+ maxBuffer: 1024 * 1024,
+ timeout: 300000
+ }, (error, stdout, stderr) => {
+ if (error && error.code === "ETIMEDOUT") {
+ const errorMessage = "Download timed out after 5 minutes";
+ sendProgress({
+ message: errorMessage,
+ success: false,
+ step: -1,
+ });
+ reject(new Error("Migration tool pull timed out after 5 minutes"));
+ }
+ });Then remove the "timeout" event handler (lines 259-269).
Also applies to: 259-269
🤖 Prompt for AI Agents
In workspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.ts
around lines 84-87 and 259-269, the exec call uses the timeout option but no
callback, so the ChildProcess will never emit a "timeout" event and your
existing timeout handler (lines 259-269) will never run; replace that approach
by creating an explicit setTimeout after spawning exec that kills the child
process and logs "Download timed out after 5 minutes", clear that timer in the
childProcess 'close' (and/or 'exit') handler, and remove the unused "timeout"
event handler at lines 259-269 (alternatively, if you prefer callbacks, pass a
callback to exec and detect error.code === "ETIMEDOUT" and remove the "timeout"
event handler).
| childProcess.stderr?.on("data", (data: Buffer) => { | ||
| const errorOutput = data.toString().trim(); | ||
| debug(`Tool pull stderr: ${errorOutput}`); | ||
|
|
||
| // Filter out non-critical messages that shouldn't cause failure | ||
| const nonCriticalPatterns = [ | ||
| /is already active/i, | ||
| /warning:/i, | ||
| /deprecated/i | ||
| ]; | ||
|
|
||
| const isNonCritical = nonCriticalPatterns.some(pattern => pattern.test(errorOutput)); | ||
|
|
||
| if (isNonCritical) { | ||
| debug(`Ignoring non-critical stderr: ${errorOutput}`); | ||
| return; | ||
| } | ||
|
|
||
| // Only treat as error if it's a real error message | ||
| if (errorOutput.length > 0) { | ||
| sendProgress({ | ||
| message: `Error: ${errorOutput}`, | ||
| success: false, | ||
| step: -1, | ||
| }); | ||
| reject(new Error(errorOutput)); | ||
| } | ||
| }); |
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.
Major: Multiple reject calls can cause confusing progress messages
The current implementation can call reject() multiple times from different handlers:
- Line 203: Rejects immediately on non-filtered stderr output
- Line 241: Rejects on non-zero exit code in the
"close"handler - Line 255: Rejects on process errors in the
"error"handler
While the Promise will ignore subsequent reject() calls after the first, each handler sends its own progress message (lines 198-202, 236-240, 250-254), which can result in conflicting UI notifications being displayed to the user.
Recommended solution: Track completion with a flag to prevent multiple terminal outcomes:
return new Promise<void>((resolve, reject) => {
+ let completed = false;
+
// Helper to send notifications to the webview
const sendProgress = (progress: DownloadProgress) => {
+ if (completed) return;
RPCLayer._messengers.get("wi-webview")?.sendNotification(
onDownloadProgress,
{ type: 'webview', webviewType: 'wso2IntegratorWelcome' },Then guard each terminal handler:
childProcess.stderr?.on("data", (data: Buffer) => {
+ if (completed) return;
const errorOutput = data.toString().trim();
// ... filtering logic ...
if (errorOutput.length > 0) {
+ completed = true;
sendProgress({ /* ... */ });
reject(new Error(errorOutput));
}
});
childProcess.on("close", (code) => {
+ if (completed) return;
debug(`Tool pull command exited with code ${code}`);
// ... success/failure logic ...
if (code === 0 || (code === 1 && isAlreadyInstalled)) {
+ completed = true;
sendProgress({ /* ... */ });
resolve();
} else {
+ completed = true;
sendProgress({ /* ... */ });
reject(new Error(errorMessage));
}
});
childProcess.on("error", (error) => {
+ if (completed) return;
+ completed = true;
// ... rest of handler ...
});Additionally, consider treating stderr as non-fatal until the "close" handler confirms a non-zero exit code, since some CLIs output informational messages or warnings to stderr that don't indicate actual failure.
Also applies to: 208-243, 246-256
🤖 Prompt for AI Agents
In workspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.ts
around lines 178 to 205 (and similarly covering the close and error handlers
later), avoid sending multiple terminal progress messages and calling reject
multiple times by introducing a local boolean "completed" flag; on entering any
terminal path (stderr-based failure, "close" with non-zero exit code, or "error"
event) first check and set completed and only then sendProgress and call
reject/resolve. Change stderr handling to not immediately reject: accumulate or
store last stderr output and log/debug or send a non-terminal warning, and only
treat it as fatal in the "close" handler if the process exit code is non-zero
(using the stored stderr text in the failure message). Ensure the "error" and
"close" handlers also respect the same completed flag so only the first terminal
outcome sends UI progress and triggers reject/resolve.
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: 1
♻️ Duplicate comments (3)
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts (2)
169-195: Consider mapping additionalCreateMiProjectRequestfields.The method only forwards
name,path,scope, andopento the MI command, butCreateMiProjectRequestincludesgroupID,artifactID,version, andmiVersionfields. If the underlyingMI.project-explorer.create-projectcommand supports these fields (or will in the future), they should be passed through.Verify whether the MI command accepts these additional parameters:
#!/bin/bash # Search for the MI command handler to check its parameter interface rg -n "MI\.project-explorer\.create-project" --type=ts -B5 -A10
60-66: Fix method signatures to matchWIVisualizerAPIinterface.These methods are declared as
asyncreturningPromise<void>, butWIVisualizerAPIexpects synchronous() => voidmethods. The RPC types define these asNotificationType<void>(fire-and-forget notifications), not requests that return promises.Remove
asyncandPromise<void>:- async openBiExtension(): Promise<void> { + openBiExtension(): void { commands.executeCommand("wso2.integrator.openBIIntegration"); } - async openMiExtension(): Promise<void> { + openMiExtension(): void { commands.executeCommand("wso2.integrator.openMIIntegration"); }workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts (1)
78-84: Removereturnstatements from void notification methods.Although a previous comment indicated this was addressed in commit 852c401, the code still contains
returnstatements in void methods. ThesendNotificationcalls should not be returned since these are fire-and-forget notifications and the return type isvoid.openBiExtension(): void { - return this._messenger.sendNotification(openBiExtension, HOST_EXTENSION); + this._messenger.sendNotification(openBiExtension, HOST_EXTENSION); } openMiExtension(): void { - return this._messenger.sendNotification(openMiExtension, HOST_EXTENSION); + this._messenger.sendNotification(openMiExtension, HOST_EXTENSION); }
🧹 Nitpick comments (3)
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts (2)
123-129: Implement actual MI version retrieval logic.The method returns placeholder data instead of actual supported MI versions. This could cause issues if the webview or other consumers rely on accurate version information.
Do you want me to help locate the actual implementation in the ballerina-core or MI extension that should be called here?
337-354: Remove redundant dynamic import.The
vscodemodule is already imported at the top of the file (line 47), so the dynamic import on line 338 is unnecessary and adds overhead.async saveMigrationReport(params: SaveMigrationReportRequest): Promise<void> { - const vscode = await import('vscode'); - // Show save dialog - const saveUri = await vscode.window.showSaveDialog({ - defaultUri: vscode.Uri.file(params.defaultFileName), + const saveUri = await window.showSaveDialog({ + defaultUri: Uri.file(params.defaultFileName), filters: { 'HTML files': ['html'], 'All files': ['*'] } }); if (saveUri) { // Write the report content to the selected file - await vscode.workspace.fs.writeFile(saveUri, Buffer.from(params.reportContent, 'utf8')); - vscode.window.showInformationMessage(`Migration report saved to ${saveUri.fsPath}`); + await workspace.fs.writeFile(saveUri, Buffer.from(params.reportContent, 'utf8')); + window.showInformationMessage(`Migration report saved to ${saveUri.fsPath}`); } }workspaces/wi/wi-extension/src/migration-report/webview.ts (1)
47-70: Surface the file name in the panel title
createOrShowacceptsfileNamebut never uses it. Besides hurting readability (and potentially trippingnoUnusedParametersif the compiler is strict), it also misses the opportunity to tell users which report they’re viewing. Wire the value into the panel title both when creating the webview and when reusing an existing panel.if (MigrationReportWebview.currentPanel) { - MigrationReportWebview.currentPanel._panel.reveal(ViewColumn.Active); + MigrationReportWebview.currentPanel._panel.title = `${fileName} Migration Report`; + MigrationReportWebview.currentPanel._panel.reveal(ViewColumn.Active); MigrationReportWebview.currentPanel.updateContent(reportContent); return; } const panel = window.createWebviewPanel( "migrationReport", - `Migration Report`, + `${fileName} Migration Report`, ViewColumn.Active, { enableScripts: true, retainContextWhenHidden: true, } );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
workspaces/wi/wi-core/src/rpc-types/main/index.ts(1 hunks)workspaces/wi/wi-core/src/rpc-types/main/rpc-type.ts(1 hunks)workspaces/wi/wi-core/src/types/rpc.types.ts(1 hunks)workspaces/wi/wi-extension/src/migration-report/webview.ts(1 hunks)workspaces/wi/wi-extension/src/rpc-managers/main/rpc-handler.ts(1 hunks)workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts(1 hunks)workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- workspaces/wi/wi-core/src/rpc-types/main/index.ts
- workspaces/wi/wi-extension/src/rpc-managers/main/rpc-handler.ts
- workspaces/wi/wi-core/src/types/rpc.types.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts
🧬 Code graph analysis (3)
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts (5)
workspaces/wi/wi-core/src/rpc-types/main/index.ts (1)
WIVisualizerAPI(24-46)workspaces/wi/wi-extension/src/rpc-managers/main/utils.ts (7)
askFilePath(35-46)askProjectPath(48-56)askFileOrFolderPath(58-66)handleOpenFile(68-145)getUsername(226-237)sanitizeName(222-224)BALLERINA_INTEGRATOR_ISSUES_URL(33-33)workspaces/wi/wi-core/src/rpc-types/migrate-integration/interfaces.ts (6)
GetMigrationToolsResponse(36-38)MigrateRequest(71-76)ImportIntegrationRPCRequest(45-50)ImportIntegrationResponse(57-64)OpenMigrationReportRequest(52-55)SaveMigrationReportRequest(66-69)workspaces/wi/wi-core/src/rpc-types/main/rpc-type.ts (1)
pullMigrationTool(43-43)workspaces/wi/wi-extension/src/rpc-managers/main/migrate-integration.ts (1)
pullMigrationTool(35-271)
workspaces/wi/wi-core/src/rpc-types/main/rpc-type.ts (3)
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts (21)
openBiExtension(60-62)openMiExtension(64-66)runCommand(68-70)selectFileOrDirPath(72-94)selectFileOrFolderPath(96-107)getWorkspaceRoot(109-114)getConfiguration(116-121)getSupportedMIVersionsHigherThan(123-130)getSubFolderNames(132-154)askProjectDirPath(156-167)createMiProject(169-195)fetchSamplesFromGithub(197-240)downloadSelectedSampleFromGithub(242-247)createBIProject(269-281)getMigrationTools(265-267)migrateProject(283-295)pullMigrationTool(297-304)importIntegration(306-325)showErrorMessage(327-331)openMigrationReport(333-335)saveMigrationReport(337-354)workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts (21)
openBiExtension(78-80)openMiExtension(82-84)runCommand(86-88)selectFileOrDirPath(90-92)selectFileOrFolderPath(94-96)getWorkspaceRoot(98-100)getConfiguration(102-104)getSupportedMIVersionsHigherThan(106-108)getSubFolderNames(110-112)askProjectDirPath(114-116)createMiProject(118-120)fetchSamplesFromGithub(122-124)downloadSelectedSampleFromGithub(126-128)createBIProject(130-132)getMigrationTools(134-136)migrateProject(138-140)pullMigrationTool(142-144)importIntegration(146-148)showErrorMessage(150-152)openMigrationReport(154-156)saveMigrationReport(158-160)workspaces/wi/wi-core/src/types/rpc.types.ts (25)
RunCommandRequest(19-22)RunCommandResponse(24-28)FileOrDirRequest(34-36)FileOrDirResponse(30-32)WorkspaceRootResponse(38-40)GetConfigurationRequest(42-44)GetConfigurationResponse(46-48)GetSupportedMIVersionsResponse(62-64)GetSubFoldersRequest(50-52)GetSubFoldersResponse(54-56)ProjectDirResponse(58-60)CreateMiProjectRequest(66-74)CreateMiProjectResponse(76-78)GettingStartedData(95-98)SampleDownloadRequest(100-102)BIProjectRequest(104-111)DownloadProgress(134-141)GetMigrationToolsResponse(130-132)MigrateRequest(152-157)PullMigrationToolRequest(159-162)ImportIntegrationRPCRequest(164-169)ImportIntegrationResponse(143-150)ShowErrorMessageRequest(178-180)OpenMigrationReportRequest(190-193)SaveMigrationReportRequest(195-198)
workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts (3)
workspaces/wi/wi-core/src/rpc-types/main/index.ts (1)
WIVisualizerAPI(24-46)workspaces/wi/wi-core/src/rpc-types/main/rpc-type.ts (21)
openBiExtension(24-24)openMiExtension(25-25)runCommand(26-26)selectFileOrDirPath(27-27)selectFileOrFolderPath(28-28)getWorkspaceRoot(29-29)getConfiguration(30-30)getSupportedMIVersionsHigherThan(31-31)getSubFolderNames(32-32)askProjectDirPath(33-33)createMiProject(34-34)fetchSamplesFromGithub(35-35)downloadSelectedSampleFromGithub(36-36)createBIProject(37-37)getMigrationTools(41-41)migrateProject(42-42)pullMigrationTool(43-43)importIntegration(44-44)showErrorMessage(45-45)openMigrationReport(46-46)saveMigrationReport(47-47)workspaces/wi/wi-core/src/types/rpc.types.ts (24)
RunCommandRequest(19-22)RunCommandResponse(24-28)FileOrDirRequest(34-36)FileOrDirResponse(30-32)WorkspaceRootResponse(38-40)GetConfigurationRequest(42-44)GetConfigurationResponse(46-48)GetSupportedMIVersionsResponse(62-64)GetSubFoldersRequest(50-52)GetSubFoldersResponse(54-56)ProjectDirResponse(58-60)CreateMiProjectRequest(66-74)CreateMiProjectResponse(76-78)GettingStartedData(95-98)SampleDownloadRequest(100-102)BIProjectRequest(104-111)GetMigrationToolsResponse(130-132)MigrateRequest(152-157)PullMigrationToolRequest(159-162)ImportIntegrationRPCRequest(164-169)ImportIntegrationResponse(143-150)ShowErrorMessageRequest(178-180)OpenMigrationReportRequest(190-193)SaveMigrationReportRequest(195-198)
🪛 Biome (2.1.2)
workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts
[error] 79-79: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
[error] 83-83: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
🔇 Additional comments (2)
workspaces/wi/wi-core/src/rpc-types/main/rpc-type.ts (1)
1-47: LGTM! RPC type definitions are well-structured.The RPC type definitions correctly distinguish between notifications (
NotificationType) for fire-and-forget operations likeopenBiExtension/openMiExtensionand requests (RequestType) for operations requiring responses. The consistent naming convention and type mappings align well with the interface definitions inWIVisualizerAPI.workspaces/wi/wi-rpc-client/src/rpc-clients/main/rpc-client.ts (1)
126-128: LGTM! Notification method correctly omits return statement.The
downloadSelectedSampleFromGithubmethod correctly callssendNotificationwithout a return statement, consistent with itsvoidreturn type. This is the correct pattern for notification methods.
workspaces/wi/wi-extension/src/rpc-managers/main/rpc-manager.ts
Outdated
Show resolved
Hide resolved
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: 1
🧹 Nitpick comments (1)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (1)
23-47: Tight WI/BI view selection is good; typeisInWIand consider progress alignmentThe WI-aware activation flow looks solid overall:
activateProjectExplorercleanly derives a singletreeviewIdbased onisInWIand uses it consistently for tree creation and BI command focusing.- Passing
isInWIdown intoregisterBallerinaCommandsand thenregisterBICommandskeeps WI/BI behavior centralized and avoids hard‑coding the BI view ID in multiple places.Two small follow‑ups to tighten this up:
Type the
isInWIparameter onregisterBICommandsRight now the function is:
function registerBICommands(isInWI) { const treeViewId = isInWI ? WI_PROJECT_EXPLORER_VIEW_ID : BI_PROJECT_EXPLORER_VIEW_ID; commands.executeCommand(`${treeViewId}.focus`); // ... }To avoid an implicit‑
any(and keep consistency withregisterBallerinaCommands), it’s better to make this explicit:-function registerBICommands(isInWI) { +function registerBICommands(isInWI: boolean) {(Optional) Make the provider’s progress location WI‑aware
From
ProjectExplorerEntryProvider.refresh, the progress location is still keyed to the BI view ID viaBI_COMMANDS.PROJECT_EXPLORER. If you want the loading indicator to appear under the WI view whenisInWIis true, you may eventually want to inject the chosen tree view ID there as well, similar to how you do withcreateProjectTree. This can be a follow‑up refactor and doesn’t block this PR.Also applies to: 63-65, 67-86, 138-143
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
workspaces/api-designer/api-designer-extension/package.json(1 hunks)workspaces/apk/apk-extension/package.json(1 hunks)workspaces/ballerina/ballerina-core/src/interfaces/constants.ts(1 hunks)workspaces/ballerina/ballerina-extension/package.json(1 hunks)workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts(6 hunks)workspaces/bi/bi-extension/package.json(3 hunks)workspaces/bi/bi-extension/src/constants/index.ts(1 hunks)workspaces/bi/bi-extension/src/project-explorer/activate.ts(4 hunks)workspaces/bi/bi-extension/src/stateMachine.ts(3 hunks)workspaces/common-libs/scripts/package-vsix.js(1 hunks)workspaces/mcp-inspector/mcp-inspector-extension/package.json(1 hunks)workspaces/mi/mi-extension/package.json(2 hunks)workspaces/mi/mi-extension/src/constants/index.ts(1 hunks)workspaces/mi/mi-extension/src/stateMachine.ts(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- workspaces/common-libs/scripts/package-vsix.js
- workspaces/ballerina/ballerina-extension/package.json
- workspaces/ballerina/ballerina-core/src/interfaces/constants.ts
- workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts
- workspaces/apk/apk-extension/package.json
- workspaces/mi/mi-extension/package.json
- workspaces/mi/mi-extension/src/stateMachine.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/bi/bi-extension/src/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/bi/bi-extension/package.jsonworkspaces/bi/bi-extension/src/constants/index.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/bi/bi-extension/src/stateMachine.tsworkspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/bi/bi-extension/package.jsonworkspaces/bi/bi-extension/src/constants/index.ts
🧬 Code graph analysis (3)
workspaces/bi/bi-extension/src/stateMachine.ts (1)
workspaces/bi/bi-extension/src/constants/index.ts (1)
WI_EXTENSION_ID(22-22)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (2)
workspaces/bi/bi-extension/src/constants/index.ts (2)
WI_PROJECT_EXPLORER_VIEW_ID(23-23)BI_PROJECT_EXPLORER_VIEW_ID(24-24)workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1)
ProjectExplorerEntryProvider(59-130)
workspaces/bi/bi-extension/src/constants/index.ts (1)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
🔇 Additional comments (5)
workspaces/bi/bi-extension/src/constants/index.ts (1)
19-24: Centralizing BI/WI identifiers via shared constants looks correctImporting
BI_COMMANDSfrom the shared core and derivingBI_PROJECT_EXPLORER_VIEW_IDfromBI_COMMANDS.PROJECT_EXPLORER, alongside the new WI IDs, keeps view IDs consistent across the BI extension and the shared command definitions without introducing cross-extension coupling. This fits the existing pattern of using core libraries for shared values rather than importing from other extensions.workspaces/bi/bi-extension/src/stateMachine.ts (1)
23-42: isInWI context propagation is coherent and side‑effect freeAdding
isInWIto the machine context, initializing it once from the WI extension presence, and forwarding it throughactivateExplorerintoactivateProjectExplorerkeeps WI-awareness contained in the state machine without disturbing the existing BI flags. TheassigninonDonedoesn’t overwriteisInWI, so its initial value is preserved as intended.Also applies to: 71-80
workspaces/mcp-inspector/mcp-inspector-extension/package.json (1)
55-68: Shared VSIX packaging script integration is consistentSwitching
"package"tonode ../../common-libs/scripts/package-vsix.jsaligns this extension with the centralized packaging flow used by other workspaces, and the existingbuild/postbuildchain still packages before copying VSIX artifacts. No issues spotted with pathing or script ordering.workspaces/api-designer/api-designer-extension/package.json (1)
82-99: Packaging script change matches the new shared patternUsing the common
package-vsix.jsscript for"package"brings this extension in line with the rest of the workspace, while preserving the existingbuild→package→postbuildpipeline. The change is self-contained and looks safe.workspaces/bi/bi-extension/package.json (1)
67-72: BI view hiding logic is plausible; confirmwhensemantics vs. config valuesThe changes here align with the PR goals:
- The
"package"script now delegates to the shared../../common-libs/scripts/package-vsix.js, matching the rest of the workspace’s extensions.- Adding a
whenclause toBI.project-exploreris a good way to hide the BI activity view when an integrator default is configured elsewhere.One detail to double‑check:
"when": "!config.integrator.defaultIntegrator"will hide the BI view wheneverintegrator.defaultIntegratoris set to any truthy value.- If the config is only ever set when WI (or another non‑BI integrator) is active, this is perfect.
- If the config can also be set to some explicit BI value, you may instead want a more specific condition (e.g., checking that the config is either unset or not equal to the WI integrator value) so that BI doesn’t accidentally hide itself when it’s the selected default.
Please confirm the expected range of values for
config.integrator.defaultIntegratorand adjust thewhenexpression if necessary so it matches the intended UX.Also applies to: 158-167
| export const WI_EXTENSION_ID = 'wso2.wso2-integrator'; | ||
| export const WI_PROJECT_EXPLORER_VIEW_ID = 'wso2-integrator.explorer'; | ||
| export const MI_PROJECT_EXPLORER_VIEW_ID = 'MI.project-explorer'; |
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.
🧩 Analysis chain
Verify constant usage and ID consistency across extensions.
The new constants look well-structured and support the PR's objective of enabling MI/WI interoperability. However, please verify:
- These constants are actually referenced in the codebase (the AI summary mentions stateMachine.ts)
- The extension and view IDs match the actual registrations in the WI extension's package.json
- All hardcoded references to these string literals have been replaced with these constants
Run the following script to verify usage and consistency:
🏁 Script executed:
#!/bin/bash
# Verify the new constants are used and IDs match registrations
echo "=== 1. Check usage of new constants in MI extension ==="
rg -n --type=ts "WI_EXTENSION_ID|WI_PROJECT_EXPLORER_VIEW_ID|MI_PROJECT_EXPLORER_VIEW_ID" \
-g '!**/node_modules/**' workspaces/mi/
echo -e "\n=== 2. Find WI extension package.json ==="
fd -t f "package.json" workspaces/wi/ --exec echo "File: {}"
echo -e "\n=== 3. Check if WI extension ID matches in package.json ==="
fd -t f "package.json" workspaces/wi/ --exec cat {} | jq -r 'select(.name != null) | "Extension name: " + .name'
echo -e "\n=== 4. Check view contributions for both extensions ==="
echo "--- WI extension views ---"
fd -t f "package.json" workspaces/wi/ --exec cat {} | jq '.contributes.views'
echo "--- MI extension views ---"
fd -t f "package.json" workspaces/mi/mi-extension/ --exec cat {} | jq '.contributes.views'
echo -e "\n=== 5. Check for hardcoded string literals that should use constants ==="
rg -n --type=ts "'wso2\.wso2-integrator'|\"wso2\.wso2-integrator\"|'wso2-integrator\.explorer'|\"wso2-integrator\.explorer\"|'MI\.project-explorer'|\"MI\.project-explorer\"" \
-g '!**/node_modules/**' -g '!**/constants/**' workspaces/mi/Length of output: 2233
🏁 Script executed:
#!/bin/bash
# Find WI extension and verify all hardcoded string literals
echo "=== 1. Search for WI extension in various locations ==="
find . -name "package.json" -type f -path "*/wso2*" 2>/dev/null | head -20
echo -e "\n=== 2. Find all references to 'wso2.wso2-integrator' ==="
rg -n "wso2\.wso2-integrator|wso2-integrator\.explorer" \
-g '!**/node_modules/**' --type=ts --type=json
echo -e "\n=== 3. Find all hardcoded 'MI.project-explorer' references ==="
rg -n "MI\.project-explorer" \
-g '!**/node_modules/**' --type=ts --type=json
echo -e "\n=== 4. Verify project-explorer-provider.ts context ==="
rg -n -B5 -A5 "'MI\.project-explorer'" \
-g '!**/node_modules/**' workspaces/mi/mi-extension/src/Length of output: 23112
🏁 Script executed:
#!/bin/bash
# Get exact context around the hardcoded viewId and search for other potential issues
echo "=== 1. Context around project-explorer-provider.ts line 66 ==="
rg -n -B10 -A10 "viewId.*MI\.project-explorer" workspaces/mi/mi-extension/src/ --type=ts --type=tsx
echo -e "\n=== 2. Check if there are other view ID references that should use constants ==="
rg -n "WI_PROJECT_EXPLORER_VIEW_ID|MI_PROJECT_EXPLORER_VIEW_ID" workspaces/mi/mi-extension/src/ --type=ts --type=tsx
echo -e "\n=== 3. Search BI extension for similar patterns ==="
rg -n "'wso2-integrator\.explorer'|'MI\.project-explorer'" workspaces/bi/bi-extension/src/ --type=ts --type=tsxLength of output: 281
🏁 Script executed:
#!/bin/bash
# Correct rg syntax for searching both TS and TSX files
echo "=== 1. Context around project-explorer-provider.ts line 66 ==="
rg -n -B10 -A10 "viewId.*MI\.project-explorer" workspaces/mi/mi-extension/src/
echo -e "\n=== 2. Check if there are other view ID references that should use constants ==="
rg -n "WI_PROJECT_EXPLORER_VIEW_ID|MI_PROJECT_EXPLORER_VIEW_ID" workspaces/mi/mi-extension/src/
echo -e "\n=== 3. Search BI extension for similar patterns ==="
rg -n "'wso2-integrator\.explorer'|'MI\.project-explorer'" workspaces/bi/bi-extension/src/
echo -e "\n=== 4. Look for any remaining hardcoded WI view ID in source files ==="
rg -n "wso2-integrator\.explorer" workspaces/mi/mi-extension/src/ --type=ts
echo -e "\n=== 5. Verify WI extension exists and get its package.json location ==="
find . -type d -name "wso2-integrator*" 2>/dev/null | grep -v node_modules
echo -e "\n=== 6. Search for WI extension in any workspace ==="
find workspaces -type f -name "package.json" -exec grep -l '"name".*wso2.wso2-integrator' {} \;Length of output: 4005
🏁 Script executed:
#!/bin/bash
# Final verification: check PR context and whether WI extension is intentionally external
echo "=== 1. Check git status for any untracked WI directories ==="
git status --short | grep -i "wi\|wso2-integrator" || echo "No untracked WI-related changes"
echo -e "\n=== 2. Check git log for WI-related commits ==="
git log --oneline -20 | grep -i "wi\|wso2-integrator" || echo "No recent WI commits in log"
echo -e "\n=== 3. Look for any documentation about WI extension ==="
find . -type f \( -name "README*" -o -name "*.md" \) -exec grep -l "wso2-integrator\|WI extension" {} \; 2>/dev/null | head -5
echo -e "\n=== 4. Check if constants are exported and used in BI extension ==="
rg -n "WI_EXTENSION_ID|WI_PROJECT_EXPLORER_VIEW_ID" workspaces/bi/bi-extension/src/ | head -20
echo -e "\n=== 5. Final check: all remaining hardcoded view IDs in MI source ==="
rg -n "'MI\.project-explorer'|\"MI\.project-explorer\"" workspaces/mi/mi-extension/src/ --type=ts | grep -v "constants/"Length of output: 1906
Replace hardcoded view ID with constant at project-explorer-provider.ts:66
The constants are well-defined and properly used in stateMachine.ts. However, there's one inconsistency: the hardcoded 'MI.project-explorer' at line 66 should use the newly defined MI_PROJECT_EXPLORER_VIEW_ID constant instead.
Change:
location: { viewId: MI_PROJECT_EXPLORER_VIEW_ID },
Ensure MI_PROJECT_EXPLORER_VIEW_ID is imported at the top of project-explorer-provider.ts from './constants'.
🤖 Prompt for AI Agents
In workspaces/mi/mi-extension/src/project-explorer-provider.ts around line 66,
replace the hardcoded viewId 'MI.project-explorer' with the exported constant
MI_PROJECT_EXPLORER_VIEW_ID and add an import for MI_PROJECT_EXPLORER_VIEW_ID
from './constants' at the top of the file; change the location entry to use
location: { viewId: MI_PROJECT_EXPLORER_VIEW_ID } and ensure the import is
grouped with other constants imports.
…command registration
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)
workspaces/mi/mi-extension/src/stateMachine.ts (1)
331-337: Remove unusedsetTitleparameter or properly wire it through context.The review comment is correct. XState's invoke.data is only used when invoking a child machine to map parent context → child context, not to pass parameters into a promise service. Promise services receive only
(context, event)parameters, making the thirdsetTitleparameter alwaysundefined.The
if (setTitle) { ... }block at lines 439-443 is dead code and will never execute. This causes no regression (titles behave as before), but misleads future maintainers about intended behavior.Either:
- Wire the flag through context (e.g., add
setTitleas a context property and check it inopenWebPanel), or- Remove the
datamapping and thesetTitle-specific branch if this behavior isn't needed.
♻️ Duplicate comments (1)
workspaces/mi/mi-extension/src/stateMachine.ts (1)
36-40: isInWI wiring is coherent, but presence‑based detection may be too coarse.The additions of
isInWItoMachineContext(Line 39), the initial context (Lines 52-53), thenewProjectguard (Line 105), and the dynamictreeViewIdselection inwaitForLS,activateOtherFeatures, andfocusProjectExplorer(Lines 394-396, 659-660, 673-675) form a consistent story: MI either drives its own explorer or the WI explorer, and skips the “new project” flow when WI is recognized.The potential risk is in how
isInWIis computed at Line 704:isInWI: vscode.extensions.getExtension(WI_EXTENSION_ID) ? true : false,This will treat MI as “running under WI” whenever the WI extension is merely installed, regardless of:
- Whether WI actually activated successfully.
- Whether the current flow is really coming from WI vs. the standalone MI activity.
In a failure mode where WI is installed but fails to activate, MI will:
- Skip the
newProjectstate due to!context.isInWIbeing false.- Use WI view IDs for focusing and activation, potentially hiding the MI explorer UI.
You might want
isInWIto indicate “WI is present and functional”, or at least “WI successfully activated”, e.g.:const wiExt = vscode.extensions.getExtension(WI_EXTENSION_ID); isInWI: wiExt?.isActive ?? false,or by moving the “try to activate WI and set a flag” logic into an async path where you can safely
await wiExt.activate()and then store the result in context/workspace state.Please double‑check the UX you expect when both WI and MI are installed but MI is opened standalone, and when WI activation fails. This overlaps with the earlier bot review about activation vs. presence.
Also applies to: 52-53, 105-106, 394-396, 659-660, 673-675, 704-705
🧹 Nitpick comments (3)
workspaces/bi/bi-extension/src/constants/index.ts (1)
19-25: WI/BI explorer constants look consistent; comment text could be clarified.Using
BI_COMMANDS.PROJECT_EXPLORERforBI_PROJECT_EXPLORER_VIEW_IDand defining the WI IDs here is reasonable and keeps IDs aligned with the core constants. The only minor mismatch is the comment at Line 21 about "BI only" attributes, while this block now also carries WI-wide IDs; consider rephrasing that comment or moving WI-wide IDs to a more generic constants file later. Also note the same WI IDs exist in the MI extension; keeping those string literals in sync will be a manual responsibility, which is acceptable per your prior decision to allow duplication across extensions. Based on learnings.workspaces/mi/mi-extension/src/stateMachine.ts (1)
698-699: Extension ID logging is useful for diagnostics but may be noisy in real use.Line 698 logs all extension IDs via
log(vscode.extensions.all.map(ext => ext.id).join(', '));every time a new state machine is created. That’s handy while bringing up WI integration, but it can spam the MI output channel on users with many extensions installed.Consider either:
- Guarding this behind a debug flag / configuration, or
- Removing it once the WI integration is stable.
workspaces/bi/bi-extension/src/project-explorer/activate.ts (1)
23-24: isInWI‑aware explorer activation is consistent; consider aligning progress location with the WI view.Plumbing
isInWIthroughExplorerActivationConfig(Line 31), selectingtreeviewIdbetweenWI_PROJECT_EXPLORER_VIEW_IDandBI_PROJECT_EXPLORER_VIEW_ID(Line 41), and usingWI_PROJECT_EXPLORER_VIEW_REFRESH_COMMANDwhenisInWIis true (Line 74) all line up nicely with the MI side and should let the same BI tree run under both BI and WI explorers.registerBICommands(isInWI)focusing the appropriate tree view (Lines 138-141) is also consistent.One small UX/detail gap:
ProjectExplorerEntryProvider.refresh()still hard‑codes its progress location toBI_COMMANDS.PROJECT_EXPLORER(seeproject-explorer-provider.ts), so in WI mode the progress indicator will be associated with the BI view ID, notWI_PROJECT_EXPLORER_VIEW_ID. If you want progress to appear in the WI explorer as well, consider passing the chosentreeviewIdintoProjectExplorerEntryProvider(e.g., via constructor) and using that inwithProgress({ location: { viewId: treeviewId } }).Also, for clarity you might annotate
registerBICommands(isInWI: boolean)explicitly to make the intent clear.Also applies to: 31-32, 35-36, 41-47, 63-65, 70-75, 84-85, 138-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workspaces/bi/bi-extension/src/constants/index.ts(1 hunks)workspaces/bi/bi-extension/src/project-explorer/activate.ts(4 hunks)workspaces/mi/mi-extension/src/constants/index.ts(1 hunks)workspaces/mi/mi-extension/src/project-explorer/activate.ts(4 hunks)workspaces/mi/mi-extension/src/stateMachine.ts(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/mi/mi-extension/src/constants/index.ts
- workspaces/mi/mi-extension/src/project-explorer/activate.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/bi/bi-extension/src/constants/index.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/bi/bi-extension/src/constants/index.ts
🧬 Code graph analysis (3)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (3)
workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)
activateProjectExplorer(40-722)workspaces/bi/bi-extension/src/constants/index.ts (3)
WI_PROJECT_EXPLORER_VIEW_ID(23-23)BI_PROJECT_EXPLORER_VIEW_ID(25-25)WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND(24-24)workspaces/bi/bi-extension/src/project-explorer/project-explorer-provider.ts (1)
ProjectExplorerEntryProvider(59-130)
workspaces/bi/bi-extension/src/constants/index.ts (1)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts (1)
BI_COMMANDS(31-57)
workspaces/mi/mi-extension/src/stateMachine.ts (3)
workspaces/mi/mi-extension/src/constants/index.ts (3)
WI_PROJECT_EXPLORER_VIEW_ID(208-208)MI_PROJECT_EXPLORER_VIEW_ID(210-210)WI_EXTENSION_ID(207-207)workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)
activateProjectExplorer(40-722)workspaces/mi/mi-extension/src/util/logger.ts (1)
log(38-41)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/mi/mi-extension/package.json (1)
22-24: Add WI extension toextensionDependenciesto support cross-extension viewsWelcome contributions.Lines 228–241 contribute three
viewsWelcomeentries targeting thewso2-integrator.explorerview, which is defined in the WI extension. Without declaring a dependency on it inextensionDependencies, this extension may attempt to render contributions before the WI extension has activated, causing the welcome messages to fail silently.Add
"wso2.wso2-integrator"to theextensionDependenciesarray:"extensionDependencies": [ - "wso2.wso2-platform" + "wso2.wso2-platform", + "wso2.wso2-integrator" ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
workspaces/ballerina/ballerina-core/src/interfaces/constants.ts(1 hunks)workspaces/ballerina/ballerina-extension/package.json(3 hunks)workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts(6 hunks)workspaces/bi/bi-extension/package.json(3 hunks)workspaces/bi/bi-extension/src/project-explorer/activate.ts(5 hunks)workspaces/mi/mi-extension/package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- workspaces/ballerina/ballerina-extension/src/features/bi/activator.ts
- workspaces/ballerina/ballerina-core/src/interfaces/constants.ts
- workspaces/bi/bi-extension/package.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/package.json
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/bi/bi-extension/src/project-explorer/activate.tsworkspaces/ballerina/ballerina-extension/package.json
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/**/*.config.{js,ts} : Use minimatch-compatible glob patterns for file matching in build and test configuration files
Applied to files:
workspaces/ballerina/ballerina-extension/package.json
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Use TypeScript 5.8.3 with strict type checking enabled for all source files
Applied to files:
workspaces/ballerina/ballerina-extension/package.json
🧬 Code graph analysis (1)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (2)
workspaces/bi/bi-extension/src/constants/index.ts (2)
WI_PROJECT_EXPLORER_VIEW_ID(23-23)WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND(24-24)workspaces/mi/mi-extension/src/constants/index.ts (2)
WI_PROJECT_EXPLORER_VIEW_ID(208-208)WI_PROJECT_EXPLORER_VIEW_REFRESH_COMMAND(209-209)
🔇 Additional comments (10)
workspaces/mi/mi-extension/package.json (3)
190-205: Verify conditional visibility logic for dual-explorer modes.The views now use
whenclauses to control visibility:
MI.project-explorershows when!config.integrator.defaultIntegrator(standalone MI mode)wso2-integrator.explorerentries appear whenconfig.integrator.defaultIntegrator != ''(WI mode)Ensure the context key
config.integrator.defaultIntegratoris properly registered and updated by the platform extension to avoid edge cases where both or neither explorer appears.
228-241: Ensure viewsWelcome entries align with loading states and error recovery.Three new entries target
wso2-integrator.explorer:
- Loading state when MI.status is not in terminal/known states
- Error state when MI.status is
disabled- NotSetUp state when MI.status is
notSetUpVerify that MI status propagation to the WI extension is reliable and that the hidden "unknownProject" and "projectLoaded" states in the when clause match the actual MI status lifecycle.
977-977: Verify centralized packaging script is production-ready across all extensions.The package script now delegates to a shared Node script:
node ../../common-libs/scripts/package-vsix.js. Confirm:
- The script correctly inherits extension-specific metadata (name, version, publisher)
- Output VSIX file paths match the expected build artifacts
- All dependent extensions (MI, BI, WI) use this script without conflicts
workspaces/bi/bi-extension/src/project-explorer/activate.ts (6)
23-23: LGTM!Import correctly adds the WI-specific constants needed for conditional view handling.
25-32: LGTM!The
isInWIflag is appropriately added as a required property, ensuring all activation paths explicitly specify the WI context.
34-62: LGTM!The activation function correctly propagates the
isInWIflag and conditionally selects the appropriate tree view ID.
68-70: LGTM!Clean parameterization allows the function to create either WI or BI tree views based on the provided ID.
72-88: LGTM!The refresh command is correctly registered with the appropriate command ID based on the WI context.
90-108: LGTM!The function correctly propagates the
isInWIparameter toregisterBICommands.workspaces/ballerina/ballerina-extension/package.json (1)
1179-1179: Centralized packaging script and context variables verified—no issues found.The referenced script at
../../common-libs/scripts/package-vsix.jsexists and correctly replaces the prior shell-based pre-release logic by reading theisPreReleaseenvironment variable and callingvsce packagewith the--pre-releaseflag when needed. Both context variablesisBIProjectandisBallerinaProjectare properly registered viasetContextcommands in the extension code (atstateMachine.ts,config-provider.ts, andextension.ts) and are actively set/updated during the extension lifecycle. No breaking changes or missing dependencies detected.
| "contents": "[Add Unit Test](command:BI.test.add.function)", | ||
| "when": "isBIProject || isBallerinaProject" |
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.
🧩 Analysis chain
Verify context variables isBIProject and isBallerinaProject are properly registered and maintained.
The visibility gating for testing view commands relies on context variables isBIProject and isBallerinaProject. These must be:
- Registered in VS Code's context
- Set/updated during extension activation and workspace changes
- Aligned with the PR objective to hide BI/MI items when WI is active
Run the following script to locate and verify context variable management in the extension source:
Also applies to: 813-813
🏁 Script executed:
#!/bin/bash
# Description: Verify context variables are defined and set during extension lifecycle
# Search for context variable registration (setContext calls)
echo "=== Searching for context variable registration ==="
fd -e ts -e tsx -x grep -n 'setContext.*\(isBIProject\|isBallerinaProject\)' {}
# Search for context evaluation patterns
echo ""
echo "=== Searching for context variable definitions ==="
fd -e ts -e tsx -x grep -n 'registerExtensionCommands\|activateExtension\|setContext' {} | head -20
# Verify these are referenced in extension activation
echo ""
echo "=== Checking for extension.ts or main activation file ==="
fd -e ts -path '*/src/*' | grep -E '(extension|activation|main)\.ts' | head -5Length of output: 2522
I'll search for more context around these context variable setups to verify they're properly coordinated and maintained during workspace changes.
🏁 Script executed:
#!/bin/bash
# Find file locations and get context around setContext calls
echo "=== Finding files containing setContext calls ==="
fd -e ts -e tsx | xargs grep -l "setContext.*\(isBIProject\|isBallerinaProject\)"
echo ""
echo "=== Context around isBallerinaProject setContext (line 2497) ==="
fd -e ts -e tsx | xargs grep -B5 -A5 "setContext.*isBallerinaProject"
echo ""
echo "=== Context around isBIProject setContext (line 1052) ==="
fd -e ts -e tsx | xargs grep -B5 -A5 "setContext.*isBIProject" | grep -v "isBIProjectRunning"
echo ""
echo "=== Find extension entry point ==="
fd -e ts -path '*/src/*' -name 'extension.ts'Length of output: 4664
🏁 Script executed:
#!/bin/bash
# Find where these context-setting functions are called
echo "=== Where is setCurrentProject() called? ==="
fd -e ts -e tsx | xargs grep -n "setCurrentProject" | grep -v "public setCurrentProject"
echo ""
echo "=== Where is setBIContext() called? ==="
fd -e ts -e tsx | xargs grep -n "setBIContext"
echo ""
echo "=== Workspace listeners and change handlers ==="
fd -e ts -e tsx | xargs grep -n "onDidChangeWorkspaceFolders\|onDidOpenTextDocument\|workspaceState\|updateContext" | head -15
echo ""
echo "=== Check extension.ts for initialization of context variables ==="
cat workspaces/ballerina/ballerina-extension/src/core/extension.ts | grep -n "isBallerinaProject\|isBIProject" | head -10Length of output: 3603
🏁 Script executed:
#!/bin/bash
# Get context around the setBIContext calls to understand the flow
echo "=== Context around setBIContext calls in stateMachine.ts (lines 996, 1009, 1019) ==="
sed -n '985,1030p' workspaces/ballerina/ballerina-extension/src/stateMachine.ts
echo ""
echo "=== Look for project unload/close handlers ==="
fd -e ts -e tsx | xargs grep -n "unload\|closeProject\|projectClose\|clearProject" | grep -i "project\|context" | head -15
echo ""
echo "=== Search for where isBallerinaProject might be reset ==="
fd -e ts -e tsx | xargs grep -n "isBallerinaProject.*false\|setContext.*isBallerinaProject.*false"
echo ""
echo "=== Check stateMachine.ts for workspace change listeners ==="
sed -n '1,100p' workspaces/ballerina/ballerina-extension/src/stateMachine.ts | grep -n "onDidChange\|listener\|workspace"Length of output: 2071
🏁 Script executed:
#!/bin/bash
# Check extension activation and workspace listeners
echo "=== Extension activation function in extension.ts ==="
sed -n '1,150p' workspaces/ballerina/ballerina-extension/src/core/extension.ts | head -60
echo ""
echo "=== Search for activate function ==="
fd -e ts -e tsx | xargs grep -n "export.*activate\|function activate"
echo ""
echo "=== Full stateMachine.ts for workspace listeners and context initialization ==="
fd -e ts -e tsx -path '*/stateMachine.ts' | xargs grep -n "onDidChange\|workspace\|activate" | head -20
echo ""
echo "=== Check if isBallerinaProject is set to false anywhere ==="
fd -e ts -e tsx | xargs grep -n "setContext.*isBallerinaProject"Length of output: 14194
Verify and fix asymmetric context variable management for isBallerinaProject.
The context variables isBIProject and isBallerinaProject are registered and maintained during extension lifecycle, but there's a critical asymmetry:
isBIProjectis properly managed viasetBIContext(isBI)inworkspaces/ballerina/ballerina-extension/src/stateMachine.ts:1051, allowing it to reset tofalsewhen a workspace is not a BI project.isBallerinaProjectis set totrueinworkspaces/ballerina/ballerina-extension/src/core/extension.ts:2497viasetCurrentProject(), but is never reset tofalse.
This one-way toggle breaks the visibility gating logic. Conditions like "isBIProject || isBallerinaProject" will remain true even after switching to a non-Ballerina workspace, defeating the PR's objective to properly hide/show items based on project type.
Required: Add a corresponding mechanism to reset isBallerinaProject to false when the workspace is not a Ballerina project, mirroring the pattern used for isBIProject.
🤖 Prompt for AI Agents
In workspaces/ballerina/ballerina-extension/package.json around lines 768-769
and code paths in workspaces/ballerina/ballerina-extension/src/core/extension.ts
(around line 2497) and
workspaces/ballerina/ballerina-extension/src/stateMachine.ts (around line 1051),
the isBallerinaProject context is only ever set true and never reset to false;
add a symmetric reset like setBIContext does: introduce or call a
setBallerinaContext/isBallerinaContext setter that calls
vscode.commands.executeCommand('setContext', 'isBallerinaProject', boolean) and
ensure it’s invoked with false when workspace/project detection shows the
current workspace is not a Ballerina project (e.g., in workspace change
handlers, in setCurrentProject when no project found, or where
setBIContext(false) is called) so the `"isBIProject || isBallerinaProject"`
conditions correctly reflect current workspace state.
| function registerBICommands(isInWI) { | ||
| const treeViewId = isInWI ? WI_PROJECT_EXPLORER_VIEW_ID : BI_PROJECT_EXPLORER_VIEW_ID; | ||
| commands.executeCommand(`${treeViewId}.focus`); | ||
| commands.executeCommand(SHARED_COMMANDS.SHOW_VISUALIZER); | ||
| commands.executeCommand('setContext', 'BI.project', true); |
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.
🛠️ Refactor suggestion | 🟠 Major
Add missing type annotation for isInWI parameter.
The parameter lacks a TypeScript type annotation, which is inconsistent with the rest of the file and may reduce type safety.
-function registerBICommands(isInWI) {
+function registerBICommands(isInWI: boolean) {
const treeViewId = isInWI ? WI_PROJECT_EXPLORER_VIEW_ID : BI_PROJECT_EXPLORER_VIEW_ID;
commands.executeCommand(`${treeViewId}.focus`);
commands.executeCommand(SHARED_COMMANDS.SHOW_VISUALIZER);
commands.executeCommand('setContext', 'BI.project', true);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function registerBICommands(isInWI) { | |
| const treeViewId = isInWI ? WI_PROJECT_EXPLORER_VIEW_ID : BI_PROJECT_EXPLORER_VIEW_ID; | |
| commands.executeCommand(`${treeViewId}.focus`); | |
| commands.executeCommand(SHARED_COMMANDS.SHOW_VISUALIZER); | |
| commands.executeCommand('setContext', 'BI.project', true); | |
| function registerBICommands(isInWI: boolean) { | |
| const treeViewId = isInWI ? WI_PROJECT_EXPLORER_VIEW_ID : BI_PROJECT_EXPLORER_VIEW_ID; | |
| commands.executeCommand(`${treeViewId}.focus`); | |
| commands.executeCommand(SHARED_COMMANDS.SHOW_VISUALIZER); | |
| commands.executeCommand('setContext', 'BI.project', true); | |
| } |
🤖 Prompt for AI Agents
In workspaces/bi/bi-extension/src/project-explorer/activate.ts around lines 160
to 164, the function parameter isInWI is missing a TypeScript type annotation;
update the function signature to include an explicit boolean type (e.g. isInWI:
boolean) to match the file's typing conventions and improve type safety, and
then run type-check to ensure no downstream signature issues.
Purpose
Resolves wso2/product-integrator#4
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
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.