fix: ensure path input to copilot tools correctly converted to URI

Author: karthiknadig

src/common/utils/pathUtils.ts

@@ -26,3 +26,26 @@ export function normalizePath(path: string): string {
     }
     return path1;
 }
+
+export function getResourceUri(resourcePath: string, root?: string): Uri | undefined {
+    try {
+        if (!resourcePath) {
+            return undefined;
+        }
+
+        const normalizedPath = normalizePath(resourcePath);
+        if (normalizedPath.includes('://')) {
+            return Uri.parse(normalizedPath);
+        }
+
+        const path = require('path');
+        if (!path.isAbsolute(resourcePath) && root) {
+            const absolutePath = path.resolve(root, resourcePath);
+            return Uri.file(absolutePath);
+        }
+
+        return Uri.file(resourcePath);
+    } catch (_err) {
+        return undefined;
+    }
+}

src/features/copilotTools.ts

@@ -15,9 +15,11 @@ import {
     PythonPackageGetterApi,
     PythonPackageManagementApi,
     PythonProjectEnvironmentApi,
+    PythonProjectGetterApi,
 } from '../api';
 import { createDeferred } from '../common/utils/deferred';
 import { EnvironmentManagers } from '../internal.api';
+import { getResourceUri } from '../common/utils/pathUtils';
 
 export interface IResourceReference {
     resourcePath?: string;
@@ -35,7 +37,7 @@ interface EnvironmentInfo {
  */
 export class GetEnvironmentInfoTool implements LanguageModelTool<IResourceReference> {
     constructor(
-        private readonly api: PythonProjectEnvironmentApi & PythonPackageGetterApi,
+        private readonly api: PythonProjectEnvironmentApi & PythonPackageGetterApi & PythonProjectGetterApi,
         private readonly envManagers: EnvironmentManagers,
     ) {}
     /**
@@ -59,7 +61,13 @@ export class GetEnvironmentInfoTool implements LanguageModelTool<IResourceRefere
         if (parameters.resourcePath === undefined || parameters.resourcePath === '') {
             throw new Error('Invalid input: resourcePath is required');
         }
-        const resourcePath: Uri = Uri.parse(parameters.resourcePath);
+
+        const projects = this.api.getPythonProjects();
+        let root = projects.length > 0 ? projects[0].uri.fsPath : undefined;
+        const resourcePath: Uri | undefined = getResourceUri(parameters.resourcePath, root);
+        if (!resourcePath) {
+            throw new Error('Invalid input: Unable to resolve resource path');
+        }
 
         // environment info set to default values
         const envInfo: EnvironmentInfo = {

src/managers/builtin/venvManager.ts

@@ -36,10 +36,11 @@ import * as path from 'path';
 import { NativePythonFinder } from '../common/nativePythonFinder';
 import { PYTHON_EXTENSION_ID } from '../../common/constants';
 import { createDeferred, Deferred } from '../../common/utils/deferred';
-import { getLatest, shortVersion, sortEnvironments } from '../common/utils';
+import { getLatest, isWindows, shortVersion, sortEnvironments } from '../common/utils';
 import { withProgress } from '../../common/window.apis';
 import { VenvManagerStrings } from '../../common/localize';
 import { showErrorMessage } from '../../common/errors/utils';
+import { normalizePath } from '../../common/utils/pathUtils';
 
 export class VenvManager implements EnvironmentManager {
     private collection: PythonEnvironment[] = [];
@@ -201,12 +202,21 @@ export class VenvManager implements EnvironmentManager {
         const changed: Uri[] = [];
         this.fsPathToEnv.forEach((env, uri) => {
             if (env.environmentPath.fsPath === environment.environmentPath.fsPath) {
-                this.fsPathToEnv.delete(uri);
+                this.deleteFsPathToEnv(uri);
                 changed.push(Uri.file(uri));
             }
         });
         return changed;
     }
+    private setFsPathToEnv(fsPath: string, environment: PythonEnvironment): void {
+        this.fsPathToEnv.set(isWindows() ? normalizePath(fsPath) : fsPath, environment);
+    }
+    private getFsPathToEnv(fsPath: string): PythonEnvironment | undefined {
+        return this.fsPathToEnv.get(isWindows() ? normalizePath(fsPath) : fsPath);
+    }
+    private deleteFsPathToEnv(fsPath: string): void {
+        this.fsPathToEnv.delete(isWindows() ? normalizePath(fsPath) : fsPath);
+    }
 
     async refresh(scope: RefreshEnvironmentsScope): Promise<void> {
         return this.internalRefresh(scope, true, VenvManagerStrings.venvRefreshing);
@@ -262,7 +272,7 @@ export class VenvManager implements EnvironmentManager {
             return [];
         }
 
-        const env = this.fsPathToEnv.get(scope.fsPath);
+        const env = this.getFsPathToEnv(scope.fsPath);
         return env ? [env] : [];
     }
 
@@ -279,7 +289,7 @@ export class VenvManager implements EnvironmentManager {
             return this.globalEnv;
         }
 
-        let env = this.fsPathToEnv.get(project.uri.fsPath);
+        let env = this.getFsPathToEnv(project.uri.fsPath);
         if (!env) {
             env = this.findEnvironmentByPath(project.uri.fsPath);
         }
@@ -305,11 +315,11 @@ export class VenvManager implements EnvironmentManager {
                 return;
             }
 
-            const before = this.fsPathToEnv.get(pw.uri.fsPath);
+            const before = this.getFsPathToEnv(pw.uri.fsPath);
             if (environment) {
-                this.fsPathToEnv.set(pw.uri.fsPath, environment);
+                this.setFsPathToEnv(pw.uri.fsPath, environment);
             } else {
-                this.fsPathToEnv.delete(pw.uri.fsPath);
+                this.deleteFsPathToEnv(pw.uri.fsPath);
             }
             await setVenvForWorkspace(pw.uri.fsPath, environment?.environmentPath.fsPath);
 
@@ -330,11 +340,11 @@ export class VenvManager implements EnvironmentManager {
 
             const before: Map<string, PythonEnvironment | undefined> = new Map();
             projects.forEach((p) => {
-                before.set(p.uri.fsPath, this.fsPathToEnv.get(p.uri.fsPath));
+                before.set(p.uri.fsPath, this.getFsPathToEnv(p.uri.fsPath));
                 if (environment) {
-                    this.fsPathToEnv.set(p.uri.fsPath, environment);
+                    this.setFsPathToEnv(p.uri.fsPath, environment);
                 } else {
-                    this.fsPathToEnv.delete(p.uri.fsPath);
+                    this.deleteFsPathToEnv(p.uri.fsPath);
                 }
             });
 
@@ -464,10 +474,10 @@ export class VenvManager implements EnvironmentManager {
 
             if (env) {
                 const found = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals);
-                const previous = this.fsPathToEnv.get(p);
+                const previous = this.getFsPathToEnv(p);
                 const pw = this.api.getPythonProject(Uri.file(p));
                 if (found) {
-                    this.fsPathToEnv.set(p, found);
+                    this.setFsPathToEnv(p, found);
                     if (pw && previous?.envId.id !== found.envId.id) {
                         events.push(() =>
                             this._onDidChangeEnvironment.fire({ uri: pw.uri, old: undefined, new: found }),
@@ -483,7 +493,7 @@ export class VenvManager implements EnvironmentManager {
                     );
                     if (resolved) {
                         // If resolved add it to the collection
-                        this.fsPathToEnv.set(p, resolved);
+                        this.setFsPathToEnv(p, resolved);
                         this.addEnvironment(resolved, false);
                         if (pw && previous?.envId.id !== resolved.envId.id) {
                             events.push(() =>
@@ -497,7 +507,7 @@ export class VenvManager implements EnvironmentManager {
             } else {
                 // There is NO selected venv, then try and choose the venv that is in the workspace.
                 if (sorted.length === 1) {
-                    this.fsPathToEnv.set(p, sorted[0]);
+                    this.setFsPathToEnv(p, sorted[0]);
                 } else {
                     // These are sorted by version and by path length. The assumption is that the user would want to pick
                     // latest version and the one that is closest to the workspace.
@@ -506,7 +516,7 @@ export class VenvManager implements EnvironmentManager {
                         return t && path.normalize(t) === p;
                     });
                     if (found) {
-                        this.fsPathToEnv.set(p, found);
+                        this.setFsPathToEnv(p, found);
                     }
                 }
             }

src/test/common/pathUtils.unit.test.ts

@@ -0,0 +1,129 @@
+import assert from 'node:assert';
+import * as sinon from 'sinon';
+import { Uri } from 'vscode';
+import { getResourceUri, normalizePath } from '../../common/utils/pathUtils';
+import * as utils from '../../managers/common/utils';
+
+suite('Path Utilities', () => {
+    let isWindowsStub: sinon.SinonStub;
+
+    setup(() => {
+        isWindowsStub = sinon.stub(utils, 'isWindows');
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+    suite('getResourceUri', () => {
+        const testRoot = process.cwd();
+
+        test('returns undefined when path is empty', () => {
+            const result = getResourceUri('', testRoot);
+            assert.strictEqual(result, undefined);
+        });
+
+        test('returns undefined when path is undefined', () => {
+            // @ts-ignore: Testing with undefined even though the type doesn't allow it
+            const result = getResourceUri(undefined, testRoot);
+            assert.strictEqual(result, undefined);
+        });
+        test('creates file URI from normal file path', () => {
+            const testPath = '/path/to/file.txt';
+            const result = getResourceUri(testPath, testRoot);
+
+            assert.ok(result instanceof Uri);
+            assert.strictEqual(result?.scheme, 'file');
+            assert.strictEqual(result?.path, testPath);
+        });
+
+        test('creates file URI from Windows path', () => {
+            const testPath = 'C:\\path\\to\\file.txt';
+            const result = getResourceUri(testPath, testRoot);
+
+            assert.ok(result instanceof Uri);
+            assert.strictEqual(result?.scheme, 'file');
+            // path in the URI should use forward slashes
+            assert.strictEqual(result?.path.includes('\\'), false);
+        });
+
+        test('parses existing URI correctly', () => {
+            const uriString = 'scheme://authority/path';
+            const result = getResourceUri(uriString, testRoot);
+
+            assert.ok(result instanceof Uri);
+            assert.strictEqual(result?.scheme, 'scheme');
+            assert.strictEqual(result?.authority, 'authority');
+            assert.strictEqual(result?.path, '/path');
+        });
+
+        test('handles exception and returns undefined', () => {
+            // Create a scenario that would cause an exception
+            // For this test, we'll mock Uri.file to throw an error
+            const originalUriFile = Uri.file;
+            Uri.file = () => {
+                throw new Error('Test error');
+            };
+            try {
+                const result = getResourceUri('some-path', testRoot);
+                assert.strictEqual(result, undefined);
+            } finally {
+                // Restore the original function
+                Uri.file = originalUriFile;
+            }
+        });
+
+        test('handles relative paths by resolving against the provided root', () => {
+            const path = require('path');
+
+            // Use a relative path
+            const relativePath = './relative/path/file.txt';
+            const customRoot = path.join(testRoot, 'custom/root');
+
+            const result = getResourceUri(relativePath, customRoot);
+
+            assert.ok(result instanceof Uri);
+            assert.strictEqual(result?.scheme, 'file');
+            // The resulting path should be resolved against the custom root
+            assert.ok(
+                result!.fsPath.replace(/\\/g, '/').toLowerCase().endsWith('relative/path/file.txt'),
+                `Expected path to end with the relative path segment, but got: ${result!.fsPath}`,
+            );
+
+            // Verify the path contains the custom root
+            const normalizedResult = result!.fsPath.replace(/\\/g, '/').toLowerCase();
+            const normalizedRoot = customRoot.replace(/\\/g, '/').toLowerCase();
+            assert.ok(
+                normalizedResult.includes(normalizedRoot),
+                `Expected path to include the custom root "${normalizedRoot}", but got: ${normalizedResult}`,
+            );
+        });
+    });
+
+    suite('normalizePath', () => {
+        test('replaces backslashes with forward slashes', () => {
+            const testPath = 'C:\\path\\to\\file.txt';
+            const result = normalizePath(testPath);
+
+            assert.strictEqual(result.includes('\\'), false);
+            assert.strictEqual(result, 'C:/path/to/file.txt');
+        });
+
+        test('converts to lowercase on Windows', () => {
+            isWindowsStub.returns(true);
+
+            const testPath = 'C:/Path/To/File.txt';
+            const result = normalizePath(testPath);
+
+            assert.strictEqual(result, 'c:/path/to/file.txt');
+        });
+
+        test('preserves case on non-Windows', () => {
+            isWindowsStub.returns(false);
+
+            const testPath = 'C:/Path/To/File.txt';
+            const result = normalizePath(testPath);
+
+            assert.strictEqual(result, 'C:/Path/To/File.txt');
+        });
+    });
+});

src/test/copilotTools.unit.test.ts

@@ -10,6 +10,7 @@ import {
     PythonPackageGetterApi,
     PythonPackageManagementApi,
     PythonProjectEnvironmentApi,
+    PythonProjectGetterApi,
 } from '../api';
 import { createDeferred } from '../common/utils/deferred';
 import {
@@ -239,16 +240,14 @@ suite('InstallPackageTool Tests', () => {
 
 suite('GetEnvironmentInfoTool Tests', () => {
     let getEnvironmentInfoTool: GetEnvironmentInfoTool;
-    let mockApi: typeMoq.IMock<PythonProjectEnvironmentApi & PythonPackageGetterApi & PythonPackageManagementApi>;
+    let mockApi: typeMoq.IMock<PythonProjectEnvironmentApi & PythonPackageGetterApi & PythonProjectGetterApi>;
     let mockEnvironment: typeMoq.IMock<PythonEnvironment>;
     let em: typeMoq.IMock<EnvironmentManagers>;
     let managerSys: typeMoq.IMock<InternalEnvironmentManager>;
 
     setup(() => {
         // Create mock functions
-        mockApi = typeMoq.Mock.ofType<
-            PythonProjectEnvironmentApi & PythonPackageGetterApi & PythonPackageManagementApi
-        >();
+        mockApi = typeMoq.Mock.ofType<PythonProjectEnvironmentApi & PythonPackageGetterApi & PythonProjectGetterApi>();
         mockEnvironment = typeMoq.Mock.ofType<PythonEnvironment>();
 
         // eslint-disable-next-line @typescript-eslint/no-explicit-any