Add support to run multiple projects in a workspace#1534
Merged
ChinthakaJ98 merged 2 commits intowso2:mainfrom Feb 23, 2026
Merged
Add support to run multiple projects in a workspace#1534ChinthakaJ98 merged 2 commits intowso2:mainfrom
ChinthakaJ98 merged 2 commits intowso2:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds multi-project debug support: project selection at launch, Maven-based build-order resolution, ordered per-project builds and artifact deployment, aggregated resource queries across selected projects via a new debug flag. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DebugAdapter
participant DebuggerConfig
participant PomResolver
participant DebugHelper
participant RPCMgr
participant LangClient
User->>DebugAdapter: start launchRequest
DebugAdapter->>DebugAdapter: enumerate workspace folders
alt multiple projects
DebugAdapter->>User: quick-pick projects
User-->>DebugAdapter: selected projects
else single project
DebugAdapter-->>DebugAdapter: auto-select project
end
DebugAdapter->>DebuggerConfig: setProjectList(selected)
DebugAdapter->>PomResolver: getBuildOrder(selected)
PomResolver-->>DebugAdapter: orderedProjects
DebugAdapter->>DebugHelper: build orderedProjects
loop per project
DebugHelper->>DebugHelper: build, copy artifacts
end
DebugAdapter->>RPCMgr: getAvailableResources(isDebugFlow=true)
RPCMgr->>LangClient: parallel queries for each project
LangClient-->>RPCMgr: resources per project
RPCMgr-->>DebugAdapter: aggregatedResources
DebugAdapter-->>User: debugger started / running
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 (3)
workspaces/mi/mi-visualizer/src/RuntimeServicesPanel.tsx (2)
194-203:⚠️ Potential issue | 🟡 MinorUnused
swaggerResponsevariable and typoaboslutePath.
swaggerResponseat line 198 is assigned but never consumed. If the swagger spec is delivered via theonSwaggerSpecReceivednotification (line 176), theawait getOpenAPISpec(...)return value is dead code — the variable declaration can be removed.aboslutePath(line 195) is a typo; should beabsolutePath.🛠️ Proposed fix
- const resource = api_resource.resources.find((resource: any) => resource.name === name.split("__").pop()); - const aboslutePath = resource?.absolutePath; - - if (aboslutePath) { - const swaggerResponse = await rpcClient.getMiDiagramRpcClient().getOpenAPISpec({ - apiName: name, - apiPath: aboslutePath, + const resource = api_resource.resources.find((resource: any) => resource.name === name.split("__").pop()); + const absolutePath = resource?.absolutePath; + + if (absolutePath) { + await rpcClient.getMiDiagramRpcClient().getOpenAPISpec({ + apiName: name, + apiPath: absolutePath, isRuntimeService: true }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-visualizer/src/RuntimeServicesPanel.tsx` around lines 194 - 203, Rename the misspelled variable aboslutePath to absolutePath where it's assigned from resource?.absolutePath and used in the getOpenAPISpec call, and remove the unused local swaggerResponse (the await rpcClient.getMiDiagramRpcClient().getOpenAPISpec(...) call can remain if you need the side-effect, but don't assign its return value); ensure this change aligns with the onSwaggerSpecReceived notification flow so you don't rely on a dead return value from getOpenAPISpec.
187-203:⚠️ Potential issue | 🟠 MajorHardcoded
isDebugFlow: truecauses silent failure when debug session is inactive.When
isDebugFlow: true,getAvailableResourcesusesDebuggerConfig.getProjectList()to resolve resources across project paths. If the project list is empty (no active debug session),Promise.all([])aggregates to an empty resources array, causingfind(...)to returnundefined. The swagger request never executes, and the user sees no panel or error message after clicking "Try it".The hardcoded unconditional
isDebugFlow: truerepresents a functional regression from pre-existing behavior, which did not use debug-flow semantics. Consider conditioning this on debug session state or providing a fallback to the standard single-project path when the project list is empty.Minor issues:
- Line 195:
aboslutePathis a typo (should beabsolutePath)- Lines 198–203:
swaggerResponseis assigned but never used (the actual spec is received viaonSwaggerSpecReceivedcallback at line 176)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-visualizer/src/RuntimeServicesPanel.tsx` around lines 187 - 203, In onTryit, avoid hardcoding isDebugFlow: true when calling rpcClient.getMiDiagramRpcClient().getAvailableResources — instead detect whether a debug session/project list is present and only set isDebugFlow true when it is non-empty, otherwise call getAvailableResources with isDebugFlow: false (or omit the flag) so the single-project path is used as a fallback; also fix the typo aboslutePath -> absolutePath when reading resource?.absolutePath and remove the unused swaggerResponse assignment (or forward the spec to the existing onSwaggerSpecReceived callback) so the retrieved OpenAPI spec is properly handled.workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts (1)
257-266:⚠️ Potential issue | 🟡 Minor
customProjectUriis sent asundefinedwhendocumentIdentifieris absent, and as a raw path when present — both inconsistent with the rest of the payload.Two issues with the conditional spread:
undefinedvalue propagated to the language server: When called fromRuntimeServicesPanel(documentIdentifier: undefined,isDebugFlow: true), the expressionreq.isDebugFlow && { customProjectUri: req.documentIdentifier }yields{ customProjectUri: undefined }. This key is included in the serialized LSP request, which may cause unexpected behaviour on the server side. The spread should be guarded onreq.documentIdentifierbeing defined.Raw path vs. file URI inconsistency: When
documentIdentifieris set,uriis converted to afile://URI viaUri.file(...).toString(), butcustomProjectUricarries the raw filesystem path. If the language server normalises paths by comparing them to file URIs, the lookup will silently fail to match.🛠️ Proposed fix
async getAvailableResources(req: GetAvailableResourcesRequest): Promise<GetAvailableResourcesResponse> { let uri: string | undefined; if (req.documentIdentifier) { uri = Uri.file(req.documentIdentifier).toString(); } return this.sendRequest("synapse/availableResources", { documentIdentifier: { uri: uri }, resourceType: req.resourceType, - ...(req.isDebugFlow && { customProjectUri: req.documentIdentifier }) + ...(req.isDebugFlow && uri && { customProjectUri: uri }) }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts` around lines 257 - 266, The getAvailableResources method is currently adding customProjectUri even when documentIdentifier is undefined and passing the raw path when present; update getAvailableResources to only include customProjectUri when req.documentIdentifier is defined (guard the spread on req.documentIdentifier) and convert that value to the same file URI form as uri (use Uri.file(req.documentIdentifier).toString()) so the payload sends no undefined keys and uses consistent file:// URIs; locate this logic in the ExtendedLanguageClient.getAvailableResources function and apply the conditional and URI conversion there.
🧹 Nitpick comments (5)
workspaces/mi/mi-extension/src/debugger/config.ts (2)
98-101:projectListis not cleared inresetCappandLibs, leaving stale project state across sessions.
resetCappandLibsis the canonical cleanup point for per-session data. OmittingprojectListhere means that after a session ends,DebuggerConfig.getProjectList()still returns the previous session's projects. SinceRuntimeServicesPanelalways passesisDebugFlow: true, the "Try it" button would continue resolving resources against the stale project set until the nextsetProjectListcall.♻️ Proposed fix
public static resetCappandLibs() { this.copiedCappUri = []; this.copiedLibs = []; + this.projectList = []; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/config.ts` around lines 98 - 101, resetCappandLibs currently clears copiedCappUri and copiedLibs but not the DebuggerConfig.projectList, leaving stale project state across sessions; update the resetCappandLibs method to also clear the projectList (the backing field referenced by getProjectList/setProjectList) so that after a session ends DebuggerConfig.getProjectList() returns an empty list and RuntimeServicesPanel (which passes isDebugFlow: true) won't resolve against old projects. Ensure you reference and reset the same projectList variable used by setProjectList/getProjectList inside resetCappandLibs.
107-113: Add explicit return-type annotations tosetProjectListandgetProjectList.TypeScript can infer these, but explicit annotations (
void/string[]) are consistent with every other public method in this class (e.g.,setServerPort,getCopiedCapp, etc.) and make the intent clearer to callers.♻️ Proposed fix
- public static setProjectList(projects: string[]) { + public static setProjectList(projects: string[]): void { this.projectList = projects; } - public static getProjectList() { + public static getProjectList(): string[] { return this.projectList; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/config.ts` around lines 107 - 113, Add explicit TypeScript return-type annotations to the two methods in the class: annotate setProjectList(projects: string[]) with a return type of void, and annotate getProjectList() with a return type of string[]; update the method signatures for setProjectList and getProjectList to match the explicit types used by other public methods (e.g., setServerPort/getCopiedCapp) so intent and consistency are clear to callers.workspaces/mi/mi-extension/src/debugger/debugAdapter.ts (1)
557-570: Use the importedMI_RUNTIME_SERVICES_PANEL_IDconstant instead of the inline string.
disconnectRequest(line 359) already usesMI_RUNTIME_SERVICES_PANEL_ID; the two call sites incontinueLaunchshould do the same to avoid silent mismatches.♻️ Proposed fix
- RPCLayer._messengers.get(this.projectUri)?.sendNotification(miServerRunStateChanged, { type: 'webview', webviewType: 'micro-integrator.runtime-services-panel' }, 'Running'); + RPCLayer._messengers.get(this.projectUri)?.sendNotification(miServerRunStateChanged, { type: 'webview', webviewType: MI_RUNTIME_SERVICES_PANEL_ID }, 'Running');Apply the same change to line 570.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts` around lines 557 - 570, In continueLaunch (in debugAdapter.ts) two call sites currently pass the inline string 'micro-integrator.runtime-services-panel' to RPCLayer._messengers.get(...).sendNotification and openRuntimeServicesWebview; replace those inline strings with the imported MI_RUNTIME_SERVICES_PANEL_ID constant (the same pattern used by disconnectRequest) and ensure the constant is imported/used where openRuntimeServicesWebview is called so both notification and webview-open calls use MI_RUNTIME_SERVICES_PANEL_ID and avoid mismatches.workspaces/mi/mi-extension/src/debugger/pomResolver.ts (1)
44-63:parsePomdoesn't validate mandatory fields — a missinggroupIdorartifactIdproduces IDs like"undefined:my-artifact"that silently skip the project.If either field is absent (e.g., malformed POM), the project is added with a garbage key, never matches any dependency string, and is quietly dropped from the build order without any warning.
♻️ Proposed guard
const groupId = project.groupId || project.parent?.groupId; const artifactId = project.artifactId; + + if (!groupId || !artifactId) { + throw new Error(`Invalid pom.xml at ${pomPath}: missing groupId or artifactId`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/pomResolver.ts` around lines 44 - 63, The parsePom function currently builds an id from project.groupId and project.artifactId without validation; add guards in parsePom to ensure project.groupId (or project.parent.groupId fallback) and project.artifactId are present and non-empty, and if not either throw a descriptive Error or return a rejected Promise (so callers fail-fast) mentioning the pomPath and which field is missing; keep the dependencies extraction (dependencies/dep mapping) as-is but only after validation so you never produce an "undefined:artifact" id and callers of parsePom receive a clear failure instead of silently skipping the project.workspaces/mi/mi-extension/src/debugger/debugHelper.ts (1)
192-196:awaitonchild_process.spawnis a no-op —spawnis synchronous.
spawnreturns aChildProcessimmediately; awaiting it is misleading and could confuse future maintainers.♻️ Proposed fix
- const buildProcess = await child_process.spawn( + const buildProcess = child_process.spawn( buildCommand, [], { shell: true, cwd: project, env: envVariables } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts` around lines 192 - 196, The code awaits child_process.spawn which is synchronous and returns a ChildProcess immediately; remove the erroneous await when assigning buildProcess (the assignment to buildProcess using child_process.spawn with buildCommand/envVariables) and instead either (a) use spawn without await and attach listeners (e.g., 'error' and 'close') or (b) if you intended to block until completion, wrap the ChildProcess in a Promise that resolves/rejects on 'close'/'error' events (or use spawnSync/exec) so the calling logic correctly waits for the build to finish.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts`:
- Around line 544-546: The code calls DebuggerConfig.setVmArgs(...) twice with
identical arguments; remove the duplicate invocation so only a single call to
DebuggerConfig.setVmArgs(args?.vmArgs ? args?.vmArgs : []) remains (locate the
duplicate calls near DebuggerConfig.setVmArgs and the surrounding vmArgs
handling in debugAdapter.ts and delete the redundant line).
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts`:
- Around line 159-179: The code calls reject('...') but `reject` is the lodash
import, so the cancel path silently continues; replace the `reject('Deployment
configurations in the project should be as the same as the runtime.'); return;`
sequence with a thrown error (e.g. `throw new Error('Deployment configurations
in the project should be the same as the runtime.')`) so the async function
rejects and upstream `executeTasks` can handle it; also remove or rename the
lodash `reject` import (or reference it via lodash namespace) to avoid shadowing
the promise rejection symbol.
- Around line 219-270: In the buildProcess.on('exit', ...) handler the Promise
can be double-settled or left pending: ensure you immediately return after any
reject to stop further execution (add returns after
reject(INCORRECT_SERVER_PATH_MSG) and after the catch reject(err) inside the
workspaceLibs block in the exit handler), and guarantee every control path
settles the Promise by adding a fallback resolve/reject when the targetDirectory
does not exist (e.g., call resolve() or reject with a clear message) so the
outer await new Promise in the build flow for each project cannot hang; look for
the buildProcess.on('exit'...) handler, getDeploymentLibJars, getCarFiles usage,
and DebuggerConfig.setCopiedLibs/setCopiedCapp to implement these fixes.
- Around line 271-279: The postBuildTask is being invoked inside the loop over
orderedProjectList (within the buildProcess.on('exit', ...) handler), causing it
to run for each project; move the postBuildTask() call so it executes only after
all projects have built: collect per-project build promises inside the loop
(e.g., push each buildProcess completion promise returned from
buildProcess.on('exit'...) into an array), await Promise.all(...) after the
loop, then call postBuildTask() and resolve once all builds succeed (and reject
if any per-project promise fails). Update the handler around
buildProcess.on('exit'...) to resolve/reject only the per-project promise and
remove the inline postBuildTask() invocation.
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 3924-3932: The loop uses the wrong language client for each
project: inside the DebuggerConfig.getProjectList() map you call
MILanguageClient.getInstance(this.projectUri) which returns the cached client
for this.projectUri for every iteration; change it to obtain the client for the
current projectPath by calling MILanguageClient.getInstance(projectPath) (and
await that instance if needed) before calling
getAvailableResources(documentIdentifier: projectPath, ...). This ensures
getAvailableResources is invoked on the correct per-project MILanguageClient.
---
Outside diff comments:
In `@workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts`:
- Around line 257-266: The getAvailableResources method is currently adding
customProjectUri even when documentIdentifier is undefined and passing the raw
path when present; update getAvailableResources to only include customProjectUri
when req.documentIdentifier is defined (guard the spread on
req.documentIdentifier) and convert that value to the same file URI form as uri
(use Uri.file(req.documentIdentifier).toString()) so the payload sends no
undefined keys and uses consistent file:// URIs; locate this logic in the
ExtendedLanguageClient.getAvailableResources function and apply the conditional
and URI conversion there.
In `@workspaces/mi/mi-visualizer/src/RuntimeServicesPanel.tsx`:
- Around line 194-203: Rename the misspelled variable aboslutePath to
absolutePath where it's assigned from resource?.absolutePath and used in the
getOpenAPISpec call, and remove the unused local swaggerResponse (the await
rpcClient.getMiDiagramRpcClient().getOpenAPISpec(...) call can remain if you
need the side-effect, but don't assign its return value); ensure this change
aligns with the onSwaggerSpecReceived notification flow so you don't rely on a
dead return value from getOpenAPISpec.
- Around line 187-203: In onTryit, avoid hardcoding isDebugFlow: true when
calling rpcClient.getMiDiagramRpcClient().getAvailableResources — instead detect
whether a debug session/project list is present and only set isDebugFlow true
when it is non-empty, otherwise call getAvailableResources with isDebugFlow:
false (or omit the flag) so the single-project path is used as a fallback; also
fix the typo aboslutePath -> absolutePath when reading resource?.absolutePath
and remove the unused swaggerResponse assignment (or forward the spec to the
existing onSwaggerSpecReceived callback) so the retrieved OpenAPI spec is
properly handled.
---
Nitpick comments:
In `@workspaces/mi/mi-extension/src/debugger/config.ts`:
- Around line 98-101: resetCappandLibs currently clears copiedCappUri and
copiedLibs but not the DebuggerConfig.projectList, leaving stale project state
across sessions; update the resetCappandLibs method to also clear the
projectList (the backing field referenced by getProjectList/setProjectList) so
that after a session ends DebuggerConfig.getProjectList() returns an empty list
and RuntimeServicesPanel (which passes isDebugFlow: true) won't resolve against
old projects. Ensure you reference and reset the same projectList variable used
by setProjectList/getProjectList inside resetCappandLibs.
- Around line 107-113: Add explicit TypeScript return-type annotations to the
two methods in the class: annotate setProjectList(projects: string[]) with a
return type of void, and annotate getProjectList() with a return type of
string[]; update the method signatures for setProjectList and getProjectList to
match the explicit types used by other public methods (e.g.,
setServerPort/getCopiedCapp) so intent and consistency are clear to callers.
In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts`:
- Around line 557-570: In continueLaunch (in debugAdapter.ts) two call sites
currently pass the inline string 'micro-integrator.runtime-services-panel' to
RPCLayer._messengers.get(...).sendNotification and openRuntimeServicesWebview;
replace those inline strings with the imported MI_RUNTIME_SERVICES_PANEL_ID
constant (the same pattern used by disconnectRequest) and ensure the constant is
imported/used where openRuntimeServicesWebview is called so both notification
and webview-open calls use MI_RUNTIME_SERVICES_PANEL_ID and avoid mismatches.
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts`:
- Around line 192-196: The code awaits child_process.spawn which is synchronous
and returns a ChildProcess immediately; remove the erroneous await when
assigning buildProcess (the assignment to buildProcess using child_process.spawn
with buildCommand/envVariables) and instead either (a) use spawn without await
and attach listeners (e.g., 'error' and 'close') or (b) if you intended to block
until completion, wrap the ChildProcess in a Promise that resolves/rejects on
'close'/'error' events (or use spawnSync/exec) so the calling logic correctly
waits for the build to finish.
In `@workspaces/mi/mi-extension/src/debugger/pomResolver.ts`:
- Around line 44-63: The parsePom function currently builds an id from
project.groupId and project.artifactId without validation; add guards in
parsePom to ensure project.groupId (or project.parent.groupId fallback) and
project.artifactId are present and non-empty, and if not either throw a
descriptive Error or return a rejected Promise (so callers fail-fast) mentioning
the pomPath and which field is missing; keep the dependencies extraction
(dependencies/dep mapping) as-is but only after validation so you never produce
an "undefined:artifact" id and callers of parsePom receive a clear failure
instead of silently skipping the project.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds multi-project workspace support for MI run/debug by letting users select multiple workspace folders, building them in dependency order, and aggregating resources across the selected projects for debugging-related flows.
Changes:
- Add multi-project selection (quick pick) during debug launch and persist selected projects in
DebuggerConfig. - Build multiple selected Maven projects in dependency order via a new
pomResolver. - Extend
getAvailableResourcesRPC flow to aggregate resources across selected projects whenisDebugFlowis enabled.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| workspaces/mi/mi-visualizer/src/RuntimeServicesPanel.tsx | Requests API resources with isDebugFlow to support multi-project runtime services. |
| workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts | Aggregates available resources across selected projects during debug flow. |
| workspaces/mi/mi-extension/src/lang-client/ExtendedLanguageClient.ts | Extends availableResources request payload for debug-flow project targeting. |
| workspaces/mi/mi-extension/src/debugger/pomResolver.ts | New Maven POM dependency scanner + topological sort for build order. |
| workspaces/mi/mi-extension/src/debugger/debugHelper.ts | Refactors build to iterate over selected projects and copy artifacts/libs per project. |
| workspaces/mi/mi-extension/src/debugger/debugAdapter.ts | Adds multi-project selection UI and factors launch logic into continueLaunch. |
| workspaces/mi/mi-extension/src/debugger/config.ts | Stores selected project list globally for multi-project flows. |
| workspaces/mi/mi-core/src/rpc-types/mi-diagram/types.ts | Adds optional isDebugFlow flag to GetAvailableResourcesRequest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ChinthakaJ98
force-pushed
the
multi-project-run
branch
from
574fd0a to
d796ffe
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts`:
- Around line 319-330: The quick-pick branch that handles no selection currently
only calls this.sendError(response, 1, 'No project selected') but does not
surface a user-visible message; update the selectedItems check inside the
vscode.window.showQuickPick.then(...) callback to also call
vscode.window.showErrorMessage('No project selected') when selectedItems is
falsy or empty (before returning), so the user sees an error similar to the
zero-folders branch; keep existing calls to this.sendError(response, ...) and
ensure you do not proceed to DebuggerConfig.setProjectList(...) or
this.continueLaunch(...) when no selection was made.
- Around line 526-606: The continueLaunch method's promise chains
(getServerPath(...).then(...) and the nested readPortOffset(...).then(...)) lack
top-level .catch handlers, so synchronous throws (e.g., from
DebuggerConfig.setConfigPortOffset or fs.readFileSync) or rejected promises can
leave the DebugProtocol response unsent; add .catch(...) handlers to both
getServerPath(...) and the nested readPortOffset(...) promise chains to catch
any error, call this.sendError(response, <code>), set vscode context
'MI.isRunning' to 'false', call deleteCopiedCapAndLibs() where appropriate, and
surface the error via vscode.window.showErrorMessage (or
showErrorAndExecuteChangeServerPath for INCORRECT_SERVER_PATH_MSG) so the debug
adapter always sends a response and cleans up on failure.
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts`:
- Around line 228-272: The exit handler's Promise (the one created with await
new Promise<void>) can never settle when workspaceFolders is falsy because the
block starting with if (workspaceFolders && workspaceFolders.length > 0) has no
else; add an else branch that always settles the promise (either resolve() or
reject(...) with a clear message) so every execution path completes; locate the
guard around workspaceFolders in debugHelper.ts and ensure the promise is
resolved/rejected in that else path and that any early-return branches (e.g.,
the catch blocks that call reject(err)) remain consistent with existing uses of
DebuggerConfig.setCopiedLibs / setCopiedCapp and logDebug.
In `@workspaces/mi/mi-extension/src/debugger/pomResolver.ts`:
- Around line 82-98: The loop over folders silently skips folders missing
pom.xml which hides misconfiguration from callers of getBuildOrder; update the
branch that checks fileExists(pomPath) so instead of just continue it surfaces
the problem (e.g., call vscode.window.showWarningMessage with a clear message
including the folder/pomPath or throw an Error if missing pom should be fatal)
before continuing or aborting; modify the logic around the for (const folder of
folders) block / the fileExists(pomPath) check so callers of getBuildOrder are
informed, and ensure any tests/consumers of projects or getBuildOrder handle the
new warning/exception flow.
- Around line 44-64: parsePom currently assumes parser.parse(xml) yields
json.project and will throw when project is missing; update parsePom to
defensively validate json.project exists and that artifactId (and groupId via
parent) are present, returning null (or undefined) for non-Maven/minimal poms so
callers can skip them; specifically, in parsePom check const project =
json.project; if (!project) return null; compute const groupId = project.groupId
|| project.parent?.groupId; const artifactId = project.artifactId; if
(!artifactId) return null; also guard dependencies by using const deps =
project.dependencies?.dependency ?? []; and normalize with Array.isArray to
build dependencies; ensure the function signature/return type reflects nullable
result so getBuildOrder (caller) can filter out nulls.
ChinthakaJ98
force-pushed
the
multi-project-run
branch
from
d796ffe to
847ec93
Compare
Contributor
There was a problem hiding this comment.
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/debugHelper.ts (1)
27-27:⚠️ Potential issue | 🟠 MajorLodash
rejectimport is still used incorrectly inexecuteCopyTask(line 151).
rejectfrom lodash is imported at line 27. At line 151 insideexecuteCopyTask, the Promise is declared asnew Promise<void>(async resolve => {— onlyresolveis destructured, notreject. Soreject(...)on line 151 calls lodash's_.reject(), which is a no-op here (returns a filtered array that's discarded). The task failure silently resolves as successful.While line 151 isn't in the changed lines of this PR, the import is still here and creates a confusing trap. Consider either removing this import entirely (and using
throwin affected locations) or renaming it to avoid shadowing.Proposed fix
-import { reject } from 'lodash';And in
executeCopyTask(line 143), addrejectto the Promise callback:- return new Promise<void>(async resolve => { + return new Promise<void>(async (resolve, reject) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts` at line 27, The lodash import "reject" is shadowing Promise reject and causing calls inside executeCopyTask to call _.reject instead of rejecting the Promise; remove the named import "reject" from 'lodash' and update the Promise creation in executeCopyTask (the new Promise<void>(async resolve => { ...) to accept both parameters (async (resolve, reject) => { ...) and ensure every place that intends to fail the task calls reject(error) (or throws) so the Promise actually rejects; alternatively, if you need lodash.reject elsewhere, rename that import (e.g., lodashReject) to avoid the shadowing.
🧹 Nitpick comments (3)
workspaces/mi/mi-extension/src/debugger/debugAdapter.ts (1)
318-321: Quick-pick shows full filesystem paths — consider showing folder names.
folderPaths.map(p => ({ label: p }))renders full paths (e.g.,/home/user/workspace/my-project) as labels. Usingpath.basename(p)for the label (withdescription: pfor disambiguation) would be more readable.Suggested improvement
- folderPaths.map(p => ({ label: p })), + folderPaths.map(p => ({ label: path.basename(p), description: p })),Then update line 328 to use
description(the full path) instead oflabel:- selectedOptions = selectedItems.map(item => item.label); + selectedOptions = selectedItems.map(item => item.description!);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts` around lines 318 - 321, The quick-pick currently shows full filesystem paths because folderPaths.map(p => ({ label: p })) is used; change the mapping in the vscode.window.showQuickPick call to use path.basename(p) for the label and the full path as description (e.g., { label: path.basename(p), description: p }), and then update any subsequent handling of the selection (the code that reads the chosen items from showQuickPick in this scope, e.g., the logic that previously used item.label) to use item.description (the full path) when you need the actual folder path for building/running.workspaces/mi/mi-extension/src/debugger/debugHelper.ts (2)
205-213:closehandler only shows UI messages,exithandler settles the Promise — verify ordering intent.Both
close(line 206) andexit(line 221) handlers are registered whenshouldCopyTargetis true. Theclosehandler shows success/failure messages whileexitperforms artifact copying and settles the Promise. Sinceexitcan fire beforeclose, the user could see the success message after the next project's build has already started. This is a cosmetic timing issue, not a correctness bug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts` around lines 205 - 213, The current buildProcess 'close' handler (registered when shouldCopyTarget is true) shows success/failure messages while the 'exit' handler performs artifact copying and settles the Promise, which can cause the UI message to appear out of order; to fix, remove or avoid registering the 'close' listener when shouldCopyTarget is true and instead move the vscode.window.showInformationMessage / showErrorMessage calls into the 'exit' handler (after the copy logic and before resolving/rejecting the Promise) so messaging happens only after artifacts are copied and the Promise is settled in the intended order; reference buildProcess, shouldCopyTarget, and the 'exit'/'close' handlers when locating the change.
183-185:getBuildOrdererrors are unhandled — will surface as an unreadable rejection.If
getBuildOrderthrows (e.g., circular dependency, missing internal dependency), the error propagates as a rejected Promise fromexecuteBuildTask. The callerexecuteTasks(line 453) catches this via.catch()and shows a generic error message. Consider wrapping thegetBuildOrdercall in a try/catch to provide a user-friendly error message specifically for build-order resolution failures.Suggested improvement
- const orderedProjectList = DebuggerConfig.getProjectList().length > 1 ? - await getBuildOrder(DebuggerConfig.getProjectList()) : DebuggerConfig.getProjectList(); + let orderedProjectList: string[]; + try { + orderedProjectList = DebuggerConfig.getProjectList().length > 1 ? + await getBuildOrder(DebuggerConfig.getProjectList()) : DebuggerConfig.getProjectList(); + } catch (err) { + vscode.window.showErrorMessage(`Failed to resolve project build order: ${err}`); + throw err; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts` around lines 183 - 185, Wrap the call to getBuildOrder when preparing orderedProjectList inside executeBuildTask in a try/catch so any errors from build-order resolution (e.g., circular or missing internal deps) are caught and converted into a user-friendly rejection or error message; specifically, catch errors around the getBuildOrder(DebuggerConfig.getProjectList()) call, log or throw a new Error with a clear message (including the original error message) so executeTasks (the caller that currently .catch()s generic failures) can present a meaningful build-order failure message instead of an unreadable rejection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts`:
- Line 27: The lodash import "reject" is shadowing Promise reject and causing
calls inside executeCopyTask to call _.reject instead of rejecting the Promise;
remove the named import "reject" from 'lodash' and update the Promise creation
in executeCopyTask (the new Promise<void>(async resolve => { ...) to accept both
parameters (async (resolve, reject) => { ...) and ensure every place that
intends to fail the task calls reject(error) (or throws) so the Promise actually
rejects; alternatively, if you need lodash.reject elsewhere, rename that import
(e.g., lodashReject) to avoid the shadowing.
---
Duplicate comments:
In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts`:
- Around line 319-326: When handling the quick-pick result in the promise chain
(the callback that receives selectedItems) add a user-visible error before
returning: if selectedItems is falsy or length === 0, call
vscode.window.showErrorMessage with a clear message (same style as the
zero-folders branch) and then call this.sendError(response, 1, 'No project
selected') and return; update the handler around the existing
vscode.window.showQuickPick .then(...) block so the user sees an error dialog
when they dismiss the picker.
- Around line 526-606: The promise chain in continueLaunch (rooted at
getServerPath(...).then(...) and the nested readPortOffset(...).then(...)) lacks
top-level .catch handlers and has synchronous calls
(DebuggerConfig.setConfigPortOffset, setPortOffset, setEnvVariables, setVmArgs)
that can throw, so add error handling: attach .catch(...) to getServerPath(...)
and to readPortOffset(...) to catch rejections, and wrap the synchronous
DebuggerConfig calls in try/catch to handle thrown errors; in every catch ensure
you call this.sendError(response, ...), set VS Code context 'MI.isRunning' to
'false' and perform any cleanup (e.g., deleteCopiedCapAndLibs()) so the debug
adapter always responds and the session doesn't hang.
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts`:
- Around line 219-276: The exit handler for buildProcess (inside
buildProcess.on('exit')) can leave the promise unsettled when shouldCopyTarget
&& code === 0 but vscode.workspace.workspaceFolders is falsy/empty; add an
explicit branch handling the case where workspaceFolders is missing so the
Promise always resolves or rejects. Specifically, inside the
buildProcess.on('exit') callback (the block referencing shouldCopyTarget,
workspaceFolders, serverPath, targetDirectory, getDeploymentLibJars,
getCarFiles, DebuggerConfig.setCopiedLibs/CoppiedCapp), add an else branch for
the workspaceFolders check that rejects with a clear error (e.g., "No workspace
folder found for project <name>") or resolves if that is the intended behavior,
and verify every path within that callback calls either resolve() or reject() so
the awaiting new Promise<void> never hangs.
In `@workspaces/mi/mi-extension/src/debugger/pomResolver.ts`:
- Around line 90-109: When iterating folders in the pomResolver loop, currently
folders with no pom.xml are silently skipped; update the loop in the function
that uses folders/pomPath/fileExists/parsePom to surface this by calling
vscode.window.showWarningMessage (or at minimum logDebug) when
fileExists(pomPath) returns false so the user knows their selected folder has no
pom.xml; also add a similar warning when parsePom returns no parsed?.id to
indicate an invalid/malformed pom so projects.set is not silently skipped.
---
Nitpick comments:
In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts`:
- Around line 318-321: The quick-pick currently shows full filesystem paths
because folderPaths.map(p => ({ label: p })) is used; change the mapping in the
vscode.window.showQuickPick call to use path.basename(p) for the label and the
full path as description (e.g., { label: path.basename(p), description: p }),
and then update any subsequent handling of the selection (the code that reads
the chosen items from showQuickPick in this scope, e.g., the logic that
previously used item.label) to use item.description (the full path) when you
need the actual folder path for building/running.
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts`:
- Around line 205-213: The current buildProcess 'close' handler (registered when
shouldCopyTarget is true) shows success/failure messages while the 'exit'
handler performs artifact copying and settles the Promise, which can cause the
UI message to appear out of order; to fix, remove or avoid registering the
'close' listener when shouldCopyTarget is true and instead move the
vscode.window.showInformationMessage / showErrorMessage calls into the 'exit'
handler (after the copy logic and before resolving/rejecting the Promise) so
messaging happens only after artifacts are copied and the Promise is settled in
the intended order; reference buildProcess, shouldCopyTarget, and the
'exit'/'close' handlers when locating the change.
- Around line 183-185: Wrap the call to getBuildOrder when preparing
orderedProjectList inside executeBuildTask in a try/catch so any errors from
build-order resolution (e.g., circular or missing internal deps) are caught and
converted into a user-friendly rejection or error message; specifically, catch
errors around the getBuildOrder(DebuggerConfig.getProjectList()) call, log or
throw a new Error with a clear message (including the original error message) so
executeTasks (the caller that currently .catch()s generic failures) can present
a meaningful build-order failure message instead of an unreadable rejection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR will fix the issue wso2/mi-vscode#1385
Screen.Recording.2026-02-21.at.00.04.00.mov
Summary by CodeRabbit
New Features
Improvements