-
Notifications
You must be signed in to change notification settings - Fork 71
Support remote server debugging #1218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds optional host to OpenAPI request types; splits launch configs into local and remote; makes the debug adapter session-aware to configure remote vs local flows; surfaces host/ports/credentials and connectionTimeout in DebuggerConfig; adds command-response timeouts and hostname normalization when fetching runtime services. Changes
Sequence DiagramsequenceDiagram
participant VSCode as VSCode (Extension Host)
participant Adapter as MiDebugAdapter
participant Config as DebuggerConfig
participant Handler as Debugger Handler
participant Server as MI Debug Server
participant Management as Runtime Management API
VSCode->>Adapter: launch(session)
Adapter->>Config: apply session.configuration.properties
alt remote
Adapter->>Config: set host/ports/credentials/timeout
Adapter->>Handler: create(commandPort,eventPort,host)
Handler->>Server: initializeDebugger() (with timeout)
Server-->>Handler: init response
Handler->>Adapter: initialized
Adapter->>Management: fetch runtime services (login -> apis/proxy/data)
Management-->>Adapter: runtime services (hostnames normalized)
Adapter->>VSCode: open runtime UI / update state
else local
Adapter->>Config: set defaults (LOCALHOST, local ports)
Adapter->>Handler: executeTasks() -> wait readiness
Handler->>Server: connect when ready
Adapter->>VSCode: open runtime UI / update state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds remote server debugging support to the MI Extension, enabling developers to debug MI servers running on remote hosts in addition to local debugging capabilities. The implementation introduces new configuration options, connection handling for remote servers, and URL manipulation to support remote debugging scenarios.
- Adds remote debugging configuration with properties for command/event ports, server host, and management credentials
- Implements timeout mechanism for server command responses with error handling
- Adds hostname replacement logic for runtime service URLs to support remote server connections
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| workspaces/mi/mi-extension/package.json | Adds debug configuration schema for remote debugging properties and creates two initial configurations (local and remote) |
| workspaces/mi/mi-extension/src/debugger/config.ts | Adds setters for server configuration properties and remote debugging state management |
| workspaces/mi/mi-extension/src/debugger/activate.ts | Updates debug adapter factory to pass debug session information to the adapter |
| workspaces/mi/mi-extension/src/debugger/debugAdapter.ts | Implements remote vs local debugging logic with configuration parsing and initialization |
| workspaces/mi/mi-extension/src/debugger/debugger.ts | Adds timeout and error handling for command client responses |
| workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts | Disables TLS certificate verification and implements hostname replacement for remote URLs |
| workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts | Adds host parameter to swagger API call for remote debugging support |
| workspaces/mi/mi-core/src/rpc-types/mi-diagram/types.ts | Extends SwaggerFromAPIRequest interface with optional host property |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Outdated
Show resolved
Hide resolved
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Show resolved
Hide resolved
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Outdated
Show resolved
Hide resolved
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Show resolved
Hide resolved
7ea9bb0 to
833886a
Compare
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Show resolved
Hide resolved
833886a to
a5c7770
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@workspaces/mi/mi-extension/package.json`:
- Around line 117-128: The launch template currently hardcodes credentials in
the JSON properties (managementUsername and managementPassword), which risks
accidental commits; change these to non-secret placeholders (e.g.,
"<your-username>", "<your-password>") or environment-variable references (e.g.,
"${env:MI_MGMT_USER}", "${env:MI_MGMT_PASS}") in the properties object, and add
a brief description/comment in the template output instructing users not to
commit real credentials and to replace placeholders or use env vars instead.
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts`:
- Around line 5439-5442: The call in the params.isRuntimeService branch passes
DebuggerConfig.getHost() unguarded into langClient.swaggerFromAPI which can be
empty/undefined and break local runtime swagger fetch; update the object
construction passed to langClient.swaggerFromAPI (in the same block that
currently calls exposeVersionedServices and computes versionedUrl) to include
host only when DebuggerConfig.getHost() returns a non-empty string (e.g.,
conditionally spread { host: host } or equivalent), keep port
(DebuggerConfig.getServerPort()), projectPath (versionedUrl ? this.projectUri :
"") and the existing conditional swaggerPath inclusion unchanged.
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts`:
- Line 627: Remove the global TLS bypass by deleting the assignment to
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0' in rpc-manager.ts (the
occurrences around where fetch is called with new https.Agent({
rejectUnauthorized: false })). Keep and pass the scoped https.Agent instance to
fetch (e.g., the existing agent usage in the RPC manager functions), and ensure
no other code in this file sets NODE_TLS_REJECT_UNAUTHORIZED (also remove the
similar occurrence near the other call). This ensures TLS is disabled only for
the intended requests via the agent, not globally for the process.
- Around line 709-718: The code path that handles failed login (uses
response.statusText) and the catch block currently only logs/shows errors but
never resolves or rejects the surrounding Promise, causing callers to hang;
update the enclosing async function (the promise returned by the MI visualizer
RPC manager method that fetches runtime services/login) to explicitly resolve or
reject on these error paths—after logging via log(...) and
vscode.window.showErrorMessage(...), either return a rejected Promise with the
error or call the function's resolve/reject handlers so the caller receives a
terminal result; ensure this is done equally in both the failed-login branch
(where response is available) and the catch block (where error is available),
and keep restoring process.env.NODE_TLS_REJECT_UNAUTHORIZED = '1' in finally
as-is.
🧹 Nitpick comments (5)
workspaces/mi/mi-extension/src/debugger/config.ts (1)
198-203: Consider adding input validation for timeout value.The
setConnectionTimeoutmethod acceptstimeoutin seconds and converts to milliseconds, but doesn't validate the input. A non-positive timeout could cause unexpected behavior.🔧 Suggested improvement
public static getConnectionTimeout(): number { return this.connectionTimeout; } public static setConnectionTimeout(timeout: number): void { + if (timeout <= 0) { + throw new Error('Connection timeout must be a positive number'); + } this.connectionTimeout = timeout * 1000; }workspaces/mi/mi-extension/src/debugger/debugger.ts (1)
397-397: Minor inconsistency in error message format.This error message uses
. ${error}while other error messages in this file (e.g., lines 338, 342, 407) use: ${error}. Consider keeping the format consistent.- reject(`Error while resuming the debugger server. ${error}`); + reject(`Error while resuming the debugger server: ${error}`);workspaces/mi/mi-extension/src/debugger/debugAdapter.ts (1)
296-313: Consider ifserverPathis required for remote debugging.The code requires a valid local server path even when remote debugging is enabled. While
setManagementCredentials(serverPath)(line 313) is called before the remote/local branch split, remote debugging already configures credentials from session properties (lines 70-71).Consider whether the server path check and credential setup could be skipped or simplified in remote debugging mode to improve UX for users who don't have a local server installed.
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts (2)
657-706: Sequential API calls could be parallelized for better performance.The three fetch calls to
/management/apis,/management/proxy-services, and/management/data-servicesare executed sequentially but have no dependencies on each other. UsingPromise.all()would reduce total latency.⚡ Proposed refactor to parallelize fetch calls
const authToken = responseBody?.AccessToken; + const headers = { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${authToken}` + }; - const apiResponse = await fetch(`https://${host}:${managementPort}/management/apis`, { - method: 'GET', - headers: { - 'Content-Type': 'application/json', - 'Authorization': `Bearer ${authToken}` - }, - agent: agent // Pass the custom agent - }); - - if (apiResponse.ok) { - const apiResponseData = await apiResponse.json() as RuntimeServiceDetails | undefined; - if (apiResponseData?.list && Array.isArray(apiResponseData.list)) { - apiResponseData.list = apiResponseData.list.map(item => ({ - ...item, - url: item.url ? this.replaceHostname(item.url, host) : item.url - })); - } - runtimeServicesResponse.api = apiResponseData; - } - - - // get the proxy details - const proxyResponse = await fetch(`https://${host}:${managementPort}/management/proxy-services`, { - method: 'GET', - headers: { - 'Content-Type': 'application/json', - 'Authorization': `Bearer ${authToken}` - }, - agent: agent // Pass the custom agent - }); - - if (proxyResponse.ok) { - const proxyResponseData = await proxyResponse.json() as RuntimeServiceDetails | undefined; - runtimeServicesResponse.proxy = proxyResponseData; - } - - // get the data services details - const dataServicesResponse = await fetch(`https://${host}:${managementPort}/management/data-services`, { - method: 'GET', - headers: { - 'Content-Type': 'application/json', - 'Authorization': `Bearer ${authToken}` - }, - agent: agent // Pass the custom agent - }); - - if (dataServicesResponse.ok) { - const dataServicesResponseData = await dataServicesResponse.json() as RuntimeServiceDetails | undefined; - runtimeServicesResponse.dataServices = dataServicesResponseData; - } + const [apiResponse, proxyResponse, dataServicesResponse] = await Promise.all([ + fetch(`https://${host}:${managementPort}/management/apis`, { method: 'GET', headers, agent }), + fetch(`https://${host}:${managementPort}/management/proxy-services`, { method: 'GET', headers, agent }), + fetch(`https://${host}:${managementPort}/management/data-services`, { method: 'GET', headers, agent }) + ]); + + if (apiResponse.ok) { + const apiResponseData = await apiResponse.json() as RuntimeServiceDetails | undefined; + if (apiResponseData?.list && Array.isArray(apiResponseData.list)) { + apiResponseData.list = apiResponseData.list.map(item => ({ + ...item, + url: item.url ? this.replaceHostname(item.url, host) : item.url + })); + } + runtimeServicesResponse.api = apiResponseData; + } + + if (proxyResponse.ok) { + const proxyResponseData = await proxyResponse.json() as RuntimeServiceDetails | undefined; + runtimeServicesResponse.proxy = proxyResponseData; + } + + if (dataServicesResponse.ok) { + const dataServicesResponseData = await dataServicesResponse.json() as RuntimeServiceDetails | undefined; + runtimeServicesResponse.dataServices = dataServicesResponseData; + }
969-978: Consider makingreplaceHostnamea private method.This method appears to be an internal helper used only within
getAvailableRuntimeServices. Making itprivatewould clarify the API surface and prevent external callers from depending on it.♻️ Proposed change to private visibility
- replaceHostname(originalUrl: string, targetHost: string) { + private replaceHostname(originalUrl: string, targetHost: string): string { try { const urlObj = new URL(originalUrl); urlObj.hostname = targetHost; return urlObj.toString(); } catch (e) { log(`Failed to parse URL. originalUrl="${originalUrl}", targetHost="${targetHost}", error=${e}`); return originalUrl; } }Based on learnings: "Document all public APIs and complex algorithms inline with JSDoc comments explaining parameters, return types, and usage examples." If this method remains public, consider adding JSDoc documentation.
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Show resolved
Hide resolved
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts`:
- Around line 652-660: The parsed login response (responseBody) may not contain
an AccessToken so authToken can be undefined; before calling fetch to
`/management/apis` (the fetch block using 'Authorization': `Bearer
${authToken}`), validate authToken (e.g., check responseBody?.AccessToken) and
if missing, log an explicit error and abort/return/throw to avoid sending
`Bearer undefined`—update the code around responseBody and authToken to handle
the missing-token path and ensure no subsequent requests are made with an
undefined Authorization header.
🧹 Nitpick comments (1)
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts (1)
967-976: Consider markingreplaceHostnameas private.This method appears to be an internal utility used only within this class. Making it
privatewould clarify the intended scope and prevent unintended external usage.♻️ Suggested change
- replaceHostname(originalUrl: string, targetHost: string) { + private replaceHostname(originalUrl: string, targetHost: string): string { try { const urlObj = new URL(originalUrl); urlObj.hostname = targetHost; return urlObj.toString(); } catch (e) { log(`Failed to parse URL. originalUrl="${originalUrl}", targetHost="${targetHost}", error=${e}`); return originalUrl; } }
workspaces/mi/mi-extension/src/rpc-managers/mi-visualizer/rpc-manager.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
workspaces/mi/mi-extension/src/debugger/debugAdapter.ts (2)
311-312:⚠️ Potential issue | 🟡 MinorDuplicate
setVmArgscall.Line 311 and Line 312 are identical. Remove the duplicate.
Proposed fix
DebuggerConfig.setEnvVariables(args?.env || {}); DebuggerConfig.setVmArgs(args?.vmArgs ? args?.vmArgs : []); - - DebuggerConfig.setVmArgs(args?.vmArgs ? args?.vmArgs : []);
379-404:⚠️ Potential issue | 🟠 Major
disconnectRequestattempts to stop server unconditionally.When remote debugging, calling
stopServerwill likely fail or behave unexpectedly since the server is running on a remote host. Consider skipping the server stop logic for remote debugging sessions.Proposed fix
protected async disconnectRequest(response: DebugProtocol.DisconnectResponse, args?: DebugProtocol.DisconnectArguments, request?: DebugProtocol.Request): Promise<void> { this.debuggerHandler?.closeDebugger(); vscode.commands.executeCommand('setContext', 'MI.isRunning', 'false'); + + if (DebuggerConfig.isRemoteDebuggingEnabled()) { + response.success = true; + this.sendResponse(response); + extension.isServerStarted = false; + RPCLayer._messengers.get(this.projectUri)?.sendNotification(miServerRunStateChanged, { type: 'webview', webviewType: 'micro-integrator.runtime-services-panel' }, 'Stopped'); + return; + } + try { if (process.platform === 'win32') {
🤖 Fix all issues with AI agents
In `@workspaces/mi/mi-extension/src/debugger/debugAdapter.ts`:
- Around line 314-329: setManagementCredentials(serverPath) is being called
unconditionally and can overwrite remote credentials; move the call into the
local-server branch so it only runs when
DebuggerConfig.isRemoteDebuggingEnabled() is false. Specifically, remove or
relocate the top-level call to setManagementCredentials(serverPath) and invoke
it inside the non-remote branch (the branch that runs executeTasks and then
initializes local debug flow) before executeTasks so remote flows that depend on
debuggerProperties credentials are not clobbered.
🧹 Nitpick comments (1)
workspaces/mi/mi-extension/src/debugger/config.ts (1)
199-204: Consider adding input validation forconnectionTimeout.The setter converts seconds to milliseconds, but doesn't validate that
timeoutis positive. A zero or negative value could cause unexpected behavior in network operations that use this timeout.Proposed fix
public static setConnectionTimeout(timeout: number): void { + if (timeout <= 0) { + timeout = 10; // fallback to default + } this.connectionTimeout = timeout * 1000; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/mi/mi-extension/src/debugger/debugAdapter.ts (1)
310-312:⚠️ Potential issue | 🟡 MinorRemove duplicate
setVmArgscall.Line 312 is a duplicate of line 310.
Proposed fix
DebuggerConfig.setEnvVariables(args?.env || {}); DebuggerConfig.setVmArgs(args?.vmArgs ? args?.vmArgs : []); - DebuggerConfig.setVmArgs(args?.vmArgs ? args?.vmArgs : []); - vscode.commands.executeCommand('setContext', 'MI.isRunning', 'true');
This PR will add remote server debugging support to the MI Extension.
Screen.Recording.2026-01-07.at.21.06.36.mov
Summary by CodeRabbit
New Features
Bug Fixes