fix(shellUtils): build terminal commands for the shell that runs them (#1822)

[abdelrahmanomar1515]

Fixes #1822.

Why

On Windows with cmd.exe as the terminal shell, "Create a new Databricks project" was
broken. The wizard built one POSIX command line and sent it to whatever shell the
terminal was running, so cmd rejected it token by token and the CLI never started:

C:\...\Temp>clear; echo "Executing: databricks bundle init --output-dir "c:\...\databricks-test"
'clear' is not recognized as an internal or external command,
'Follow' is not recognized as an internal or external command,
'Press' is not recognized as an internal or external command,

clear, ; as a separator, and read are all POSIX-only. cmd needs cls, & and
pause.

The reported wizard turned out not to be the only casualty. The same shared helpers are
used by "Databricks SSH Tunnel" and az login, and both were broken in cmd on
main for the same reason — details in their own sections below.

The root cause is structural, not a missing branch. shellUtils read env.shell inside
every helper and split only "PowerShell" vs "everything else", using a substring match on
the full path — so cmd.exe fell into the POSIX bucket. Because the branching sat behind a
module-level VS Code read, none of it could be reached from a unit test, so this class of
bug had no way to be caught. Fixing only the clear/read symptoms would have left the
next shell-specific bug just as invisible, which is why this is a refactor rather than a
two-line patch.

What changed

One classifier instead of scattered predicates.
detectShellKind(shell, platform) -> "cmd" | "powershell" | "posix" is a pure function;
every helper takes that kind. env.shell is now read in exactly one place,
currentShellKind(), matching the existing venvInterpreterPath injection pattern in
this repo. Detection matches on the shell's basename, so
C:\cmder\...\bash.exe is no longer mistaken for cmd.

Terminals still resolve their own shell. I deliberately did not pin shellPath.
Passing an explicit executable makes VS Code build a synthetic profile from
{path, args} and never supply the resolved profile's defaults
(terminalProfileResolverService.ts:113-124) —
and on macOS the fallback profile is what appends --login. So pinning
shellPath: env.shell would silently turn a login shell into a non-login one, stop
~/.zprofile being sourced, and change PATH. env.shell exposes only profile.path,
never args, so those args can't be recovered.

Per file:

File	Change
utils/shellUtils.ts	ShellKind + detectShellKind; all helpers parameterised; per-shell quoting; clearCmd/commandSeparator/echoLine/hasCmdUnsafeChars added
utils/shellUtils.test.ts	New — 60 cases across three layers
bundle/BundleInitWizard.ts	Assembles the line via the helpers; refuses a cmd-unsafe output dir
cli/CliWrapper.ts	escapedCliPathFor(kind); escapedCliPath delegates to it — which also fixes the SSH tunnel, see below
configuration/auth/AzureCliCheck.ts	Same defect in the az login line; also quotes the az path and the tenant
run/RunCommands.ts	Explicit unknown-shell quoting (behaviour unchanged — see below)

Bugs found once the logic became testable

Each is a real defect on main, not hypothetical:

• pwsh[.exe] was unhandled — PowerShell 6+/7, the default on modern Windows. It
  matched neither "powershell" nor cmd, so those users got POSIX read, which does not
  exist in PowerShell.
• PowerShell paths were double-quoted, which interpolates.
  C:\Users\me\$RECYCLE.BIN expanded to C:\Users\me\.BIN, and a directory named
  C:\a$(whoami)b would execute whoami. Both PowerShell and POSIX now use single
  quotes, which are literal.
• escapedCliPath quoted the CLI wrongly for both Windows shells, which broke the
  SSH tunnel as well as the init wizard — see the section below.
• escapedCliPath used .replace, not .replaceAll — every quote after the first
  was left unescaped.
• az login interpolated both the az path and the tenant bare. A custom azBinPath
  containing spaces (C:\Program Files\Azure CLI\az.cmd) split into several arguments,
  and the tenant — which comes from a token's iss claim, i.e. external data reaching a
  command line — was unquoted. Both are quoted now. (Quoting a bare command name still
  resolves via PATH in all three shells, so the default az is unaffected.)
• POSIX used echo, whose builtin form interprets backslash escapes in zsh and dash
  even inside single quotes: \t printed a tab, and \c truncated the rest of the
  line. Now printf '%s\n', which is verbatim everywhere. (The message is a printf
  argument, not the format string, so a literal % is safe.)
• The read pause needs a named variable; every shorter spelling breaks a shell.
  Bare read is a usage error in dash (read: arg count, exit 2), and in fish it
  exits 0 but echoes the keypress back into the terminal. read _ fixes dash but is an
  error in fish, where _ is read-only. read -r name is an error in fish too — it has no
  -r flag. Now read discard, the only form sh, bash, zsh, dash and fish all
  accept. A failed pause isn't cosmetic: the following exit closes the terminal and
  discards exactly the CLI error the pause exists to keep on screen. (Matrix in the testing
  section below.)
• cmd's echo printed shell operators as operators. The message was interpolated
  unquoted, so a banner containing &, | or > was parsed rather than printed — and the
  banner embeds the user's output directory, so a folder named a&b made cmd try to run
  b as a command ('pipe' is not recognized...). Now caret-escaped. Quoting isn't an
  option: cmd's echo prints the rest of the line raw, so the quotes would show. The
  escaping skips text already inside double quotes, where cmd treats operators as literal
  and a caret would itself print — that's the shape the real banner has.
• The cmd separator had a leading space, so echo one & ... printed one with a
  trailing space, for the same raw-text reason. Now & rather than &.
• %VAR% cannot be escaped in cmd — it expands even inside double quotes. Output dir
  C:\p%TEMP%q\proj would have scaffolded into a directory the user never chose, after
  which getSubProjects reports "no Databricks projects detected". BundleInitWizard now
  refuses up front with an actionable message. (!VAR! is included too, since it expands
  the same way under delayed expansion.)

The SSH tunnel had the same bug, and is fixed by the same change

Not just the init wizard: "Databricks SSH Tunnel" was broken in cmd on main too, and
in pwsh. SshCommands.launchSshTunnel sends ${this.cli.escapedCliPath} …
(src/ssh/SshCommands.ts:435), and main's getter had no cmd branch at all:

get escapedCliPath(): string {
    return isPowershell()
        ? `& "${this.cliPath.replace('"', '\\"')}"`
        : `'${this.cliPath.replaceAll("'", "\\'")}'`;   // ← cmd landed here
}

The consequence differs per shell, and neither starts the tunnel:

• cmd got the single-quoted POSIX form. cmd has no single-quote semantics, so the
  quotes are part of the name and it looks for a program literally called
  'C:\…\databricks.exe' → is not recognized as an internal or external command.
• pwsh failed isPowershell()'s "powershell" substring match, so it also got
  single quotes — and without the & call operator a quoted string on its own line is
  just a string expression, so PowerShell prints the path rather than running it.

No change to SshCommands.ts was needed: escapedCliPath now delegates to
escapedCliPathFor(currentShellKind()), which routes cmd to double quotes and PowerShell
to & '…'. The call site was already correct — the terminal is created without
shellPath (SshCommands.ts:428-433), which is exactly the precondition the getter
documents, so the default profile really is the shell that parses the line. That's what
distinguishes it from RunCommands below.

The tunnel's arguments need no quoting: getSshConnectCommand
(CliWrapper.ts:128-142) emits only internally-generated tokens — ssh connect,
--ide=vscode|cursor, --auto-approve, --auto-start-cluster, a cluster ID, and an
accelerator from a hardcoded list — with no spaces or metacharacters. And it's a single
command, so none of the clear/read/separator/echo syntax that varies per shell is
involved. Worth noting for a follow-up: those args are interpolated unquoted, which is
fine for today's fixed set but would silently become a bug if a flag ever carried a
user-supplied value. There's also no src/ssh/ test file, so this path is covered only
indirectly through escapeExecutableForTerminal's round-trips.

RunCommands is deliberately unchanged in behaviour

runFileUsingDbconnect sends to window.activeTerminal ?? createTerminal(). For a
reused terminal the shell is not env.shell, and TerminalState.shell doesn't exist in
the @types/vscode@1.86.0 this extension targets, so no escaping choice is safe.
main's "…" quoting happens to parse in both cmd and POSIX, so it survives a
mismatch by accident; switching its POSIX branch to single quotes would have newly
broken a focused cmd tab (cmd would look for a program literally named
'C:\Python\python.exe'). It now calls explicit escapePathArgumentForUnknownShell /
escapeExecutableForUnknownShell helpers that keep the double quotes and say why. The
real fix there is a ProcessExecution/task with an argv array, which needs no quoting —
left as a follow-up.

Known gap: fish and backslashes

escapePathArgument's POSIX branch is not correct for fish, which — unlike sh, bash,
zsh and dash — treats \ as an escape inside single quotes. Two shapes go wrong:

output dir	sh/bash/zsh/dash	fish
/Users/me/two\\slashes	verbatim	arrives as two\slashes
/Users/me/trailing\	verbatim	Unexpected end of string, quotes are not balanced

This is reachable, not theoretical: a directory literally named two\\slashes is legal on
macOS and Linux, and I confirmed it can be created. The first shape scaffolds into a
directory the user didn't choose (the same failure mode as %VAR% in cmd); the second
fails to parse.

I've left this out of this PR deliberately — it needs a fourth ShellKind (fish is
currently classified posix, which is right for clear/;/printf and wrong only for
backslash quoting), and that's a wider change than the bug this PR is about. Everything
else the wizard emits was verified byte-identical in fish. Happy to file it as a separate
issue.

How this was tested

Manually on Windows, across several terminal.integrated.defaultProfile.windows
settings — this is the bug report's scenario and it now works.

yarn test:unit — 669 passing, 0 failing with PowerShell 7.6.2 and fish 4.8.1 present
(599 before this branch, so 70 are new). Verified green in all three shapes this affects:
with pwsh and $TERM set, with pwsh and $TERM unset (the CI shape, via
env -u TERM), and with no pwsh at all. yarn test:lint clean.

fish is included in the POSIX round-trips, probed for on PATH so the tests skip
rather than fail where it isn't installed — neither CI image has it, nor does macOS out of
the box. It earns its place by not being strictly POSIX, which is how the read _ bug
above was found. The full read matrix, measured by running each candidate in each shell:

candidate	sh	bash	zsh	dash	fish
read	ok	ok	ok	X arg count	X echoes the input back
read _	ok	ok	ok	ok	X read-only variable
read -r name	ok	ok	ok	ok	X no -r flag
read discard	ok	ok	ok	ok	ok

Three layers, chosen so the weaknesses of each are covered by another:

1. Table-driven classification. detectShellKind across 13 shell/platform pairs plus
   regression cases: the cmder false positive, cmd on Linux, pwsh on macOS, and the
   empty-shell fallback per platform. Both inputs are parameters, so these are
   deterministic on any host OS — no process.platform patching and no mocking anywhere
   in the suite.
2. Exact command strings. The full bundle-init line is asserted verbatim for cmd,
   PowerShell and POSIX, so a reviewer can read the expected output for each shell
   directly in the test.
3. Round-trip execution against real shells. String equality only proves the code
   built what the author intended — not that the shell agrees. Reverting the PowerShell
   single-quoting fix makes the round-trip fail with C:\a$VAR\dollar arriving as
   C:\a\dollar, i.e. real PowerShell silently dropping the variable. The generated commands are
   executed through /bin/sh, zsh, bash, dash and fish, and through cmd and
   PowerShell, asserting output matches input byte for byte across awkward messages and
   paths: embedded quotes of both kinds, $VAR, `cmd`, $(cmd), &/|/>,
   backslashes. This layer is what caught the echo and read bugs; the string assertions
   alone had passed.

Verifying the tests actually bite. Since a test that passes either way proves nothing,
I reverted each fix individually and recorded the failures:

Fix reverted	Tests failing
PowerShell single-quoting	6
printf instead of echo	6 — in sh/zsh/dash only; bash passed, matching the real defect
read discard → read _	6 (incl. both fish round-trips)
read discard → bare read	7 — dash and fish; the fish echo-back is caught only by the new silent-consume assertion
%/! guard	1
empty-shell fallback	1

Every fix has at least one test that fails without it. Doing this also exposed two
weaknesses in my own tests, both times an assertion too weak to see a real failure:

• the read round-trip originally used || true, which masked dash's exit-2 usage error.
  It now asserts empty stderr and status !== 2.
• that stderr-and-status check still wasn't enough: bare read in fish exits 0 with
  empty stderr while echoing the keypress back into the terminal, so it slipped through.
  A second round-trip now asserts the pause consumes its line silently (stdout === ""),
  which is the only assertion that catches it.

Reviewer notes

• The round-trip layer earned its keep, and the CI matrix is what proved it. Neither
  cmd nor PowerShell can be exercised on a macOS dev machine, so both CI legs found real
  problems that a green local run had missed:
  • The Linux leg has pwsh installed (PowerShell is probed for rather than gated on
    platform), so it ran those four tests and caught two Mocha timeouts from per-case
    pwsh spawns plus two assertions that depended on $TERM.
  • The Windows leg then caught the two genuine echo/separator defects listed above.
    Both are fixed and both legs are now green: Windows 660 passing / 0 failing with all
    ten cmd and PowerShell round-trips executing (non-trivial durations, 0 pending), Linux 663
    passing / 0 failing. So every assertion for the two shells [BUG] 'clear' is not recognized as an internal or external command #1822 is about has now run
    against a real shell, not just against a string comparison.
• The cmd round-trips run the generated line from a temp .cmd file rather than
  cmd /c <line>. Passing it as an argv entry lets Node apply its own Windows quoting,
  which escapes an embedded " as \" — a sequence cmd doesn't understand — so the test
  would measure Node's escaping instead of ours.
• Five commits, reviewable in order: (1) the bundle-init fix plus the shellUtils
  refactor — which is also where the SSH tunnel fix lands, via escapedCliPath, (2)
  az login, which had the identical defect, (3) making the PowerShell round-trips pass on
  a real pwsh, (4) the two cmd echo/separator fixes the Windows leg found, (5) the
  read discard fix for fish.
• Three user-facing commands were broken in cmd on main, not one: the init wizard
  (reported), the SSH tunnel, and az login. All three are fixed here. The manual Windows
  testing covered the wizard — the bug report's scenario — so the tunnel and az login
  paths are verified by the round-trip tests rather than by hand, and are worth a manual
  look if you have a Windows box with cmd as the default profile.
• echoLine takes a single line and callers emit blank lines as separate commands.
  The old code embedded \n in a double-quoted echo, which isn't portable.
• Once engines.vscode moves past 1.86, terminal.state.shell becomes available and
  would make a good dev-only assertion: compare it against detectShellKind(env.shell)
  and log a warning on disagreement. That would also cover sub-shells and WSL distro
  wrappers, which basename matching can't see.
• Follow-ups I'd file rather than grow this PR: fish's backslash quoting (section
  above — needs a fourth ShellKind), RunCommands moving to a ProcessExecution with an
  argv array, and the SSH tunnel's unquoted args.

This pull request and its description were written by Isaac.

[rugpanov]

Reviewed via multi-source pass (Codex + Claude deep review + independent verification). Solid, well-reasoned refactor — commands are now built in the correct shell dialect. No blocking issues; the cmd caret-escaping, POSIX/PowerShell quoting, and call-site shell kinds all check out. One narrow nit: on a pwsh-default host, the reused-terminal (RunCommands) path can emit a stray & prefix into a non-pwsh terminal — not a hard bug, and the documented argv-array follow-up is the real fix.

[github-actions]

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/vscode

Inputs:

• PR number: 2092
• Commit SHA: 7af3a578754b39cd067e1edda9ab6de4c9e4236f

Checks will be approved automatically on success.