Skip to content

Commit 2151183

Browse files
eleanorjboydCopilot
andcommitted
Address Copilot review feedback
- Strip surrounding quotes from executable before passing as --python to uv; quoting the argument value causes uv to fail to resolve the interpreter (mirrors the same fix in runInBackground.ts) - Add regression test: quoted python path under uv mode is unquoted - Update shouldUseUv JSDoc to document the new scope parameter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9b4b7be commit 2151183

3 files changed

Lines changed: 60 additions & 4 deletions

File tree

src/features/execution/runAsTask.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,14 @@ export async function runAsTask(
3838
const useUv = await shouldUseUv(undefined, environment.environmentPath.fsPath, options.project?.uri);
3939

4040
if (useUv) {
41-
allArgs.unshift('--python', executable);
41+
// Strip surrounding quotes before passing as --python value; uv receives the raw path
42+
// and shell-quoting the argument value causes it to fail to resolve the interpreter.
43+
// (cf. runInBackground.ts which strips quotes for the same reason before spawn)
44+
let pythonArg = executable;
45+
if (pythonArg.startsWith('"') && pythonArg.endsWith('"')) {
46+
pythonArg = pythonArg.substring(1, pythonArg.length - 1);
47+
}
48+
allArgs.unshift('--python', pythonArg);
4249
allArgs.unshift('run');
4350
executable = 'uv';
4451
}

src/managers/builtin/helpers.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,15 @@ export async function isUvInstalled(log?: LogOutputChannel): Promise<boolean> {
3636

3737
/**
3838
* Determines if uv should be used for managing a virtual environment.
39-
* @param log - Optional log output channel for logging operations
40-
* @param envPath - Optional environment path to check against UV environments list
41-
* @returns True if uv should be used, false otherwise. For UV environments, returns true if uv is installed. For other environments, checks the 'python-envs.alwaysUseUv' setting and uv availability.
39+
* @param log - Optional log output channel for logging operations.
40+
* @param envPath - Optional environment path to check against the known uv environments list.
41+
* @param scope - Optional configuration scope used when reading the `python-envs.alwaysUseUv` setting.
42+
* Pass the relevant project or workspace-folder `Uri` when available so VS Code resolves settings
43+
* using normal precedence: workspace folder, then workspace, then user/global. If omitted, the
44+
* user/global value is used unless VS Code can infer a broader scope.
45+
* @returns True if uv should be used, false otherwise. For uv-managed environments, returns true
46+
* if uv is installed. For other environments, checks the `python-envs.alwaysUseUv` setting for
47+
* the provided scope and uv availability.
4248
*/
4349
export async function shouldUseUv(log?: LogOutputChannel, envPath?: string, scope?: ConfigurationScope): Promise<boolean> {
4450
if (envPath) {

src/test/features/execution/runAsTask.unit.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,49 @@ suite('runAsTask Tests', () => {
715715
);
716716
});
717717

718+
test('should strip surrounding quotes from executable before passing as --python to uv (regression guard)', async () => {
719+
// Mock - executable is already quoted (e.g. from a shell-escape step or a provider that returns
720+
// a quoted path). uv must receive the raw path without the surrounding quotes or it fails to
721+
// locate the interpreter.
722+
const quotedPython = '"C:\\Program Files\\Python311\\python.exe"';
723+
const unquotedPython = 'C:\\Program Files\\Python311\\python.exe';
724+
const environment: PythonEnvironment = {
725+
envId: { id: 'test-env', managerId: 'test-manager' },
726+
name: 'Test Environment',
727+
displayName: 'Test Environment',
728+
displayPath: 'C:\\Program Files\\Python311',
729+
version: '3.11.0',
730+
environmentPath: Uri.file(unquotedPython),
731+
execInfo: {
732+
run: { executable: quotedPython, args: [] },
733+
},
734+
sysPrefix: 'C:\\Program Files\\Python311',
735+
};
736+
737+
const options: PythonTaskExecutionOptions = {
738+
name: 'Quoted Python UV Task',
739+
args: ['script.py'],
740+
};
741+
742+
mockGetWorkspaceFolder.returns(undefined);
743+
mockShouldUseUv.resolves(true);
744+
mockQuoteStringIfNecessary.withArgs('uv').returns('uv');
745+
mockExecuteTask.resolves({} as TaskExecution);
746+
747+
// Run
748+
await runAsTask(environment, options);
749+
750+
// Assert - the --python value must be the unquoted path so uv can resolve it
751+
const taskArg = mockExecuteTask.firstCall.args[0] as Task;
752+
const execution = taskArg.execution as ShellExecution;
753+
assert.strictEqual(execution.command, 'uv', 'Should execute uv when uv mode is enabled');
754+
assert.deepStrictEqual(
755+
execution.args,
756+
['run', '--python', unquotedPython, 'script.py'],
757+
'Should strip surrounding quotes from the python path before passing to uv --python',
758+
);
759+
});
760+
718761
test('should pass user args containing flags through to python under uv (regression guard)', async () => {
719762
// Mock - The run button only ever appends a file path, but API callers can pass arbitrary args.
720763
// This guards the contract that user args land after the script positional and are NOT consumed by uv.

0 commit comments

Comments
 (0)