feat(ssh): pre-check the host CLI before starting the SSH tunnel - #2127
Conversation
`databricks ssh connect` shells out to the host editor's shell command (`code`/`cursor`) to open the remote window. When that command is off PATH, the connect only failed deep inside the terminal after a long wait. Fail fast instead: pre-check the command before touching auth, compute, or a terminal, and surface an actionable prompt whose button installs the shell command (falling back to the editor's download page on hosts without the built-in installer). Adds HostUtils.getHostCliCommand/isHostCliOnPath to resolve and probe the command through a shell so PATH resolves as the terminal would.
|
🤖 Integration tests ✅ all 35 test jobs passed for |
|
Reviewed across several passes (Isaac Review, Codex, a Claude review agent, and a code-conventions check). Isaac and the conventions pass came back clean; the findings below were each verified against the code and/or the VS Code source. [must-address] The PATH probe doesn't resolve PATH the way the terminal that runs the connect does —
[should-fix] The probe treats every failure as "not on PATH" — A permission error, a non-zero [should-fix] The "Install shell command" button falls back to a re-download on Windows and Linux —
[nit] VS Code Insiders picks the wrong command name —
[nit] Unhandled rejection in the detached prompt chain — Returning [nit] The new logic is untested — Tests cover only |
|
🤖 Integration tests triggered for |
| * elsewhere we point at the editor's PATH-setup docs instead of telling the | ||
| * user to reinstall an editor they already have. | ||
| */ | ||
| private async warnIfHostCliMissing(): Promise<void> { |
There was a problem hiding this comment.
It would be nice to have tests for this, but not blocking since there aren't tests for the rest of this file
|
Re-reviewed at [should-fix]
So the extension probes for a command the CLI will never invoke. For an Insiders user with only [nit]
Two notes on things I checked and am not asking you to change:
For completeness: I couldn't run typecheck/lint/unit tests in my checkout — |
|
🤖 Integration tests ✅ all 37 test jobs passed for |
| // Insiders ships its shell command as `code-insiders`, not `code`; an | ||
| // Insiders-only install has that on PATH. | ||
| if (env.uriScheme === "vscode-insiders") { | ||
| return "code-insiders"; |
There was a problem hiding this comment.
[must-fix] The probe checks code-insiders, but the CLI always invokes code — so on Insiders the warning fires (or stays silent) for the wrong command.
getHostCliCommand() returns code-insiders for VS Code Insiders, but that's not the command the tunnel actually needs. getSshConnectCommand() (CliWrapper.ts:131) builds the invocation as --ide=${isCursor() ? "cursor" : "vscode"} — there's no Insiders case, so Insiders sends --ide=vscode, and the CLI maps vscode to the literal command code. The bundled CLI (v1.12.1) has no code-insiders descriptor at all.
So the pre-check and the CLI look at two different commands. Concrete failure for a common Insiders-only setup (code-insiders on PATH, code not):
- Probe checks
code-insiders→ present → no warning. - Tunnel starts; CLI invokes
code→ missing → fails deep in the terminal after a wait.
That's exactly the slow failure this PR is meant to pre-empt, and the user got no heads-up — the check is silent in the case it was designed to catch. (Mirror case: an Insiders user with neither command is told to install code-insiders, which won't help, since the CLI needs code.)
Fix: probe whatever --ide resolves to. Since the CLI maps everything non-Cursor to code, drop the code-insiders branch and return code for both stable and Insiders:
export function getHostCliCommand(): "code" | "cursor" {
return isCursor() ? "cursor" : "code";
}Real Insiders support belongs upstream in the CLI (an Insiders descriptor so --ide can mean code-insiders); until then, probing code-insiders just checks for a command the CLI never calls. The getHostCliCommand() test that asserts "code-insiders" should be updated to match.
| @@ -1,4 +1,8 @@ | |||
| import {env} from "vscode"; | |||
| import {logging} from "@databricks/sdk-experimental"; | |||
| import {isFileNotFound} from "@databricks/sdk-experimental/dist/config/execUtils"; | |||
There was a problem hiding this comment.
[nit] Deep /dist/... import — CODE_CONVENTIONS §5.
§5 says never to import through deep /dist/... paths — they aren't a stable entry point; import from the package root. (AzureCliCheck.ts does the same, but per AGENTS.md that's a pre-existing legacy case, not precedent for a new one.)
The root re-exports this as the ExecUtils namespace (verified: ExecUtils.isFileNotFound is exported; isFileNotFound is not available directly off the root):
import {ExecUtils} from "@databricks/sdk-experimental";
// ...
if (ExecUtils.isFileNotFound(e)) { ... }|
🤖 Integration tests triggered for |
|
🤖 Integration tests ❌ 1 of 41 test jobs failed for |
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
rugpanov
left a comment
There was a problem hiding this comment.
Both earlier findings are addressed:
getHostCliCommand()now probescodefor Insiders (matching the CLI's--ide=vscode→code), test updated accordingly.- The
isFileNotFoundimport now comes through theExecUtilspackage-root namespace.
Also reviewed the follow-up rework that resolves the probe against the login/interactive shell env via shell-env on POSIX — verified that execFile resolves the bare command against the passed env.PATH, so it correctly sees the terminal's PATH; new branches are covered by tests. LGTM.
Minor, non-blocking: warnIfHostCliMissing() is awaited on the tunnel-start path, so it spawns a login shell via shell-env on every start — a bounded timeout would guard against a slow/hanging profile.
|
🤖 Integration tests ✅ all 41 test jobs passed for |
## packages/databricks-vscode ## (2026-08-21) * Narrate live progress during Python environment setup (#2139) ([f606d0a](f606d0a)) * Pre-check the host CLI before starting the SSH tunnel (#2127) ([5af58f2](5af58f2)) * Show an info toast on a successful Python environment setup that had warnings (#2138) ([2acdc44](2acdc44)) * Avoid duplicate profile section creation in `.databrickscfg` during OAuth setup (#2133) ([6e1c472](6e1c472)) ## packages/databricks-vscode-types ## (2026-08-21) --------- Co-authored-by: releasebot <noreply@github.com> Co-authored-by: Grigory Panov <grigory.panov@databricks.com> Co-authored-by: Isaac <no-reply@databricks.com>
Changes
databricks ssh connectshells out to the host editor's shell command (code/cursor) to open the remote window. When that command is off PATH, the connect only failed deep inside the terminal after a long wait. Fail fast instead: pre-check the command before touching auth, compute, or a terminal, and surface an actionable prompt whose button installs the shell command (falling back to the editor's download page on hosts without the built-in installer).Adds HostUtils.getHostCliCommand/isHostCliOnPath to resolve and probe the command through a shell so PATH resolves as the terminal would.
Tests
hostUtils.ts