Skip to content

Commit 5af58f2

Browse files
authored
feat(ssh): pre-check the host CLI before starting the SSH tunnel (#2127)
## Changes `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. ## Tests [hostUtils.ts](https://github.com/databricks/databricks-vscode/compare/fix/ssh-tunnel-host-cli-precheck?expand=1#diff-c1b4d8aeff4272f092f04f6ffbccb9c3d97f0f96c549d129b915dc1f9d2aba57)
1 parent 2acdc44 commit 5af58f2

6 files changed

Lines changed: 495 additions & 4 deletions

File tree

packages/databricks-vscode/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1900,6 +1900,7 @@
19001900
"lodash": "^4.17.21",
19011901
"markdown-it": "^14.2.0",
19021902
"minimatch": "^10.0.1",
1903+
"shell-env": "^4.0.3",
19031904
"shell-quote": "^1.8.4",
19041905
"triple-beam": "^1.4.1",
19051906
"winston": "^3.11.0",
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
import assert from "assert";
2+
import {commands, Uri, window} from "vscode";
3+
import {instance, mock} from "ts-mockito";
4+
import {CliWrapper} from "../cli/CliWrapper";
5+
import {SshCommands} from "./SshCommands";
6+
import {HostUtils} from "../utils";
7+
8+
/**
9+
* Exercises the advisory host-CLI PATH warning in isolation. The prompt is
10+
* fired-and-forgotten from startTunnelCommand, so tests reach the private
11+
* warnIfHostCliMissing directly and flush the detached prompt chain it kicks
12+
* off before asserting on the message and any follow-up command.
13+
*/
14+
describe(__filename, () => {
15+
let originalIsHostCliOnPath: typeof HostUtils.isHostCliOnPath;
16+
let originalGetHostCliCommand: typeof HostUtils.getHostCliCommand;
17+
let originalIsCursor: typeof HostUtils.isCursor;
18+
let originalShowWarningMessage: typeof window.showWarningMessage;
19+
let originalExecuteCommand: typeof commands.executeCommand;
20+
let originalPlatform: PropertyDescriptor | undefined;
21+
22+
// Messages shown and their offered actions, plus the executed commands, so
23+
// each test can assert what the user was prompted with and what ran.
24+
let shownWarnings: {message: string; items: string[]}[];
25+
let executedCommands: {command: string; args: unknown[]}[];
26+
// The action the stubbed warning resolves with (what the user "clicks").
27+
let warningResponse: string | undefined;
28+
29+
function stubPlatform(value: NodeJS.Platform) {
30+
Object.defineProperty(process, "platform", {
31+
value,
32+
configurable: true,
33+
});
34+
}
35+
36+
function newSshCommands(): SshCommands {
37+
return new SshCommands(instance(mock(CliWrapper)));
38+
}
39+
40+
// Runs the fire-and-forget warning, then drains the microtask queue so the
41+
// detached prompt chain (warning → follow-up command) has fully settled.
42+
async function warn(sshCommands: SshCommands) {
43+
await (sshCommands as any).warnIfHostCliMissing();
44+
await new Promise((resolve) => setTimeout(resolve, 0));
45+
}
46+
47+
beforeEach(() => {
48+
shownWarnings = [];
49+
executedCommands = [];
50+
warningResponse = undefined;
51+
52+
originalIsHostCliOnPath = HostUtils.isHostCliOnPath;
53+
originalGetHostCliCommand = HostUtils.getHostCliCommand;
54+
originalIsCursor = HostUtils.isCursor;
55+
originalShowWarningMessage = window.showWarningMessage;
56+
originalExecuteCommand = commands.executeCommand;
57+
originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
58+
59+
(window as any).showWarningMessage = (
60+
message: string,
61+
...items: string[]
62+
) => {
63+
shownWarnings.push({message, items});
64+
return Promise.resolve(warningResponse);
65+
};
66+
(commands as any).executeCommand = (
67+
command: string,
68+
...args: unknown[]
69+
) => {
70+
executedCommands.push({command, args});
71+
return Promise.resolve();
72+
};
73+
});
74+
75+
afterEach(() => {
76+
(HostUtils as any).isHostCliOnPath = originalIsHostCliOnPath;
77+
(HostUtils as any).getHostCliCommand = originalGetHostCliCommand;
78+
(HostUtils as any).isCursor = originalIsCursor;
79+
(window as any).showWarningMessage = originalShowWarningMessage;
80+
(commands as any).executeCommand = originalExecuteCommand;
81+
if (originalPlatform) {
82+
Object.defineProperty(process, "platform", originalPlatform);
83+
}
84+
});
85+
86+
it("shows no warning when the host CLI is on PATH", async () => {
87+
(HostUtils as any).isHostCliOnPath = async () => true;
88+
89+
await warn(newSshCommands());
90+
91+
assert.strictEqual(shownWarnings.length, 0);
92+
assert.strictEqual(executedCommands.length, 0);
93+
});
94+
95+
it("offers the shell-command installer on macOS", async () => {
96+
(HostUtils as any).isHostCliOnPath = async () => false;
97+
(HostUtils as any).getHostCliCommand = () => "code";
98+
stubPlatform("darwin");
99+
warningResponse = "Install shell command";
100+
101+
await warn(newSshCommands());
102+
103+
assert.strictEqual(shownWarnings.length, 1);
104+
assert.ok(shownWarnings[0].message.includes('"code"'));
105+
assert.deepStrictEqual(shownWarnings[0].items, [
106+
"Install shell command",
107+
]);
108+
assert.deepStrictEqual(executedCommands, [
109+
{command: "workbench.action.installCommandLine", args: []},
110+
]);
111+
});
112+
113+
it("does not run the installer on macOS when the prompt is dismissed", async () => {
114+
(HostUtils as any).isHostCliOnPath = async () => false;
115+
(HostUtils as any).getHostCliCommand = () => "code";
116+
stubPlatform("darwin");
117+
warningResponse = undefined;
118+
119+
await warn(newSshCommands());
120+
121+
assert.strictEqual(shownWarnings.length, 1);
122+
assert.strictEqual(executedCommands.length, 0);
123+
});
124+
125+
it("points at the VS Code PATH docs off macOS", async () => {
126+
(HostUtils as any).isHostCliOnPath = async () => false;
127+
(HostUtils as any).getHostCliCommand = () => "code";
128+
(HostUtils as any).isCursor = () => false;
129+
stubPlatform("linux");
130+
warningResponse = "Setup instructions";
131+
132+
await warn(newSshCommands());
133+
134+
assert.strictEqual(shownWarnings.length, 1);
135+
assert.deepStrictEqual(shownWarnings[0].items, ["Setup instructions"]);
136+
assert.strictEqual(executedCommands.length, 1);
137+
assert.strictEqual(executedCommands[0].command, "vscode.open");
138+
assert.strictEqual(
139+
(executedCommands[0].args[0] as Uri).toString(),
140+
Uri.parse(
141+
"https://code.visualstudio.com/docs/setup/setup-overview"
142+
).toString()
143+
);
144+
});
145+
146+
it("points at the Cursor PATH docs off macOS in Cursor", async () => {
147+
(HostUtils as any).isHostCliOnPath = async () => false;
148+
(HostUtils as any).getHostCliCommand = () => "cursor";
149+
(HostUtils as any).isCursor = () => true;
150+
stubPlatform("linux");
151+
warningResponse = "Setup instructions";
152+
153+
await warn(newSshCommands());
154+
155+
assert.strictEqual(
156+
(executedCommands[0].args[0] as Uri).toString(),
157+
Uri.parse("https://docs.cursor.com/en/cli/installation").toString()
158+
);
159+
});
160+
161+
it("does not open docs off macOS when the prompt is dismissed", async () => {
162+
(HostUtils as any).isHostCliOnPath = async () => false;
163+
(HostUtils as any).getHostCliCommand = () => "code";
164+
(HostUtils as any).isCursor = () => false;
165+
stubPlatform("linux");
166+
warningResponse = undefined;
167+
168+
await warn(newSshCommands());
169+
170+
assert.strictEqual(shownWarnings.length, 1);
171+
assert.strictEqual(executedCommands.length, 0);
172+
});
173+
});

packages/databricks-vscode/src/ssh/SshCommands.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import {
2+
commands,
23
Disposable,
34
Event,
45
EventEmitter,
56
QuickPick,
67
QuickPickItem,
78
QuickPickItemKind,
9+
Uri,
810
window,
911
} from "vscode";
1012
import {WorkspaceClient} from "@databricks/sdk-experimental";
@@ -24,6 +26,7 @@ import {AuthProvider} from "../configuration/auth/AuthProvider";
2426
import {LoginWizard} from "../configuration/LoginWizard";
2527
import {Cluster} from "../sdk-extensions";
2628
import {onError} from "../utils/onErrorDecorator";
29+
import {HostUtils} from "../utils";
2730
import {logging} from "@databricks/sdk-experimental";
2831
import {Loggers} from "../logger";
2932

@@ -170,6 +173,15 @@ export class SshCommands implements Disposable {
170173

171174
@onError({popup: {prefix: "Error starting SSH tunnel."}})
172175
async startTunnelCommand() {
176+
// `databricks ssh connect` shells out to the host command (`code`/
177+
// `cursor`) to open the remote window; when it is off PATH the connect
178+
// only fails deep inside the terminal after a long wait. Surface an
179+
// actionable in-editor hint up front when the command looks missing.
180+
// This is advisory only — the probe runs a non-interactive shell whose
181+
// PATH can differ from the terminal's, so we still proceed and let the
182+
// terminal report the real error rather than blocking a tunnel that
183+
// would have worked.
184+
await this.warnIfHostCliMissing();
173185
const context = await this.resolveTunnelContext();
174186
if (context === undefined) {
175187
return;
@@ -412,6 +424,64 @@ export class SshCommands implements Disposable {
412424
return false;
413425
}
414426

427+
/**
428+
* When the host editor's shell command (`code`/`cursor`) looks missing from
429+
* PATH — which `databricks ssh connect` needs to open the remote window —
430+
* shows a non-blocking hint. The probe is advisory (its PATH can differ from
431+
* the terminal's), so this never gates the tunnel; the prompt is fired and
432+
* forgotten while the caller proceeds.
433+
*
434+
* On macOS the prompt offers the built-in "Install shell command" installer.
435+
* `workbench.action.installCommandLine` is registered only on macOS, so
436+
* elsewhere we point at the editor's PATH-setup docs instead of telling the
437+
* user to reinstall an editor they already have.
438+
*/
439+
private async warnIfHostCliMissing(): Promise<void> {
440+
if (await HostUtils.isHostCliOnPath()) {
441+
return;
442+
}
443+
const cmd = HostUtils.getHostCliCommand();
444+
const message =
445+
`The "${cmd}" command may not be on your PATH, which the ` +
446+
`Databricks SSH tunnel needs to open the remote window. If the ` +
447+
`tunnel fails to open a window, add it to your PATH and try again.`;
448+
449+
const promptChain =
450+
process.platform === "darwin"
451+
? this.promptInstallShellCommand(message)
452+
: this.promptPathSetupDocs(message);
453+
// The prompt outlives this call by design; make sure a rejection in the
454+
// detached chain can't surface as an unhandled rejection.
455+
promptChain.catch((e) => {
456+
logging.NamedLogger.getOrCreate(Loggers.Extension).error(
457+
"Failed to handle host CLI PATH prompt",
458+
e
459+
);
460+
});
461+
}
462+
463+
private async promptInstallShellCommand(message: string): Promise<void> {
464+
const install = "Install shell command";
465+
const choice = await window.showWarningMessage(message, install);
466+
if (choice !== install) {
467+
return;
468+
}
469+
// Built-in "Shell Command: Install '<cmd>' command in PATH" (macOS only).
470+
await commands.executeCommand("workbench.action.installCommandLine");
471+
}
472+
473+
private async promptPathSetupDocs(message: string): Promise<void> {
474+
const setup = "Setup instructions";
475+
const choice = await window.showWarningMessage(message, setup);
476+
if (choice !== setup) {
477+
return;
478+
}
479+
const url = HostUtils.isCursor()
480+
? "https://docs.cursor.com/en/cli/installation"
481+
: "https://code.visualstudio.com/docs/setup/setup-overview";
482+
await commands.executeCommand("vscode.open", Uri.parse(url));
483+
}
484+
415485
private async launchSshTunnel(
416486
authProvider: AuthProvider,
417487
compute: Compute

packages/databricks-vscode/src/utils/hostUtils.test.ts

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {env} from "vscode";
22
import assert from "assert";
3-
import {isCursor} from "./hostUtils";
3+
import {getHostCliCommand, isCursor, isHostCliOnPath} from "./hostUtils";
4+
import {cancellableExecFile} from "../cli/CliWrapper";
45

56
describe(__filename, () => {
67
let originalAppName: PropertyDescriptor | undefined;
@@ -31,4 +32,90 @@ describe(__filename, () => {
3132
stubUriScheme("vscode");
3233
assert.strictEqual(isCursor(), false);
3334
});
35+
36+
it("resolves the host CLI command to cursor in Cursor", () => {
37+
stubUriScheme("cursor");
38+
assert.strictEqual(getHostCliCommand(), "cursor");
39+
});
40+
41+
it("resolves the host CLI command to code in VS Code", () => {
42+
stubUriScheme("vscode");
43+
assert.strictEqual(getHostCliCommand(), "code");
44+
});
45+
46+
it("resolves the host CLI command to code in Insiders (the CLI has no Insiders descriptor)", () => {
47+
stubUriScheme("vscode-insiders");
48+
assert.strictEqual(getHostCliCommand(), "code");
49+
});
50+
51+
describe("isHostCliOnPath", () => {
52+
// A resolved profile env; the probe passes this to `execFile` as PATH.
53+
// eslint-disable-next-line @typescript-eslint/naming-convention
54+
const loadShellEnv = async () => async () => ({PATH: "/usr/bin"});
55+
56+
it("is true when the probe succeeds", async () => {
57+
const exec = (async () => ({
58+
stdout: "1.0.0",
59+
stderr: "",
60+
})) as unknown as typeof cancellableExecFile;
61+
assert.strictEqual(await isHostCliOnPath(exec, loadShellEnv), true);
62+
});
63+
64+
it("is false only when the command is definitively not found", async () => {
65+
const notFound = Object.assign(new Error("spawn ENOENT"), {
66+
code: "ENOENT",
67+
});
68+
const exec = (async () => {
69+
throw notFound;
70+
}) as unknown as typeof cancellableExecFile;
71+
assert.strictEqual(
72+
await isHostCliOnPath(exec, loadShellEnv),
73+
false
74+
);
75+
});
76+
77+
it("is true (advisory) when the probe fails for another reason", async () => {
78+
const exec = (async () => {
79+
throw Object.assign(new Error("permission denied"), {
80+
code: "EACCES",
81+
});
82+
}) as unknown as typeof cancellableExecFile;
83+
assert.strictEqual(await isHostCliOnPath(exec, loadShellEnv), true);
84+
});
85+
86+
it("probes with the profile PATH on POSIX", async function () {
87+
if (process.platform === "win32") {
88+
this.skip();
89+
}
90+
let seenEnv: Record<string, string> | undefined;
91+
const exec = (async (
92+
_cmd: string,
93+
_args: string[],
94+
opts: {env?: Record<string, string>}
95+
) => {
96+
seenEnv = opts.env;
97+
return {stdout: "1.0.0", stderr: ""};
98+
}) as unknown as typeof cancellableExecFile;
99+
assert.strictEqual(await isHostCliOnPath(exec, loadShellEnv), true);
100+
assert.strictEqual(seenEnv?.PATH, "/usr/bin");
101+
});
102+
103+
it("is true (advisory) when the shell profile fails to resolve", async function () {
104+
// Windows never loads shell-env, so this branch is POSIX-only.
105+
if (process.platform === "win32") {
106+
this.skip();
107+
}
108+
const exec = (async () => ({
109+
stdout: "1.0.0",
110+
stderr: "",
111+
})) as unknown as typeof cancellableExecFile;
112+
const failingShellEnv = async () => async () => {
113+
throw new Error("profile blew up");
114+
};
115+
assert.strictEqual(
116+
await isHostCliOnPath(exec, failingShellEnv),
117+
true
118+
);
119+
});
120+
});
34121
});

0 commit comments

Comments
 (0)