Skip to content
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

copilot-debug terminal integration breaks python venv activation #237338

Open
dnabb opened this issue Dec 21, 2024 · 7 comments
Open

copilot-debug terminal integration breaks python venv activation #237338

dnabb opened this issue Dec 21, 2024 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug terminal-env-collection Issues related to the EnvironmentVariableCollection API
Milestone

Comments

@dnabb
Copy link

dnabb commented Dec 21, 2024

  • Copilot Chat Extension Version: 0.23.2
  • VS Code Version: 1.96.2
  • OS Version: Windows 11
  • Logs:

Steps to Reproduce:

  1. Start with a clean VSCode profile
  2. Install the python extension
  3. Create a virtual environment and set it as the python interpreter
  4. Open a new terminal and run (get-command python).Source (or equivalent for your shell): it should point to the python interpreter from the virtual environment, because the Python extension activates the environment for you and prepends it to env:PATH
  5. Now, install Copilot Chat and restart VSCode
  6. Open a new terminal and re-run (get-command python).Source: this time, it should point to the global python interpreter.

The issue seems to be that both Python extension and Copilot Chat seem to be trying to modify the PATH at the same time, probably overriding each others changes (even if in my case, Copilot Chat seems to always override Python).

Even worse, Copilot Chat seems to completely ignore the "github.copilot.chat.copilotDebugCommand.enabled": false setting, with the extension still trying to add features to the terminal.
EDIT: Adding a separate issue for this, microsoft/vscode-copilot-release#3499

@connor4312
Copy link
Member

copilot-debug only appends to the path, it doesn't overwrite it. If this is destructive I think there might be something up with how Python does the contribution or how the terminal applies them

https://github.com/microsoft/vscode-copilot/blob/a803c8aa2cdcd8d7a381c663916814b8c9b16479/src/extension/onboardDebug/vscode-node/copilotDebugCommandContribution.ts#L223-L229

@dnabb
Copy link
Author

dnabb commented Dec 22, 2024

copilot-debug only appends to the path, it doesn't overwrite it. If this is destructive I think there might be something up with how Python does the contribution or how the terminal applies them

https://github.com/microsoft/vscode-copilot/blob/a803c8aa2cdcd8d7a381c663916814b8c9b16479/src/extension/onboardDebug/vscode-node/copilotDebugCommandContribution.ts#L223-L229

From what I can see from Show Environment Contribution, both Python and Copilot append/prepend to the PATH by taking the "old path" and prepending/appending to it:

Python:
PATH=c:\Users\myuser\.vscode\extensions\ms-python.python-2024.22.0-win32-x64\python_files\deactivate\powershell;C:\Users\myuser\customer_repos\myfolder\.venv\Scripts;${env:PATH}

Copilot:
PATH=${env:PATH};c:\Users\DanieleAbbatelli\AppData\Roaming\Code\User\globalStorage\github.copilot-chat\debugCommand

My guess is that, for both exensions, the "old path" (${env:PATH}) refers to the PATH before contributions from other extensions.

@gymlt
Copy link

gymlt commented Jan 3, 2025

We are experiencing a similar issue after the update. We are utilising Dev Containers, and if your container is based on any non-Windows operating system, your PATH environment variable will be corrupted due to differing syntax. This results in the PATH appearing as follows:

PATH=/root/.vscode-server/bin/fabdb6a30b49f79a7aba0f2ad9df9b399473380f/bin/remote-cli:/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin;c:\Users\maryte\AppData\Roaming\Code\User\globalStorage\github.copilot-chat\debugCommand

All Linux commands cease to function, leading to numerous similar errors:

root@2412dad5f1da:/workspace# grep
bash: sed: command not found
bash: grep: command not found
root@2412dad5f1da:/workspace#

Could this be looked into? Thank you.

@DonJayamanne DonJayamanne removed their assignment Jan 5, 2025
@Tyriar
Copy link
Member

Tyriar commented Jan 6, 2025

My guess is that, for both exensions, the "old path" (${env:PATH}) refers to the PATH before contributions from other extensions.

I think that's the case and to fix this we will need to do an initial pass of all the mutators to merge multiple append/replace mutators that target the same environment variable (mainly PATH) together.

Relevant code:

async applyToProcessEnvironment(env: IProcessEnvironment, scope: EnvironmentVariableScope | undefined, variableResolver?: VariableResolver): Promise<void> {
let lowerToActualVariableNames: { [lowerKey: string]: string | undefined } | undefined;
if (isWindows) {
lowerToActualVariableNames = {};
Object.keys(env).forEach(e => lowerToActualVariableNames![e.toLowerCase()] = e);
}
for (const [variable, mutators] of this.getVariableMap(scope)) {
const actualVariable = isWindows ? lowerToActualVariableNames![variable.toLowerCase()] || variable : variable;
for (const mutator of mutators) {
const value = variableResolver ? await variableResolver(mutator.value) : mutator.value;
// Default: true
if (mutator.options?.applyAtProcessCreation ?? true) {
switch (mutator.type) {
case EnvironmentVariableMutatorType.Append:
env[actualVariable] = (env[actualVariable] || '') + value;
break;
case EnvironmentVariableMutatorType.Prepend:
env[actualVariable] = value + (env[actualVariable] || '');
break;
case EnvironmentVariableMutatorType.Replace:
env[actualVariable] = value;
break;
}
}
// Default: false
if (mutator.options?.applyAtShellIntegration ?? false) {
const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + this._encodeColons(value);
}
}
}
}

@Tyriar
Copy link
Member

Tyriar commented Jan 6, 2025

Also I believe this method of activating Python terminals will get removed soon. There's a setting that @anthonykim1 can point you to in the meantime.

@Tyriar Tyriar transferred this issue from microsoft/vscode-copilot-release Jan 6, 2025
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal-env-collection Issues related to the EnvironmentVariableCollection API labels Jan 6, 2025
@Tyriar Tyriar added this to the Backlog milestone Jan 6, 2025
@martincpt
Copy link

I wonder if my issue is related but since last update my vscode keep forgot to which python interpreter i used on that specific workspace, which is quite irritating since I have to set it over and over again every time I open a project.

I also use python.defaultInterpreterPath setting my venv dynamically but that only applies to my terminal and after a few restarts it started working. Right after the update it just didn't work for a while.

@connor4312
Copy link
Member

I think that's the case and to fix this we will need to do an initial pass of all the mutators to merge multiple append/replace mutators that target the same environment variable (mainly PATH) together.

It looks like this works fine for things that happen at applyAtProcessCreation -- except for the "replace" type mutations. I don't have a good understanding of what Python is doing here though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug terminal-env-collection Issues related to the EnvironmentVariableCollection API
Projects
None yet
Development

No branches or pull requests

9 participants