FIx LS not working when webview closed issue #1035
Conversation
…ove state machine integration
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRegisters per-project lifecycle listeners that start/stop MILanguageClient based on open editors and visualizer presence; removes stored langClient from MachineContext and replaces state-machine–provided client usage with awaited MILanguageClient.getInstance(projectUri) across many modules; activates project explorer by projectUri. Changes
Sequence Diagram(s)sequenceDiagram
participant Editor as VSCode Editor / Tabs
participant LSReg as enableLS
participant Checker as hasOpenedDocumentInProject
participant Webview as Visualizer Panel
participant LC as MILanguageClient
Editor->>LSReg: onDidChangeActiveTextEditor(event)
LSReg->>Checker: hasOpenedDocumentInProject(projectUri)
alt document exists
Checker-->>LSReg: true
LSReg->>LC: MILanguageClient.getInstance(projectUri)
LC-->>LSReg: start / return instance
else no document
Checker-->>LSReg: false
LSReg->>LC: stopInstance(projectUri) (debounced)
end
Webview->>Checker: onDispose(projectUri)
alt document exists
Checker-->>Webview: true
Webview-->>LC: keep running
else none
Checker-->>Webview: false
Webview->>LC: stopInstance(projectUri)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workspaces/mi/mi-extension/src/extension.ts(2 hunks)workspaces/mi/mi-extension/src/project-explorer/activate.ts(2 hunks)workspaces/mi/mi-extension/src/stateMachine.ts(1 hunks)workspaces/mi/mi-extension/src/util/workspace.ts(4 hunks)workspaces/mi/mi-extension/src/visualizer/webview.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 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/mi/mi-extension/src/extension.tsworkspaces/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
📚 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} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.ts
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.tsworkspaces/mi/mi-extension/src/visualizer/webview.ts
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/index.ts : Export the main Diagram component as the default export from the package entry point (index.ts)
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/components/(Diagram|DiagramCanvas|Controls)*.tsx : Store diagram zoom and offset positions in state; optionally cache them in localStorage for session persistence
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/components/Diagram.tsx : Debounce diagram repaints to avoid excessive re-renders when state changes occur
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/components/DiagramContext.tsx : Use DiagramContext (React Context API) for state management across the diagram component, including expandedNodes and graphQLGroupOpen state
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
🧬 Code graph analysis (3)
workspaces/mi/mi-extension/src/project-explorer/activate.ts (2)
workspaces/bi/bi-extension/src/project-explorer/activate.ts (1)
activateProjectExplorer(34-62)workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts (1)
ExtendedLanguageClient(160-474)
workspaces/mi/mi-extension/src/extension.ts (1)
workspaces/mi/mi-extension/src/util/workspace.ts (1)
enableLSForWorkspace(138-161)
workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
workspaces/mi/mi-extension/src/util/workspace.ts (1)
hasActiveDocumentInWorkspace(163-173)
🔇 Additional comments (5)
workspaces/mi/mi-extension/src/extension.ts (1)
97-99: LGTM: Per-workspace LS enablement.The loop correctly enables language services for each workspace folder, aligning with the PR objective to fix LS availability when webviews are closed.
workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
281-285: LGTM: Conditional LS lifecycle management.This change directly addresses the reported issue. By checking for active documents before stopping the language server, the LS now remains available when switching between diagram and source views.
workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)
45-46: LGTM: Refactored to derive language client internally.The signature change aligns with workspace-aware activation. The code properly awaits
getInstanceand derives the language client.workspaces/mi/mi-extension/src/util/workspace.ts (1)
163-173: LGTM: Clean implementation of active document check.The function correctly iterates through all tab groups and checks for text documents in the artifacts path. This utility properly supports the workspace-aware LS lifecycle management.
workspaces/mi/mi-extension/src/stateMachine.ts (1)
662-662: Verify projectUri is always defined in ready.activateOtherFeatures state.The non-null assertion (
context.projectUri!) assumes projectUri is set before reaching this state. While the state machine flow likely ensures this, consider adding a guard or type refinement for safety.
There was a problem hiding this comment.
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/util/workspace.ts (1)
95-136: Tighten upsaveIdpSchemaToFileedge cases: file content guard and validation ordering for data safetyTwo concerns worth addressing:
- Empty schema content behavior is ambiguous
Theif (fileContent)guard treats empty string the same as undefined, skipping the write. If clearing schema JSON to an empty string is valid, usefileContent !== undefinedinstead:- if (fileContent) { + if (fileContent !== undefined) { fs.writeFileSync(documentUri, fileContent, "utf-8"); }
File deletion happens before validation—risking data loss
The code deletes existing image/PDF files (lines 105–112) before validating the base64 string and MIME type. IfimageOrPdfis malformed or has an unsupported type, files are already lost when the error is returned.Reorder to validate first, then delete only if the new data is valid:
if (imageOrPdf) { + const mimeTypeMatch = imageOrPdf.match(/^data:(.*?);base64,/); + if (mimeTypeMatch) { + const mimeType = mimeTypeMatch[1]; + let extension = ""; + if (mimeType === "application/pdf") { + extension = ".pdf"; + } else if (mimeType.startsWith("image/")) { + extension = mimeType.split("/")[1]; + extension = `.${extension}`; + } else { + console.error("Unsupported MIME type:", mimeType); + return false; + } + const base64Data = imageOrPdf.replace(/^data:.*;base64,/, ""); + const binaryData = Buffer.from(base64Data, "base64"); + const filePath = path.join(folderPath, fileName + extension); const extensionsToDelete = [".pdf", ".jpg", ".jpeg", ".png", ".gif", ".webp"]; const filesInFolder = await fs.promises.readdir(folderPath); for (const file of filesInFolder) { const ext = path.extname(file).toLowerCase(); if (extensionsToDelete.includes(ext)) { await fs.promises.unlink(path.join(folderPath, file)); } } - const mimeTypeMatch = imageOrPdf.match(/^data:(.*?);base64,/); - if (mimeTypeMatch) { - const mimeType = mimeTypeMatch[1]; - let extension = ""; - if (mimeType === "application/pdf") { - extension = ".pdf"; - } else if (mimeType.startsWith("image/")) { - extension = mimeType.split("/")[1]; - extension = `.${extension}`; - } else { - console.error("Unsupported MIME type:", mimeType); - return false; - } - const base64Data = imageOrPdf.replace(/^data:.*;base64,/, ""); - const binaryData = Buffer.from(base64Data, "base64"); - const filePath = path.join(folderPath, fileName + extension); fs.writeFileSync(filePath, binaryData); } else { console.error("Invalid base64 string format."); return false; } }This ensures existing assets remain intact if the new data fails validation.
🧹 Nitpick comments (2)
workspaces/mi/mi-extension/src/util/workspace.ts (2)
19-26: Consolidate vscode imports and avoid namespace + named style mixYou’re importing both specific symbols and the
vscodenamespace. SincewindowandTabInputTextare the only reasons to use the namespace, you can keep everything consistent with a single named import and drop the namespace import:-import { Position, Range, Uri, WorkspaceEdit, commands, workspace, window } from "vscode"; +import { Position, Range, Uri, WorkspaceEdit, commands, workspace, window, TabInputText } from "vscode"; @@ -import * as vscode from 'vscode'; import { webviews } from "../visualizer/webview"; @@ -export function hasActiveDocumentInWorkspace(workspacePath: string): boolean { - const artifactsPath = path.join(workspacePath, 'src', 'main', 'wso2mi', 'artifacts'); - for (const tabGroup of vscode.window.tabGroups.all) { - for (const tab of tabGroup.tabs) { - if (tab.input instanceof vscode.TabInputText && tab.input.uri.fsPath.startsWith(artifactsPath)) { - return true; - } - } - } - return false; -} +export function hasActiveDocumentInWorkspace(workspacePath: string): boolean { + const artifactsPath = path.join(workspacePath, "src", "main", "wso2mi", "artifacts"); + for (const tabGroup of window.tabGroups.all) { + for (const tab of tabGroup.tabs) { + if (tab.input instanceof TabInputText && tab.input.uri.fsPath.startsWith(artifactsPath)) { + return true; + } + } + } + return false; +}Also note this file now depends on
webviewswhilevisualizer/webview.tsdepends onhasActiveDocumentInWorkspace, which introduces a circular dependency. It’s probably fine given both sides only use exported values in callbacks, but keep this in mind if you ever see initialization‑order issues; refactoring the shared LS state into a lighter‑weight module would break the cycle.Please confirm this still compiles and runs against your targeted VS Code engine (i.e.,
TabInputTextis available as a runtime export) and that there are no circular‑init warnings at extension activation.
138-157: LS lifecycle logic now matches desired behavior; consider keeping the DisposableThis handler now:
- Ignores events while a webview is active (
webviews.has(workspacePath)).- Stops the LS when there are no text editors (
!event).- Otherwise uses
hasActiveDocumentInWorkspace(workspacePath)to decide whether to start or stop the LS, independent of which file is currently active.That fixes the earlier issue where switching away from artifacts or closing the diagram could leave the LS running or stopped incorrectly, and should address the “no suggestions after closing diagram tab” scenario.
One small robustness improvement would be to keep and dispose the
onDidChangeActiveTextEditorsubscription when the workspace is torn down (e.g., from your activation/deactivation plumbing), to avoid accidental double‑subscriptions in future refactors:export function enableLSForWorkspace(workspacePath: string): vscode.Disposable { return window.onDidChangeActiveTextEditor(async (event) => { // existing body... }); }Please verify manually (or via MI UI tests) that:
- Synapse suggestions remain available in the source view after closing the diagram tab, and
- The LS stops once all MI artifacts editors and MI webviews for a workspace are closed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/mi/mi-extension/src/util/workspace.ts(4 hunks)
🧰 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/mi/mi-extension/src/util/workspace.ts
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.ts
📚 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} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
🧬 Code graph analysis (1)
workspaces/mi/mi-extension/src/util/workspace.ts (1)
workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
webviews(34-34)
🔇 Additional comments (2)
workspaces/mi/mi-extension/src/util/workspace.ts (2)
47-65: Retry loop after file creation is bounded and reasonableThe access‑retry loop after creating a new file is bounded (
maxRetries = 5,retryDelay = 100) and will at worst wait ~500 ms, which is acceptable for extension flows like this. No changes needed here from a correctness perspective.Please re‑check any flows that rely on
replaceFullContentToFile(especially on slower disks / network filesystems) to ensure the file is reliably available before the refresh command runs.
159-169:hasActiveDocumentInWorkspacelogic looks correct; double‑check multi‑root and path normalizationThe tab‑based scan for any
src/main/wso2mi/artifactsdocument:
- Correctly checks all tab groups and only considers
TabInputTexttabs.- Uses a simple
startsWith(artifactsPath)prefix to bind tabs to the workspace.Two things to keep an eye on:
- Ensure
workspacePathpassed here is exactlyworkspaceFolder.uri.fsPathso that the computedartifactsPathmatches the actual on‑disk paths on all OSes.- In multi‑root workspaces, you’ll be calling this once per workspace folder; the current implementation should behave well there, but it’s worth testing with MI projects in multiple folders.
Please test this helper in a multi‑root workspace (two MI projects open) and on at least one Windows environment to confirm the
startsWith(artifactsPath)check behaves as expected with OS‑specific path casing and separators.
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the language server (LS) stops working when the webview is closed. The solution implements automatic language server lifecycle management based on open documents and webviews, ensuring the LS remains active when relevant files are still open even after closing the visualizer webview.
Key Changes:
- Language server now only stops on webview disposal if no project documents remain open
- New event-based system enables/disables language server per workspace based on active editor changes
- Project explorer initialization refactored to fetch language client internally rather than receiving it as a parameter
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
workspaces/mi/mi-extension/src/visualizer/webview.ts |
Conditionally stops language server on disposal only when no active documents remain in project |
workspaces/mi/mi-extension/src/util/workspace.ts |
Adds enableLSForProject to manage LS lifecycle via editor change events and hasActiveDocumentInProject helper to check for open project documents |
workspaces/mi/mi-extension/src/stateMachine.ts |
Passes project URI instead of language client to project explorer activation |
workspaces/mi/mi-extension/src/project-explorer/activate.ts |
Accepts project URI parameter and fetches language client internally |
workspaces/mi/mi-extension/src/extension.ts |
Enables LS management for all workspace folders on activation |
Comments suppressed due to low confidence (1)
workspaces/mi/mi-extension/src/project-explorer/activate.ts:281
- Avoid automated semicolon insertion (98% of all statements in the enclosing function have an explicit semicolon).
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
workspaces/mi/mi-extension/src/util/workspace.ts (1)
19-19: Per‑project LS lifecycle logic is correct; consider returning and tracking the DisposableThe new
enableLSForProject+hasOpenedDocumentInProjectpairing looks logically solid:
- Skips LS decisions while a project visualizer webview is open via
webviews.has(projectUri).- Stops the LS when there are no text editors (
!event), and otherwise bases start/stop solely on whether any artifacts tab from that project is open (hasOpenedDocumentInProjectscanningwindow.tabGroupswithTabInputText), which means the LS stays alive even when the active editor is elsewhere as long as an artifacts tab exists.- This aligns with the fix in
VisualizerWebview.dispose, so closing the diagram alone no longer stops the LS while source tabs are open.Two non‑blocking improvements:
Return and track the Disposable (already raised in an earlier review):
window.onDidChangeActiveTextEditorreturns aDisposable. IfenableLSForProjectcan be called more than once for a given project (e.g., dynamic workspace folder changes, re‑activation), you should return that disposable and push it intocontext.subscriptionsto avoid accumulating listeners.
- export function enableLSForProject(projectUri: string): void {
- window.onDidChangeActiveTextEditor(async (event) => {
- export function enableLSForProject(projectUri: string): vscode.Disposable {
- return window.onDidChangeActiveTextEditor(async (event) => {
// ...
});
}2. **Slight behaviour note:** because `hasOpenedDocumentInProject` checks all tab groups, the LS will remain running while any artifacts tab for that project is open, even if the active editor is another project or a non‑artifacts file. That seems desirable for this MI use‑case, but just be aware this differs from “active editor only” semantics. Also applies to: 24-24, 137-168 </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>workspaces/mi/mi-extension/src/util/workspace.ts (1)</summary><blockquote> `94-135`: **IdP schema save helper logic is sound; consider guarding the “no content” case** The updated `saveIdpSchemaToFile` correctly: - Ensures the folder exists before writes. - Writes the JSON schema when `fileContent` is provided. - Cleans up previous artifacts and writes the decoded image/PDF when `imageOrPdf` is provided. - Returns `false` for unsupported MIME types or invalid base64, which is useful to callers. One optional edge‑case improvement: if both `fileContent` and `imageOrPdf` are omitted, the function currently does nothing and still returns `true`. If that scenario is possible, you might want to fail fast instead: ```diff export async function saveIdpSchemaToFile(folderPath: string, fileName: string, fileContent?: string, imageOrPdf?: string): Promise<boolean> { const documentUri = path.join(folderPath, fileName + ".json"); if (!fs.existsSync(folderPath)) { fs.mkdirSync(folderPath, { recursive: true }); } + if (!fileContent && !imageOrPdf) { + console.warn("saveIdpSchemaToFile called without content or imageOrPdf"); + return false; + } if (fileContent) { fs.writeFileSync(documentUri, fileContent, 'utf-8'); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workspaces/mi/mi-extension/src/util/workspace.ts(4 hunks)workspaces/mi/mi-extension/src/visualizer/webview.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 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/index.ts : Export the main Diagram component as the default export from the package entry point (index.ts)
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/components/(Diagram|DiagramCanvas|Controls)*.tsx : Store diagram zoom and offset positions in state; optionally cache them in localStorage for session persistence
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/components/DiagramContext.tsx : Use DiagramContext (React Context API) for state management across the diagram component, including expandedNodes and graphQLGroupOpen state
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 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/components/Diagram.tsx : Debounce diagram repaints to avoid excessive re-renders when state changes occur
Applied to files:
workspaces/mi/mi-extension/src/visualizer/webview.ts
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
📚 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} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.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/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
📚 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/resources/icons/**/*.tsx : Create separate SVG icon components in src/resources/icons/ for all diagram icons and import them as React components
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.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/mi/mi-extension/src/util/workspace.ts
🧬 Code graph analysis (2)
workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
workspaces/mi/mi-extension/src/util/workspace.ts (1)
hasOpenedDocumentInProject(158-168)
workspaces/mi/mi-extension/src/util/workspace.ts (2)
workspaces/bi/bi-extension/src/biExtentionContext.ts (1)
extension(32-32)workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
webviews(34-34)
🔇 Additional comments (2)
workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
32-32: Guarded LS shutdown on webview dispose correctly respects open project documentsUsing
hasOpenedDocumentInProject(this.projectUri)before callingMILanguageClient.stopInstanceavoids tearing down the language server when closing the visualizer while artifacts are still open in text editors, which directly addresses the reported “no suggestions after closing diagram” issue. The check is cheap and consistent with the new per‑project LS lifecycle inenableLSForProject.Also applies to: 281-285
workspaces/mi/mi-extension/src/util/workspace.ts (1)
49-63: File readiness retry loop is bounded and reasonableThe retry loop after creating a new file (
maxRetries = 5,retryDelay = 100) gives the FS up to ~500 ms to settle before logging a warning and proceeding, which is a good balance between robustness and not blocking the extension thread for too long. No changes needed here.
- Replaced instances of `getStateMachine(projectUri).context().langClient` with `await MILanguageClient.getInstance(projectUri)`
There was a problem hiding this comment.
Actionable comments posted: 4
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/debugger/debugger.ts (1)
187-273:createStepOverBreakpointswallows errors due to using lodashrejectinstead of rejecting the PromiseIn the
catchofcreateStepOverBreakpoint, you callreject(\${error}`), which is the lodashrejectimport, not the Promise reject function. This silently discards the error and resolves theasync` function instead of propagating the failure to callers.Consider changing the catch block to actually reject/throw, and then removing the lodash
rejectimport:- public async createStepOverBreakpoint(path: string, breakpoints: DebugProtocol.SourceBreakpoint[]): Promise<void> { - try { + public async createStepOverBreakpoint(path: string, breakpoints: DebugProtocol.SourceBreakpoint[]): Promise<void> { + try { // ... existing logic ... - } catch (error) { - reject(`${error}`); - } + } catch (error) { + logDebug(`Error creating step-over breakpoints: ${error}`, LogLevel.ERROR); + throw error; + } }And at the top of the file:
-import { reject } from 'lodash';This ensures debugger failures are visible to callers instead of being silently ignored.
🧹 Nitpick comments (11)
workspaces/mi/mi-extension/src/test-explorer/activator.ts (3)
99-110: MILanguageClient usage for syntax trees is consistent; consider tightening project-root handling and DRY-ingUsing
await MILanguageClient.getInstance(...).getSyntaxTree(...)in the EDIT/DELETE test case handlers and ingetTestCases()is aligned with the new architecture and should keep the LS available regardless of webview state. You now have three very similar call sites (edit, delete, and discovery) that all:
- Derive a project root from a URI.
- Call
MILanguageClient.getInstance(projectRoot!).- Immediately request a syntax tree for a given XML file.
To reduce future edge-case bugs and make behavior consistent:
- Consider centralizing this into a small helper like
getUnitTestSyntaxTree(uri: Uri | string)that:
- Resolves
projectRootand returns early if it’s missing.- Wraps
getInstance/getSyntaxTreein atry/catchand maps failures to the user-facing “Syntax tree is not available” message.- This will avoid repeating the
projectRoot!pattern and keep error handling behavior uniform across edit/delete/list flows.Also applies to: 160-176, 395-405
271-279: Avoid callingMILanguageClient.getInstancewhenprojectRootis falsy increateTestsRight now
createTests()callsMILanguageClient.getInstance(projectRoot!)and only afterwards checks!projectRoot:const projectRoot = getProjectRoot(uri); const langClient = await MILanguageClient.getInstance(projectRoot!); const projectDetails = await langClient.getProjectDetails(); const projectName = projectDetails?.primaryDetails?.projectName?.value ?? getProjectName(uri); if (!testController || !projectRoot || !projectName) { return; }Reordering the guards avoids ever passing
undefinedintogetInstancein edge cases (e.g., a URI outside a recognized MI project) and makes the nullability story clearer:export async function createTests(uri: Uri) { - const projectRoot = getProjectRoot(uri); - const langClient = await MILanguageClient.getInstance(projectRoot!); - const projectDetails = await langClient.getProjectDetails(); - const projectName = projectDetails?.primaryDetails?.projectName?.value ?? getProjectName(uri); - - if (!testController || !projectRoot || !projectName) { - return; - } + const projectRoot = getProjectRoot(uri); + if (!testController || !projectRoot) { + return; + } + + const langClient = await MILanguageClient.getInstance(projectRoot); + const projectDetails = await langClient.getProjectDetails(); + const projectName = projectDetails?.primaryDetails?.projectName?.value ?? getProjectName(uri); + if (!projectName) { + return; + }This keeps behavior the same in the happy path while being safer in unusual workspace setups.
353-379: HardengetTestCaseNamesAndTestSuiteTypeagainst missing artifacts and artifact pathsThe switch to
MILanguageClienthere looks good, but this block can still blow up at runtime if parts of the project structure are missing:const projectStructure = await langClient!.getProjectStructure(projectUri); const artifacts = projectStructure.directoryMap.src?.main?.wso2mi?.artifacts; const apis = artifacts?.apis?.map(/* ... */); const sequences = artifacts?.sequences?.map(/* ... */); const templates = artifacts?.templates?.map(/* ... */); const allArtifacts = [...apis, ...sequences, ...templates];If any of
artifacts?.apis,artifacts?.sequences, orartifacts?.templatesisundefined, the correspondingapis/sequences/templateswill beundefined, and the spread[...apis, ...sequences, ...templates]will throw (undefinedis not iterable).Similarly,
testArtifactcan be falsy, in which casenormalize(testArtifact)is suspect and the relative-path comparison may not behave as intended.You can make this more robust without changing behavior when all data is present:
- const unitTestST: UnitTest = st?.syntaxTree["unit-test"]; - const testArtifact = unitTestST?.unitTestArtifacts?.testArtifact?.artifact?.textNode; - - const projectStructure = await langClient!.getProjectStructure(projectUri); - - const artifacts = projectStructure.directoryMap.src?.main?.wso2mi?.artifacts; - const apis = artifacts?.apis?.map((api: ProjectStructureArtifactResponse) => { return { name: api.name, path: api.path.split(projectUri)[1], type: "Api" } }); - const sequences = artifacts?.sequences?.map((sequence: ProjectStructureArtifactResponse) => { return { name: sequence.name, path: sequence.path.split(projectUri)[1], type: "Sequence" } }); - const templates = artifacts?.templates?.map((template: ProjectStructureArtifactResponse) => { return { name: template.name, path: template.path.split(projectUri)[1], type: "Template" } }); - const allArtifacts = [...apis, ...sequences, ...templates]; + const unitTestST: UnitTest = st?.syntaxTree["unit-test"]; + const testArtifact = unitTestST?.unitTestArtifacts?.testArtifact?.artifact?.textNode; + if (!testArtifact) { + window.showErrorMessage("Test suite artifact path is not available"); + return; + } + + const projectStructure = await langClient.getProjectStructure(projectUri); + + const artifacts = projectStructure.directoryMap.src?.main?.wso2mi?.artifacts; + const apis = + artifacts?.apis?.map((api: ProjectStructureArtifactResponse) => ({ + name: api.name, + path: api.path.split(projectUri)[1], + type: "Api", + })) ?? []; + const sequences = + artifacts?.sequences?.map((sequence: ProjectStructureArtifactResponse) => ({ + name: sequence.name, + path: sequence.path.split(projectUri)[1], + type: "Sequence", + })) ?? []; + const templates = + artifacts?.templates?.map((template: ProjectStructureArtifactResponse) => ({ + name: template.name, + path: template.path.split(projectUri)[1], + type: "Template", + })) ?? []; + const allArtifacts = [...apis, ...sequences, ...templates];Also note: because
MILanguageClient.getInstancealready guarantees a client here (postprojectUricheck), the non-null assertion onlangClient!is probably unnecessary and can be dropped for cleaner types.workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
124-129: Potential race condition with 100ms wait.If
instance.stop()takes longer than 100ms,getInstanceproceeds and may create a new instance while the old one is still stopping. Consider waiting for the actual stop completion rather than a fixed timeout, or increase the wait with exponential backoff.// If instance is currently stopping, wait for it to complete and create a new one if (this.stoppingInstances.has(projectUri)) { - // Wait a bit for the stop operation to complete - await new Promise(resolve => setTimeout(resolve, 100)); - this.stoppingInstances.delete(projectUri); + // Wait for the stop operation to complete with retries + let retries = 0; + while (this.stoppingInstances.has(projectUri) && retries < 50) { + await new Promise(resolve => setTimeout(resolve, 100)); + retries++; + } }workspaces/mi/mi-extension/src/rpc-managers/ai-panel/rpc-manager.ts (1)
378-381: Null check is now redundant but harmless.Since
MILanguageClient.getInstance()throws an error if the language client fails to initialize, the null check on line 379 is technically unnecessary. However, keeping it as a defensive check is acceptable.If you prefer cleaner code, you could remove the redundant check:
const langClient = await MILanguageClient.getInstance(this.projectUri); - if (!langClient) { - throw new Error('Language client not available'); - }workspaces/mi/mi-extension/src/rpc-managers/mi-debugger/rpc-manager.ts (1)
36-37: MILanguageClient migration looks correct; consider simplifying async wrappersUsing
await MILanguageClient.getInstance(this.projectUri)invalidateBreakpoints,getBreakpointInfo, andgetStepOverBreakpointis consistent with the new activator contract and preserves previous behavior. The outerreturn new Promise(async (resolve) => { … })layers are redundant withasyncand can be safely collapsed to plainasyncfunctions that justreturn await langClient.<method>(…)to reduce noise and avoid any risk of hanging promises if an awaited call throws.Also applies to: 41-48, 50-57, 115-122
workspaces/mi/mi-extension/src/rpc-managers/mi-data-mapper/rpc-manager.ts (1)
60-60: Language client retrieval increateDMFilesis aligned with new lifecycleSwitching from state-machine access to
await MILanguageClient.getInstance(this.projectUri)before callinggetProjectDetails()fits the per-project LS lifecycle and is correctly wrapped in a try/catch that rejects on failure. The logic for derivingruntimeVersionandisResourceContentUsedremains unchanged.Also applies to: 327-339
workspaces/mi/mi-extension/src/debugger/debugger.ts (1)
83-185: MILanguageClient usage in debugger is consistent; minor cleanups possibleThe migration to
await MILanguageClient.getInstance(this.projectUri)increateRuntimeBreakpoints,createStepOverBreakpoint,getBreakpointInformation, andgetNextMediatorBreakpointis consistent with the new activator behavior and keeps LS access centralized. You could optionally (a) drop the unusedprojectUrilocals derived fromworkspacewhere they are no longer used, and (b) remove theif (!langClient)guard aftergetInstanceingetNextMediatorBreakpoint, sincegetInstanceeither returns a client or throws.Also applies to: 275-295
workspaces/mi/mi-extension/src/visualizer/activate.ts (1)
39-40: Pom.xml change flow now correctly ensures an active language clientUsing
await MILanguageClient.getInstance(projectUri!)in the pom.xml change handler is consistent with the new LS lifecycle and guaranteesupdateConnectorDependencies/loadDependentCAppResourceshave a live client, which should help with the original “suggestions stop after closing webview” issue. You could optionally reuse the earlierprojectUricomputed at the top of the handler instead of recomputing it, but that’s purely cosmetic.Also applies to: 209-255
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts (1)
95-96: Visualizer RPCs now use MILanguageClient consistently; consider simplifying async patterns laterThe refactor to obtain the language client via
await MILanguageClient.getInstance(this.projectUri)across visualizer RPCs (project structure/details, dependency ops, overview, OpenAPI import, AI dependencies, etc.) is coherent with the new per-project LS lifecycle and should behave the same as the old stateMachine-based access, while auto-starting LS when needed. Longer-term, you might want to replace the repeatedreturn new Promise(async (resolve) => { … })wrappers with plainasyncfunctions that directlyreturnthe awaited results, which will also make error propagation from language-client calls more predictable.Also applies to: 147-152, 156-161, 164-189, 198-205, 213-323, 326-367, 369-375, 425-432, 434-441, 777-782, 863-872, 905-951
workspaces/mi/mi-extension/src/project-explorer/activate.ts (1)
663-685: Consider checking language client availability earlier.The language client is obtained at line 663 but not checked for null until lines 682-685. While this works because the client isn't used in between, it would be clearer and more maintainable to check immediately after obtaining it.
Consider this refactor:
const langClient = await MILanguageClient.getInstance(workspace.uri.fsPath); + if (!langClient) { + window.showErrorMessage('Language client is not available'); + throw new Error('Language client is not available'); + } // Read the POM file const workspaceFolder = vscode.workspace.getWorkspaceFolder(Uri.file(filePath)); if (!workspaceFolder) { window.showErrorMessage('Cannot find workspace folder'); throw new Error('Cannot find workspace folder'); } const pomPath = path.join(workspaceFolder.uri.fsPath, 'pom.xml'); const pomContent = fs.readFileSync(pomPath, 'utf-8'); if (remove) { // Remove the <mainSequence> tag from the POM const updatedPomContent = pomContent.replace(/\s*<mainSequence>.*?<\/mainSequence>/, ''); fs.writeFileSync(pomPath, updatedPomContent, 'utf-8'); return; } - - if (!langClient) { - window.showErrorMessage('Language client is not available'); - throw new Error('Language client is not available'); - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
workspaces/mi/mi-extension/src/debugger/debugHelper.ts(2 hunks)workspaces/mi/mi-extension/src/debugger/debugger.ts(5 hunks)workspaces/mi/mi-extension/src/extension.ts(2 hunks)workspaces/mi/mi-extension/src/lang-client/activator.ts(2 hunks)workspaces/mi/mi-extension/src/project-explorer/activate.ts(4 hunks)workspaces/mi/mi-extension/src/project-explorer/project-explorer-provider.ts(1 hunks)workspaces/mi/mi-extension/src/rpc-managers/ai-panel/rpc-manager.ts(2 hunks)workspaces/mi/mi-extension/src/rpc-managers/mi-data-mapper/rpc-manager.ts(2 hunks)workspaces/mi/mi-extension/src/rpc-managers/mi-debugger/rpc-manager.ts(3 hunks)workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts(36 hunks)workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts(13 hunks)workspaces/mi/mi-extension/src/stateMachine.ts(4 hunks)workspaces/mi/mi-extension/src/test-explorer/activator.ts(6 hunks)workspaces/mi/mi-extension/src/util/schemaBuilder.ts(2 hunks)workspaces/mi/mi-extension/src/util/swagger.ts(2 hunks)workspaces/mi/mi-extension/src/util/workspace.ts(5 hunks)workspaces/mi/mi-extension/src/visualizer/activate.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/mi/mi-extension/src/extension.ts
- workspaces/mi/mi-extension/src/stateMachine.ts
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/mi/mi-extension/src/test-explorer/activator.ts
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/mi/mi-extension/src/test-explorer/activator.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/mi/mi-extension/src/test-explorer/activator.ts
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.tsworkspaces/mi/mi-extension/src/rpc-managers/mi-data-mapper/rpc-manager.ts
📚 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} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
📚 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/mi/mi-extension/src/util/workspace.ts
📚 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/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
Applied to files:
workspaces/mi/mi-extension/src/util/workspace.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/mi/mi-extension/src/util/workspace.ts
📚 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} : All async operations and promise handling in diagram utilities should use async/await syntax instead of .then() callbacks
Applied to files:
workspaces/mi/mi-extension/src/rpc-managers/mi-data-mapper/rpc-manager.ts
🧬 Code graph analysis (13)
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/rpc-managers/mi-debugger/rpc-manager.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/rpc-managers/ai-panel/rpc-manager.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/util/swagger.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/debugger/debugHelper.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/test-explorer/activator.ts (1)
workspaces/ballerina/ballerina-extension/src/features/ai/activator.ts (1)
langClient(33-33)
workspaces/mi/mi-extension/src/util/schemaBuilder.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/debugger/debugger.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/project-explorer/project-explorer-provider.ts (1)
workspaces/ballerina/ballerina-extension/src/features/ai/activator.ts (1)
langClient(33-33)
workspaces/mi/mi-extension/src/visualizer/activate.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
workspaces/mi/mi-extension/src/util/workspace.ts (2)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)workspaces/mi/mi-extension/src/visualizer/webview.ts (1)
webviews(34-34)
workspaces/mi/mi-extension/src/rpc-managers/mi-data-mapper/rpc-manager.ts (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
MILanguageClient(102-502)
🪛 Biome (2.1.2)
workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts
[error] 4627-4631: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 4635-4639: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 4643-4647: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 5619-5623: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 5627-5631: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
[error] 5688-5692: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (10)
workspaces/mi/mi-extension/src/util/swagger.ts (1)
300-301: LGTM!The migration to
MILanguageClient.getInstance(projectUri)is correctly implemented. The async/await pattern is properly used, and theprojectUriis validated before use.workspaces/mi/mi-extension/src/project-explorer/project-explorer-provider.ts (1)
147-156: LGTM!The removal of optional chaining (
?.) is correct sinceMILanguageClient.getInstance()now throws if the language client fails to initialize. The surrounding try-catch ensures errors are handled gracefully.workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
145-165: LGTM on debounced stop logic.The debounced stop implementation correctly prevents rapid start/stop cycles when switching between editors quickly. The timer management and state tracking are well-implemented.
workspaces/mi/mi-extension/src/util/workspace.ts (2)
137-174: LGTM on the LS lifecycle management.The implementation correctly addresses the original issue:
- Starts LS when an artifacts document becomes active
- Stops LS (with debounce) when no artifacts documents remain open AND no webview is active
- Returns disposables for proper cleanup
The separation between
onDidChangeActiveTextEditor(for starting) andonDidCloseTextDocument(for stopping) ensures proper lifecycle management.
176-186: LGTM on open document detection.Using
window.tabGroups.allto check all open tabs (not just the active editor) is the correct approach here. This ensures the LS stays running as long as any artifacts file is open, regardless of which tab is currently active.workspaces/mi/mi-extension/src/util/schemaBuilder.ts (1)
22-23: Schema generation now uses MILanguageClient in a clean async flowSwitching
generateSchemaandgenerateSchemaFromContentto callawait MILanguageClient.getInstance(projectUri)is consistent with the new LS lifecycle, and keeping these as plainasyncfunctions (without extra Promise wrappers) gives straightforward error propagation and readability. Call sites passingprojectUri(e.g., data-mapper browseSchema) are in line with the new signatures.Also applies to: 29-42, 44-57
workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts (2)
342-343: MILanguageClient migration looks consistent and should fix LS lifetime couplingUsing
MILanguageClient.getInstance(this.projectUri)across the diagram RPC manager (instead ofgetStateMachine(this.projectUri).context().langClient) is applied consistently and awaited everywhere before invoking LS methods (e.g.,tryOutMediator,getSyntaxTree,getProjectStructure, Swagger helpers, DB helpers, mediator helpers, Devant deployment, and the newexposeVersionedServiceshelper). This aligns with the activator’s per‑project singleton design and should decouple LS lifecycle from the webview/state machine, which matches the PR goal of keeping LS functional after webview closure. I don’t see functional regressions in the updated call patterns.Also applies to: 475-507, 529-538, 640-696, 1511-1551, 2625-2631, 3127-3137, 3779-3787, 3838-3840, 4203-4223, 4250-4263, 4304-4351, 4385-4397, 4548-4552, 4619-4655, 4758-4773, 4810-4832, 4870-4916, 5422-5428, 5611-5640, 5670-5692, 5720-5733, 5735-5758, 5765-5770, 5819-5834, 5889-5900, 6125-6153
4626-4655: Unable to verify review comment due to repository access failureThe review cannot be rewritten without access to the codebase to confirm the code patterns, validate line numbers, and ensure the refactoring suggestions are accurate.
workspaces/mi/mi-extension/src/project-explorer/activate.ts (2)
516-520: LGTM! Good defensive null handling.The language client is properly checked for null before use, with an appropriate error message displayed to the user. This pattern should be applied consistently throughout the file.
40-40: Verify all callers have been updated for the new function signature.The function signature changed from accepting
lsClient: ExtendedLanguageClienttoprojectUri: string. Search the codebase for all invocations ofactivateProjectExplorerto ensure they passprojectUriinstead oflsClient. Check that no callsites still reference the old parameter pattern.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (2)
workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts (1)
ExtendedLanguageClient(160-474)workspaces/mi/mi-core/src/state-machine-types.ts (1)
ErrorType(238-241)
🔇 Additional comments (1)
workspaces/mi/mi-extension/src/lang-client/activator.ts (1)
102-109: Per-project lifecycle fields and OutputChannel caching look consistent.The introduction of
lsChannels,stopTimers,stoppingInstances,STOP_DEBOUNCE_MS, and theExtendedLanguageClient-typedlanguageClienttogether withgetOrCreateOutputChannelkeyed byprojectUrigives you a clean per-project lifecycle and logging story. The constant value and comment forSTOP_DEBOUNCE_MSare now consistent, and the per-project channel naming is reasonable.Also applies to: 175-182
| public static async getInstance(projectUri: string): Promise<ExtendedLanguageClient> { | ||
| // Cancel any pending stop operation for this project | ||
| const existingTimer = this.stopTimers.get(projectUri); | ||
| if (existingTimer) { | ||
| clearTimeout(existingTimer); | ||
| this.stopTimers.delete(projectUri); | ||
| } | ||
|
|
||
| // If instance is currently stopping, wait for it to complete and create a new one | ||
| if (this.stoppingInstances.has(projectUri)) { | ||
| // Wait a bit for the stop operation to complete | ||
| await new Promise(resolve => setTimeout(resolve, 100)); | ||
| this.stoppingInstances.delete(projectUri); | ||
| } | ||
|
|
||
| if (!this._instances.has(projectUri)) { | ||
| const instance = new MILanguageClient(projectUri); | ||
| await instance.launch(projectUri); | ||
| this._instances.set(projectUri, instance); | ||
| } | ||
| return this._instances.get(projectUri)!; | ||
| const languageClient = this._instances.get(projectUri)!.languageClient; | ||
| if (!languageClient) { | ||
| const errorMessage = "Language client failed to initialize"; | ||
| window.showErrorMessage(errorMessage); | ||
| throw new Error(errorMessage); | ||
| } | ||
| return languageClient; | ||
| } | ||
|
|
||
| public static async stopInstance(projectUri: string) { | ||
| const instance = this._instances.get(projectUri); | ||
| if (instance) { | ||
| await instance.stop(); | ||
| this._instances.delete(projectUri); | ||
| // Cancel any existing timer for this project | ||
| const existingTimer = this.stopTimers.get(projectUri); | ||
| if (existingTimer) { | ||
| clearTimeout(existingTimer); | ||
| } | ||
|
|
||
| // Schedule the stop operation with debounce | ||
| const timer = setTimeout(async () => { | ||
| this.stoppingInstances.add(projectUri); | ||
| const instance = this._instances.get(projectUri); | ||
| if (instance) { | ||
| await instance.stop(); | ||
| this._instances.delete(projectUri); | ||
| } | ||
| this.stopTimers.delete(projectUri); | ||
| this.stoppingInstances.delete(projectUri); | ||
| }, this.STOP_DEBOUNCE_MS); | ||
|
|
||
| this.stopTimers.set(projectUri, timer); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "activator.ts" | grep -E "mi-extension.*lang-client"Repository: wso2/vscode-extensions
Length of output: 188
Fix race between debounced stopInstance and getInstance returning a stopping client.
A real race exists in the current implementation:
stopInstance's timer callback addsprojectUritostoppingInstances, awaitsinstance.stop(), then deletes the_instancesentry.- If
getInstanceruns whileawait instance.stop()is in progress:- It detects
stoppingInstances.has(projectUri), sleeps 100ms, then unconditionally deletes the flag. _instancesstill holds the old instance (becausestop()has not finished), so!this._instances.has(projectUri)is false.getInstancereturnsthis._instances.get(projectUri)!.languageClient, which is the same client currently being stopped.
- It detects
- When
stop()eventually completes, the timer callback deletes_instances, leaving callers holding a reference to a stoppedExtendedLanguageClient.
Replace the 100ms delay and bare Set with tracking the actual in-flight stop Promise per project. Change stoppingInstances: Set<string> to stoppingInstances: Map<string, Promise<void>> and have getInstance await the actual Promise instead of using a time-based delay. Update stopInstance to store the instance.stop() Promise in the map so getInstance can wait for it to complete before proceeding.
🤖 Prompt for AI Agents
In workspaces/mi/mi-extension/src/lang-client/activator.ts around lines 116-165,
the current use of stoppingInstances as a Set plus a 100ms sleep allows
getInstance to return a client that is still being stopped; replace the Set with
stoppingInstances: Map<string, Promise<void>> and in stopInstance store the
actual in-flight stop Promise (e.g. const stopPromise = instance.stop();
this.stoppingInstances.set(projectUri, stopPromise); await stopPromise; then
cleanup the map entry), and in getInstance if a stop promise exists await that
exact Promise (await this.stoppingInstances.get(projectUri)) before deleting the
map entry and proceeding to check/create the instance, ensuring getInstance
never returns a client that is mid-stop.
2 participants
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
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.