Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 49 additions & 20 deletions core/context/mcp/MCPConnection.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js";
import * as URI from "uri-js";
import { fileURLToPath } from "url";

import {
Expand Down Expand Up @@ -445,6 +446,28 @@ Org-level secrets can only be used for MCP by Background Agents (https://docs.co
};
}

/**
* Converts a URI (file://, vscode-remote://, etc.) to a filesystem path.
* @param uri The URI to convert
* @returns The filesystem path
*/
private uriToFsPath(uri: string): string {
// Handle file:// URIs using Node's built-in converter
if (uri.startsWith("file://")) {
return fileURLToPath(uri);
}

// For other URI schemes (vscode-remote://, etc.), extract the path portion
const parsed = URI.parse(uri);
if (parsed.scheme && parsed.path) {
// The path from URI.parse already includes the leading /
return parsed.path;
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path extracted from URI.parse() needs to be decoded using decodeURIComponent() to handle URL-encoded characters (spaces, special characters, etc.). Without decoding, paths like "vscode-remote://ssh-remote+myserver/home/user/my%20project" will become "/home/user/my%20project" instead of "/home/user/my project".

This is consistent with how URIs are handled elsewhere in the codebase (see core/util/paths.ts lines 495 and 504, and core/tools/implementations/runTerminalCommand.ts line 71 which uses decodeURIComponent(url.pathname) for the same purpose).

Suggested change
return parsed.path;
return decodeURIComponent(parsed.path);

Copilot uses AI. Check for mistakes.
}

// If it's not a URI, return as-is
return uri;
Comment on lines +455 to +468
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The uriToFsPath method should include error handling for malformed URIs. If URI.parse() encounters an invalid URI or fileURLToPath() throws an error, the method should handle it gracefully rather than propagating the exception. Consider wrapping the parsing and conversion logic in a try-catch block and falling back to returning the input as-is if parsing fails, consistent with the pattern used in core/tools/implementations/runTerminalCommand.ts lines 56-61.

Suggested change
// Handle file:// URIs using Node's built-in converter
if (uri.startsWith("file://")) {
return fileURLToPath(uri);
}
// For other URI schemes (vscode-remote://, etc.), extract the path portion
const parsed = URI.parse(uri);
if (parsed.scheme && parsed.path) {
// The path from URI.parse already includes the leading /
return parsed.path;
}
// If it's not a URI, return as-is
return uri;
try {
// Handle file:// URIs using Node's built-in converter
if (uri.startsWith("file://")) {
return fileURLToPath(uri);
}
// For other URI schemes (vscode-remote://, etc.), extract the path portion
const parsed = URI.parse(uri);
if (parsed.scheme && parsed.path) {
// The path from URI.parse already includes the leading /
return parsed.path;
}
// If it's not a URI, return as-is
return uri;
} catch {
// If parsing or conversion fails, return the input as-is
return uri;
}

Copilot uses AI. Check for mistakes.
}

/**
* Resolves the current working directory of the current workspace.
* @param cwd The cwd parameter provided by user.
Expand All @@ -455,8 +478,9 @@ Org-level secrets can only be used for MCP by Background Agents (https://docs.co
return this.resolveWorkspaceCwd(undefined);
}

if (cwd.startsWith("file://")) {
return fileURLToPath(cwd);
// Check if it's a URI (has a scheme like file://, vscode-remote://, etc.)
if (cwd.includes("://")) {
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using includes("://") to detect URIs is overly broad and could produce false positives. For example, a Windows path like "C://path" would incorrectly be treated as a URI. Consider using a more robust check such as checking for a valid URI scheme pattern (e.g., /^[a-z][a-z0-9+.-]*:///i.test(cwd)) or checking if URI.parse(cwd).scheme is defined.

Suggested change
if (cwd.includes("://")) {
const parsedCwd = URI.parse(cwd);
if (parsedCwd.scheme) {

Copilot uses AI. Check for mistakes.
return this.uriToFsPath(cwd);
}

// Return cwd if cwd is an absolute path.
Expand All @@ -473,10 +497,8 @@ Org-level secrets can only be used for MCP by Background Agents (https://docs.co
const target = cwd ?? ".";
const resolved = await resolveRelativePathInDir(target, IDE);
if (resolved) {
if (resolved.startsWith("file://")) {
return fileURLToPath(resolved);
}
return resolved;
// resolved is a URI, convert to filesystem path
return this.uriToFsPath(resolved);
}
return resolved;
}
Expand Down Expand Up @@ -559,24 +581,31 @@ Org-level secrets can only be used for MCP by Background Agents (https://docs.co
...(options.env ?? {}),
};

// Always try to set a proper PATH
const ideInfo = await this.extras?.ide?.getIdeInfo();
const isWindowsHostWithWslRemote =
process.platform === "win32" && ideInfo?.remoteName === "wsl";

// Set initial PATH - use process.env.PATH if available
if (process.env.PATH !== undefined) {
// Set the initial PATH from process.env
env.PATH = process.env.PATH;
}

// For non-Windows platforms or WSL remotes, try to get the PATH from user shell
const ideInfo = await this.extras?.ide?.getIdeInfo();
const isWindowsHostWithWslRemote =
process.platform === "win32" && ideInfo?.remoteName === "wsl";
if (process.platform !== "win32" || isWindowsHostWithWslRemote) {
try {
const shellEnvPath = await getEnvPathFromUserShell(
ideInfo?.remoteName,
// For non-Windows platforms or remote connections, get proper shell PATH
if (process.platform !== "win32" || isWindowsHostWithWslRemote) {
try {
const shellEnvPath = await getEnvPathFromUserShell(ideInfo?.remoteName);
// Always use shell path if available (it includes defaults as fallback)
if (shellEnvPath) {
env.PATH = shellEnvPath;
}
} catch (err) {
console.error("Error getting PATH from shell:", err);
// If we still don't have a PATH, this is a critical issue
if (!env.PATH) {
console.error(
"WARNING: No PATH set for MCP server. Command execution will likely fail.",
);
if (shellEnvPath && shellEnvPath !== process.env.PATH) {
env.PATH = shellEnvPath;
}
} catch (err) {
console.error("Error getting PATH:", err);
}
}
}
Expand Down
33 changes: 33 additions & 0 deletions core/context/mcp/MCPConnection.vitest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,39 @@ describe("MCPConnection", () => {
);
expect(mockResolve).toHaveBeenCalledWith("src", ide);
});

it("should convert file:// URIs to filesystem paths", async () => {
const conn = new MCPConnection(baseOptions);

await expect(
(conn as any).resolveCwd("file:///home/user/project"),
).resolves.toBe("/home/user/project");
});

it("should convert vscode-remote:// URIs to filesystem paths", async () => {
const conn = new MCPConnection(baseOptions);

await expect(
(conn as any).resolveCwd(
"vscode-remote://ssh-remote+myserver/home/user/project",
),
).resolves.toBe("/home/user/project");
});
Comment on lines +187 to +195
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests should include a test case for URL-encoded paths (e.g., paths with spaces or special characters like "vscode-remote://ssh-remote+myserver/home/user/my%20project") to ensure that decodeURIComponent is applied correctly. This would match the test coverage pattern seen in core/tools/implementations/runTerminalCommand.vitest.ts (lines 578-591) and core/tools/implementations/resolveWorkingDirectory.vitest.ts (lines 55-74).

Copilot uses AI. Check for mistakes.

it("should handle workspace URIs from remote IDE", async () => {
const ide = {} as any;
const mockResolve = vi
.spyOn(ideUtils, "resolveRelativePathInDir")
.mockResolvedValue(
"vscode-remote://ssh-remote+myserver/home/user/workspace/src",
);
const conn = new MCPConnection(baseOptions, { ide });

await expect((conn as any).resolveCwd("src")).resolves.toBe(
"/home/user/workspace/src",
);
expect(mockResolve).toHaveBeenCalledWith("src", ide);
});
});

describe("connectClient", () => {
Expand Down
39 changes: 33 additions & 6 deletions core/util/shellPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@ import { exec } from "child_process";
import { promisify } from "util";

const execAsync = promisify(exec);

// Common Unix binary paths that should be included in PATH
const DEFAULT_UNIX_PATHS = [
"/usr/local/bin",
"/usr/bin",
"/bin",
"/usr/local/sbin",
"/usr/sbin",
"/sbin",
"/opt/homebrew/bin", // macOS Homebrew on Apple Silicon
"/home/linuxbrew/.linuxbrew/bin", // Linux Homebrew
];

export async function getEnvPathFromUserShell(
remoteName?: string,
): Promise<string | undefined> {
Expand All @@ -11,20 +24,34 @@ export async function getEnvPathFromUserShell(
return undefined;
}

if (!process.env.SHELL) {
return undefined;
}
// Try to find a shell to use
const shell = process.env.SHELL || "/bin/bash";
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If /bin/bash doesn't exist on the system (e.g., on some minimal Docker images or Alpine Linux which uses /bin/sh), the exec command will fail with an ENOENT error. Consider adding /bin/sh as a fallback or checking if the shell exists before executing. Alternatively, use a more robust shell detection strategy that tries multiple common shells.

Copilot uses AI. Check for mistakes.

try {
// Source common profile files
const command = `${process.env.SHELL} -l -c 'for f in ~/.zprofile ~/.zshrc ~/.bash_profile ~/.bashrc; do [ -f "$f" ] && source "$f" 2>/dev/null; done; echo $PATH'`;
const command = `${shell} -l -c 'for f in ~/.zprofile ~/.zshrc ~/.bash_profile ~/.bashrc; do [ -f "$f" ] && source "$f" 2>/dev/null; done; echo $PATH'`;

const { stdout } = await execAsync(command, {
encoding: "utf8",
timeout: 5000, // 5 second timeout
});

return stdout.trim();
const pathFromShell = stdout.trim();
if (pathFromShell) {
return pathFromShell;
}
} catch (error) {
return process.env.PATH; // Fallback to current PATH
// If shell command fails, fall through to default handling
console.warn("Failed to get PATH from shell:", error);
}

// Fallback: build a reasonable default PATH
if (!process.env.PATH) {
return DEFAULT_UNIX_PATHS.join(":");
}

// Merge current PATH with common paths (avoiding duplicates)
const currentPaths = process.env.PATH.split(":");
const allPaths = [...new Set([...currentPaths, ...DEFAULT_UNIX_PATHS])];
return allPaths.join(":");
}
Loading