Fix deactivate: command not found when deactivating restored or half-activated terminals (#1529)

## Description

### TL;DR

In some scenarios a terminal ends up with `VIRTUAL_ENV` still set but
the `deactivate` shell function undefined. When the extension then
issues a deactivate against that terminal, the shell prints `deactivate:
command not found`. This PR makes the extension's deactivation command
**safe to send even when the function isn't there**: it now checks
first, and only calls `deactivate` if it actually exists.

This is a small, contained fix that removes the user-visible error in
#1490. It does not redesign the underlying activation tracking — that's
a separate, larger piece of work outlined at the bottom of this
description.

---

### Background — why this bug happens

The immediate cause of the error is simple: **the `deactivate` shell
function is not defined in the terminal when the extension tries to call
it.** The shell then prints `command not found`.

How does that happen? When the extension activates a Python venv, two
different things get set up inside the shell:

1. An **environment variable** called `VIRTUAL_ENV` is set, and the
venv's `bin`/`Scripts` directory is prepended to `PATH`. These are
**exported**, so they're carried into child processes.
2. A **shell function** called `deactivate` is defined. It knows how to
undo step 1 — restore the old `PATH`, unset `VIRTUAL_ENV`, fix the
prompt, and remove itself. **Shell functions are not exported** and
don't carry across process boundaries the way env vars do.

That asymmetry means there are several real-world scenarios in which
`VIRTUAL_ENV` is still set but `deactivate` has gone missing — the shell
looks "activated" but can't undo it. Known triggers include:

- The user (or a script) ran `unset -f deactivate`, which is the easiest
way to reproduce the bug deliberately.
- The user ran a subshell (`bash` inside `zsh`, etc.). The subshell
inherits exported env vars but not the parent's function definitions.
- A VS Code terminal restore path that actually re-spawns the shell
process (for example, certain remote / dev-container reconnection
scenarios). VS Code's normal persistent-terminal feature keeps the shell
process alive across window reloads, in which case the function *is*
preserved; the buggy paths are the ones where a fresh shell process is
started but `VIRTUAL_ENV` is reintroduced via the new shell's
environment.

In every one of those scenarios, the resulting state is the same:
`VIRTUAL_ENV` set, `PATH` modified, prompt still shows `(.venv)`, but
`deactivate` is gone. The wrap added by this PR fixes the user-visible
error in **all** of them — it doesn't need to know which trigger fired,
only whether `deactivate` is callable right now.

---

### What this PR changes

This PR fixes the symptom at the smallest possible code surface: the
**one function** the extension uses to generate the deactivation command
before sending it to the terminal.

**Files changed:**

- `src/features/terminal/shells/common/shellUtils.ts` — adds
`wrapDeactivationCommand(shell, command)`.
- `src/features/common/activation.ts` — calls the wrap from
`getDeactivationCommand`.
- `src/test/features/terminal/shells/common/shellUtils.unit.test.ts` —
21 new unit tests covering every supported shell and command shape.

**What `wrapDeactivationCommand` does:**

For shells where the deactivation command is the bare token `deactivate`
(bash, zsh, fish, pwsh, gitbash), the wrap rewrites the command into a
**guarded call**: check whether `deactivate` exists, and only run it if
it does. For example, for bash/zsh/gitbash the command becomes:

```bash
 command -v deactivate >/dev/null 2>&1 && deactivate
```

(The leading space is preserved so the command stays out of shell
history, just like before.)

For shells where the deactivation command is something else — `conda
deactivate`, `pyenv shell --unset`, `deactivate.bat` on cmd, fish's
`overlay hide`, or anything we don't recognize — the wrap returns the
command **unchanged**. We only protect bare-token cases where we're
confident a missing function is the problem.

**Where the wrap hooks in:**

In exactly one place: `getDeactivationCommand` in
`src/features/common/activation.ts`. That function is the single
chokepoint called by both deactivation paths in
`terminalActivationState.ts` (`deactivateLegacy` and
`deactivateUsingShellIntegration`), so every extension-initiated
deactivation now flows through the wrap. No other code path is touched.
Activation paths are completely unaffected.

**Result:**

- On `main`, deactivating a half-activated terminal produces a visible
`bash: deactivate: command not found` error.
- With this PR, the same scenario silently no-ops — no error message, no
state change.

---

### What this PR does NOT fix

This is important to call out this is a "surgical" fix so reviewers and
users have the right expectations.

The PR removes the visible error but **does not clean up the
half-activated state**. After deactivating a terminal that was in the
broken state:

- `$VIRTUAL_ENV` is still set.
- `$PATH` still has the venv directory prepended.
- The prompt still shows `(.venv)`.
- The extension's internal tracker considers the terminal "deactivated,"
so subsequent deactivate commands are no-ops.

The terminal is in a recoverable but mildly confusing state. The user
can recover by closing the terminal and opening a new one (which
auto-activates cleanly), or by manually unsetting `VIRTUAL_ENV` and
resetting `PS1`.

The reason we don't address this in the same PR is that fully cleaning
up the half-state requires changes that are **fundamentally bigger and
riskier** — the entire section below explains why.

---

### Why a structural fix is needed (and why it's a separate piece of
work)

The wrap removes the visible error, but it doesn't prevent the
underlying half-activated state from arising. To prevent it, the rc
snippet that the extension installs in `shellStartup` mode needs to be
smarter than it is today.

Today's snippet uses an exported environment variable,
`VSCODE_PYTHON_AUTOACTIVATE_GUARD`, as a boolean "I've already
activated, skip" flag. The design has two practical problems:

- **The guard is exported, so it propagates to places the `deactivate`
function does not.** A child shell or a re-spawned shell can inherit the
guard, decide it's "already activated," and skip activation — including
the part that defines `deactivate`. That's one way the half-state can
arise on its own, without the user doing anything.
- **The guard tracks a synthetic boolean rather than the real state.**
Even if `deactivate` is missing for an unrelated reason (a plugin unset
it, the user ran `unset -f`, an `exec` replaced the process), the guard
still says "activated" and the snippet declines to re-define the
function.

So while the guard isn't the *only* way `deactivate` can go missing,
it's a design that lets the half-state stick around even when the shell
has a fresh chance to fix itself.

#### Why this is a separate, larger PR

There are three real costs that make this not a casual one-line change:

**1. It changes behavior in interactive subshells.**
Today, opening `bash` inside an activated `zsh` silently inherits the
guard and skips activation. After the structural fix, the subshell would
re-run `activate` because it doesn't see `VIRTUAL_ENV` and `deactivate`
together. That means the venv's `bin` directory gets prepended to `PATH`
a second time, leading to visible PATH duplication, and `deactivate` in
the subshell only partially undoes things. Manageable but real.

**2. Existing users wouldn't get the fix without a migration.**
The code that installs the rc snippet — `editUtils.ts hasStartupCode` —
only checks for region markers and an environment-key string. It does
not compare the script version. So if a user already has the old snippet
in their `.bashrc`/`.zshrc`/etc., the extension thinks setup is done and
never rewrites it. The version comment in the snippet today is for
humans, not code.

Shipping a new snippet without a migration path means **only new users
get the fix**. Everyone who already enabled `shellStartup` mode would
still have the broken guard in their rc file. A full structural fix
needs:

- A parseable version line inside the managed region.
- A `getStartupCodeVersion` helper that reads it.
- A `setupStartup` path that detects a version mismatch and rewrites the
block.
- Atomic write-with-rename to avoid corrupting rc files mid-migration.
- Careful preservation of everything outside the managed markers.

Editing user rc files automatically is a sensitive operation. It needs
its own PR with its own review and its own tests.

That's a multi-PR effort — appropriate for the root-cause fix,
inappropriate to bolt onto a small bug-fix PR.

---

Author: StellaHuang95

src/features/common/activation.ts

@@ -4,6 +4,7 @@ import {
     getShellActivationCommand,
     getShellCommandAsString,
     getShellDeactivationCommand,
+    wrapDeactivationCommand,
 } from '../terminal/shells/common/shellUtils';
 import { identifyTerminalShell } from './shellDetector';
 
@@ -28,7 +29,7 @@ export function getDeactivationCommand(terminal: Terminal, environment: PythonEn
     const shell = identifyTerminalShell(terminal);
     const command = getShellDeactivationCommand(shell, environment);
     if (command) {
-        return getShellCommandAsString(shell, command);
+        return wrapDeactivationCommand(shell, getShellCommandAsString(shell, command));
     }
     return undefined;
 }

src/features/terminal/shells/common/shellUtils.ts

@@ -50,6 +50,68 @@ export function getShellCommandAsString(shell: string, command: PythonCommandRun
     return commandStr;
 }
 
+// Shells whose bare `deactivate` command is a shell function/alias defined by venv's
+// `activate` script. These can disappear (e.g. on session restore where VIRTUAL_ENV
+// persists but the shell function does not), so we must guard the call with an
+// existence check before sending it to the terminal — otherwise the shell prints
+// `deactivate: command not found`.
+const bareDeactivateGuardByShell = new Map<string, (cmd: string) => string>([
+    // POSIX-family shells: `command -v <name>` is the portable existence check.
+    [ShellConstants.BASH, (cmd) => `command -v deactivate >/dev/null 2>&1 && ${cmd.trimStart()}`],
+    [ShellConstants.SH, (cmd) => `command -v deactivate >/dev/null 2>&1 && ${cmd.trimStart()}`],
+    [ShellConstants.ZSH, (cmd) => `command -v deactivate >/dev/null 2>&1 && ${cmd.trimStart()}`],
+    [ShellConstants.KSH, (cmd) => `command -v deactivate >/dev/null 2>&1 && ${cmd.trimStart()}`],
+    [ShellConstants.GITBASH, (cmd) => `command -v deactivate >/dev/null 2>&1 && ${cmd.trimStart()}`],
+    // fish uses `functions -q` for function existence.
+    [ShellConstants.FISH, (cmd) => `functions -q deactivate; and ${cmd.trimStart()}`],
+    // PowerShell: Get-Command returns silently if not found with -ErrorAction SilentlyContinue.
+    [ShellConstants.PWSH, (cmd) => `if (Get-Command deactivate -ErrorAction SilentlyContinue) { ${cmd.trimStart()} }`],
+]);
+
+/**
+ * Returns the bare `deactivate` token if and only if `command` represents a single,
+ * bare invocation of a shell function/alias literally named `deactivate` — meaning
+ * it is safe and meaningful to gate on the existence of that function in the shell.
+ *
+ * Returns `undefined` for anything else (full paths like `path/to/deactivate.bat`,
+ * multi-token forms like `conda deactivate`, `pyenv shell --unset`, `overlay hide ...`,
+ * etc.), since those have different failure modes that should not be silently swallowed.
+ *
+ * The token is normalized to lowercase so the generated guard is consistent across
+ * shells (notably PowerShell, which is case-insensitive).
+ */
+function bareDeactivateInvocation(command: string): string | undefined {
+    const trimmed = command.trim();
+    return trimmed.toLowerCase() === 'deactivate' ? 'deactivate' : undefined;
+}
+
+/**
+ * Wraps a deactivation command in a shell-specific existence guard so that sending
+ * it to a terminal where the `deactivate` shell function no longer exists does not
+ * print `deactivate: command not found`.
+ *
+ * Only applies when the command is a single bare `deactivate` token and the shell
+ * has a known guard template. All other deactivation forms (cmd's `deactivate.bat`
+ * path, `conda deactivate`, `pyenv shell --unset`, nu's `overlay hide ...`, etc.)
+ * are returned unchanged — their failure modes are legitimate and should surface.
+ */
+export function wrapDeactivationCommand(shell: string, command: string): string {
+    const bare = bareDeactivateInvocation(command);
+    if (!bare) {
+        return command;
+    }
+    const guard = bareDeactivateGuardByShell.get(shell);
+    if (!guard) {
+        return command;
+    }
+    const wrapped = guard(bare);
+    // Preserve the leading-space history-ignore behavior for shells that honor it.
+    if (shellsWithLeadingSpaceHistorySupport.has(shell)) {
+        return ` ${wrapped}`;
+    }
+    return wrapped;
+}
+
 export function normalizeShellPath(filePath: string, shellType?: string): string {
     if (isWindows() && shellType) {
         if (shellType.toLowerCase() === ShellConstants.GITBASH || shellType.toLowerCase() === 'git-bash') {

src/test/features/terminal/shells/common/shellUtils.unit.test.ts

@@ -7,6 +7,7 @@ import {
     PROFILE_TAG_END,
     PROFILE_TAG_START,
     shellsWithLeadingSpaceHistorySupport,
+    wrapDeactivationCommand,
 } from '../../../../../features/terminal/shells/common/shellUtils';
 
 suite('Shell Utils', () => {
@@ -205,4 +206,97 @@ suite('Shell Utils', () => {
             });
         });
     });
+
+    suite('wrapDeactivationCommand', () => {
+        // Each tuple: [shell, expected guard substring]. Guards must include the bare
+        // `deactivate` token so the wrapped command still deactivates when the function
+        // exists. See issue #1490.
+        const posixShellsCases: Array<[string, string]> = [
+            [ShellConstants.BASH, 'command -v deactivate >/dev/null 2>&1 && deactivate'],
+            [ShellConstants.SH, 'command -v deactivate >/dev/null 2>&1 && deactivate'],
+            [ShellConstants.ZSH, 'command -v deactivate >/dev/null 2>&1 && deactivate'],
+            [ShellConstants.KSH, 'command -v deactivate >/dev/null 2>&1 && deactivate'],
+            [ShellConstants.GITBASH, 'command -v deactivate >/dev/null 2>&1 && deactivate'],
+        ];
+
+        posixShellsCases.forEach(([shell, expected]) => {
+            test(`wraps bare deactivate with existence guard for ${shell}`, () => {
+                const result = wrapDeactivationCommand(shell, 'deactivate');
+                assert.ok(
+                    result.includes(expected),
+                    `Expected wrapped command to contain '${expected}', got '${result}'`,
+                );
+            });
+
+            test(`tolerates a leading space (history-ignore) on input for ${shell}`, () => {
+                const result = wrapDeactivationCommand(shell, ' deactivate');
+                assert.ok(
+                    result.includes(expected),
+                    `Expected wrapped command to contain '${expected}', got '${result}'`,
+                );
+            });
+        });
+
+        test('preserves leading space for shells that support history-ignore', () => {
+            // bash/zsh/gitbash get a leading space so HISTCONTROL=ignorespace works.
+            assert.ok(wrapDeactivationCommand(ShellConstants.BASH, 'deactivate').startsWith(' '));
+            assert.ok(wrapDeactivationCommand(ShellConstants.ZSH, 'deactivate').startsWith(' '));
+            assert.ok(wrapDeactivationCommand(ShellConstants.GITBASH, 'deactivate').startsWith(' '));
+        });
+
+        test('does not add leading space for shells without history-ignore support', () => {
+            assert.ok(!wrapDeactivationCommand(ShellConstants.PWSH, 'deactivate').startsWith(' '));
+            assert.ok(!wrapDeactivationCommand(ShellConstants.FISH, 'deactivate').startsWith(' '));
+        });
+
+        test('wraps bare deactivate with functions -q for fish', () => {
+            const result = wrapDeactivationCommand(ShellConstants.FISH, 'deactivate');
+            assert.strictEqual(result, 'functions -q deactivate; and deactivate');
+        });
+
+        test('wraps bare deactivate with Get-Command for pwsh', () => {
+            const result = wrapDeactivationCommand(ShellConstants.PWSH, 'deactivate');
+            assert.strictEqual(result, 'if (Get-Command deactivate -ErrorAction SilentlyContinue) { deactivate }');
+        });
+
+        test('passes through non-bare deactivate commands unchanged (cmd full path)', () => {
+            const cmd = 'C:\\envs\\myenv\\Scripts\\deactivate.bat';
+            const result = wrapDeactivationCommand(ShellConstants.CMD, cmd);
+            assert.strictEqual(result, cmd);
+        });
+
+        test('passes through conda deactivate unchanged (legitimate failure if conda missing)', () => {
+            const result = wrapDeactivationCommand(ShellConstants.BASH, 'conda deactivate');
+            assert.strictEqual(result, 'conda deactivate');
+        });
+
+        test('passes through pyenv shell --unset unchanged', () => {
+            const result = wrapDeactivationCommand(ShellConstants.BASH, 'pyenv shell --unset');
+            assert.strictEqual(result, 'pyenv shell --unset');
+        });
+
+        test('passes through nu overlay hide unchanged', () => {
+            const result = wrapDeactivationCommand(ShellConstants.NU, 'overlay hide activate');
+            assert.strictEqual(result, 'overlay hide activate');
+        });
+
+        test('passes through unknown shell unchanged even for bare deactivate', () => {
+            const result = wrapDeactivationCommand('unknown', 'deactivate');
+            assert.strictEqual(result, 'deactivate');
+        });
+
+        test('passes through case-mismatched tokens unchanged (no false-positive wraps)', () => {
+            // Only an exact `deactivate` token triggers wrapping; anything containing other
+            // tokens (e.g. wrapper functions in user shells) is the user's responsibility.
+            const result = wrapDeactivationCommand(ShellConstants.BASH, 'my-deactivate');
+            assert.strictEqual(result, 'my-deactivate');
+        });
+
+        test('wraps DEACTIVATE token case-insensitively (pwsh)', () => {
+            // PowerShell is case-insensitive; the activation script's function may be
+            // reported in any case. The wrap should still apply.
+            const result = wrapDeactivationCommand(ShellConstants.PWSH, 'DEACTIVATE');
+            assert.strictEqual(result, 'if (Get-Command deactivate -ErrorAction SilentlyContinue) { deactivate }');
+        });
+    });
 });