Say why when a remote review's host is down, in Source windows, Ask and Source tree

apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts

@@ -75,6 +75,7 @@ import type {
 	ReviewRuntimeConfig,
 	ReviewSurfaceEvent,
 	ReviewTheme,
+	ReviewVerbResponse,
 	TutorialProgressV1,
 	TutorialStepId,
 } from "../../../common/reviewProtocol.js";
@@ -87,6 +88,7 @@ import {
 } from "../../../common/reviewProtocol.js";
 import { IReviewVerbsService } from "../../../contrib/verbs/reviewVerbs.js";
 import { vetoStoppingAskAgents } from "../../reviewAskShutdown.js";
+import { notifyOpenFailure } from "./reviewHostFailure.js";
 import { confirmReviewDeletion } from "../../reviewDeleteConfirmation.js";
 import { showReviewNotification } from "../../reviewNotification.js";
 import { showReviewCanvasMenu } from "../../reviewCanvasMenu.js";
@@ -489,20 +491,18 @@ export class ReviewCanvasEditorPane extends EditorPane {
 							request: requestReviewApi,
 							post: async (request) => {
 								if (request.name === "openSourceTree") {
-									await this.tabsService.openApiSource(sourceSelection, input.getName());
-									return { ok: true };
+									return this.openSource(() => this.tabsService.openApiSource(sourceSelection, input.getName()));
 								}
 								if (request.name === "reveal") {
-									const range = { startLine: request.args.startLine, endLine: request.args.endLine };
-									await this.apiSource.open(
-										{ view: reviewSourceAnchor(sourceView, request.args.pins), file: request.args.path, side: request.args.side ?? "head" },
-										range,
-									);
-									return { ok: true };
+									const { args } = request;
+									return this.openSource(() => this.apiSource.open(
+										{ view: reviewSourceAnchor(sourceView, args.pins), file: args.path, side: args.side ?? "head" },
+										{ startLine: args.startLine, endLine: args.endLine },
+									));
 								}
 								if (request.name === "openDiff") {
-									await this.apiSource.openDiff(sourceView, request.args.path);
-									return { ok: true };
+									const { path } = request.args;
+									return this.openSource(() => this.apiSource.openDiff(sourceView, path));
 								}
 								return this.verbs.dispatch(request);
 							},
@@ -580,7 +580,7 @@ export class ReviewCanvasEditorPane extends EditorPane {
 						openSourceTree: (uuid) => {
 							const api = this.apiCatalog.reviews.find((review) => review.reviewId === uuid);
 							if (api) {
-								void this.tabsService.openApiSource({ reviewId: api.reviewId, kind: "current" }, api.title).catch(error => this.notificationService.error(error));
+								void this.tabsService.openApiSource({ reviewId: api.reviewId, kind: "current" }, api.title).catch(error => this.notifyOpenFailure(error));
 								return;
 							}
 						},
@@ -644,6 +644,21 @@ export class ReviewCanvasEditorPane extends EditorPane {
 		}
 	}
 
+	private notifyOpenFailure(error: unknown): void {
+		notifyOpenFailure(this.notificationService, (alias) => this.desktopConnection.retryRemoteHost(alias), error);
+	}
+
+	/** Answers the canvas instead of rejecting, once the failure is shown. */
+	private async openSource(open: () => Promise<void>): Promise<ReviewVerbResponse> {
+		try {
+			await open();
+			return { ok: true };
+		} catch (error) {
+			this.notifyOpenFailure(error);
+			return { ok: false, error: error instanceof Error ? error.message : String(error) };
+		}
+	}
+
 	private remoteHostActions(): ReviewRemoteHostActions {
 		return {
 			states: () => this.desktopConnection.readRemoteHosts(),

apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewHostFailure.ts

@@ -0,0 +1,28 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) dev.fast. All rights reserved.
+ *  Licensed under the MIT License. See LICENSE in the repository root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import Severity from "../../../../base/common/severity.js";
+import type { INotificationService } from "../../../../platform/notification/common/notification.js";
+import { reviewHostStatus, type ReviewGatewayHostState } from "../../../common/reviewProtocol.js";
+
+type HostDown = Pick<ReviewGatewayHostState, "alias" | "state" | "detail">;
+
+export class ReviewHostDown extends Error {
+	constructor(readonly host: HostDown) {
+		super(reviewHostStatus(host).sentence);
+	}
+}
+
+export function notifyOpenFailure(notifications: INotificationService, retry: (alias: string) => Promise<void>, error: unknown): void {
+	if (!(error instanceof ReviewHostDown)) {
+		notifications.error(error instanceof Error ? error : String(error));
+		return;
+	}
+	const { alias } = error.host;
+	const choices = reviewHostStatus(error.host).action === "retry"
+		? [{ label: "Retry", run: () => void retry(alias).catch(retryError => notifications.error(retryError)) }]
+		: [];
+	notifications.prompt(Severity.Warning, error.message, choices);
+}

apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.test.ts

@@ -15,6 +15,8 @@ test("a review a remote opened before its list entry arrived takes the host once
 	const updated = withRemoteEntry(mounted, entry("wb-a"));
 	assert.deepEqual(updated, { kind: "api", reviewId: "r1", host: "wb-a", available });
 	assert.equal(withRemoteEntry(updated!, entry("wb-a")), undefined);
+
+	assert.deepEqual(withRemoteEntry(updated!, { ...entry("wb-a"), hostState: "offline" }), { ...updated, hostState: "offline" });
 });
 
 test("a laptop review's entry changes nothing", () => {

apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.ts

@@ -1,12 +1,12 @@
 import type { ReviewApiSummary } from "../../../common/reviewProtocol.js";
 
-type RemoteFields = Pick<ReviewApiSummary, "host" | "available">;
+type RemoteFields = Pick<ReviewApiSummary, "host" | "hostState" | "available">;
 
 export const remoteEntry = (review: ReviewApiSummary | undefined): RemoteFields =>
-	review?.host ? { host: review.host, available: review.available } : {};
+	review?.host ? { host: review.host, ...(review.hostState && { hostState: review.hostState }), available: review.available } : {};
 
 export function withRemoteEntry<T extends RemoteFields>(content: T, review: ReviewApiSummary | undefined): T | undefined {
 	const entry = remoteEntry(review);
-	if (!entry.host || (entry.host === content.host && JSON.stringify(entry.available) === JSON.stringify(content.available))) return undefined;
+	if (!entry.host || (entry.host === content.host && entry.hostState === content.hostState && JSON.stringify(entry.available) === JSON.stringify(content.available))) return undefined;
 	return { ...content, ...entry };
 }

apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.test.ts

@@ -381,7 +381,7 @@ test("a cancelled prompt is auth-failed, and there is no second attempt", async
 	await until(() => last()?.problem !== undefined);
 
 	assert.equal(last()?.problem?.state, "auth-failed");
-	assert.equal(last()!.problem!.detail, "dev@127.0.0.1: Permission denied (publickey,password).");
+	assert.equal(last()!.problem!.detail, `wb-test-a did not accept your SSH key. Check that "ssh wb-test-a" works in a terminal.`);
 	assert.equal(clock.pending, 0);
 	assert.equal(ssh.of("wb-test-a", "master").length, 1);
 });
@@ -404,7 +404,7 @@ test("the master exits and the host reconnects after the backoff", async (t) =>
 	await until(() => last()?.problem !== undefined);
 
 	assert.equal(last()?.problem?.state, "unreachable");
-	assert.match(last()!.problem!.detail, /closed by remote host/);
+	assert.equal(last()!.problem!.detail, "wb-test-a closed the SSH connection.");
 	assert.equal(ssh.of("wb-test-a", "master").length, 1);
 	assert.ok(clock.next());
 	await until(() => last()?.endpoint !== undefined);
@@ -488,7 +488,7 @@ test("an authenticated master that ends is unreachable and retried, whatever its
 	await until(() => last()?.problem !== undefined);
 
 	assert.equal(last()?.problem?.state, "unreachable");
-	assert.equal(last()!.problem!.detail, "The SSH connection to wb-test-a ended: Connection reset by peer");
+	assert.equal(last()!.problem!.detail, "wb-test-a closed the SSH connection.");
 	assert.equal(clock.pending, 1);
 });
 

apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.ts

@@ -83,6 +83,28 @@ export function classifySshFailure(stderr: string, promptCancelled: boolean): "a
 		: "unreachable";
 }
 
+const SSH_SENTENCES: [RegExp, (alias: string) => string][] = [
+	[/REMOTE HOST IDENTIFICATION HAS CHANGED/, (alias) => `${alias}'s host key changed since you last connected. Verify the new fingerprint before replacing its old key in known_hosts.`],
+	[/Host key verification failed/, (alias) => `${alias}'s host key could not be verified. Run "ssh ${alias}" in a terminal and verify its fingerprint before accepting it.`],
+	[/Permission denied/, (alias) => `${alias} did not accept your SSH key. Check that "ssh ${alias}" works in a terminal.`],
+	[/Could not resolve hostname/, (alias) => `${alias}'s host name could not be found. Check its address in your SSH config.`],
+	[/Connection refused/, (alias) => `${alias} refused the SSH connection. Check that its SSH server is running.`],
+	[/(Operation|Connection) timed out/, (alias) => `The SSH connection to ${alias} timed out. Check that it is on and reachable from this network.`],
+];
+
+function sshFailureSentence(stderr: string, alias: string): string | undefined {
+	return SSH_SENTENCES.find(([pattern]) => pattern.test(stderr))?.[1](alias);
+}
+
+const SSH_DROPS: [RegExp, (alias: string) => string][] = [
+	[/Timeout, server .* not responding/, (alias) => `${alias} stopped answering over SSH. It may be asleep, or off this network.`],
+	[/Connection reset|Broken pipe|closed by remote host/, (alias) => `${alias} closed the SSH connection.`],
+];
+
+function sshDropSentence(lastLines: string, alias: string): string | undefined {
+	return SSH_DROPS.find(([pattern]) => pattern.test(lastLines))?.[1](alias);
+}
+
 export const OPENSSH_NEEDED = "OpenSSH is needed: no `ssh` command was found on PATH.";
 
 type Problem = NonNullable<ReviewGatewayHost["problem"]>;
@@ -557,14 +579,17 @@ export class ReviewRemoteHost {
 		if (error?.code === "ENOENT") return this.fail({ state: "unreachable", detail: OPENSSH_NEEDED });
 		const exited = error?.message || `ssh exited with code ${code ?? "none"}.`;
 		if (this.connectedAt !== undefined) {
+			const last = lastLines(this.masterStderr);
 			return this.fail({
 				state: "unreachable",
-				detail: `The SSH connection to ${this.alias} ended: ${lastLines(this.masterStderr) || exited}`,
+				detail: sshDropSentence(last, this.alias) ?? `The SSH connection to ${this.alias} ended: ${last || exited}`,
 			});
 		}
+		const sentence = sshFailureSentence(this.masterStderr, this.alias);
+		if (sentence) this.options.log(`${this.alias}: ${exited} ${firstLines(this.masterStderr)}`);
 		this.fail({
 			state: classifySshFailure(this.masterStderr, this.promptCancelled),
-			detail: firstLines(this.masterStderr) || exited,
+			detail: sentence ?? (firstLines(this.masterStderr) || exited),
 		});
 	}
 

apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.test.ts

@@ -133,6 +133,16 @@ test("a machine whose entry lost its id while unreachable is known through its a
 	assert.equal(manager.hostState(FAKE_SERVER_ID, unreachable), undefined);
 });
 
+test("after a restart, a window's last alias gives its machine's state until that alias attaches", async (t) => {
+	const { manager } = await managerFor(t, { "wb-test-a": { master: "hang" } });
+	manager.update(true, ["wb-test-a"]);
+
+	const unreachable = [{ alias: "wb-test-a", state: "unreachable" as const }];
+	assert.deepEqual(manager.hostState(FAKE_SERVER_ID, unreachable, "wb-test-a"), { alias: "wb-test-a", state: "unreachable" });
+	assert.equal(manager.hostState(FAKE_SERVER_ID, unreachable), undefined);
+	assert.equal(manager.hostState(FAKE_SERVER_ID, unreachable, "gone"), undefined);
+});
+
 test("an alias added again while its old host closes starts only after the old master exited", async (t) => {
 	const { manager, ssh, sentUntil } = await managerFor(t, { "wb-test-a": { exitDelayMs: 150 } });
 

apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.ts

@@ -226,9 +226,10 @@ export class ReviewRemoteHosts {
 		return online && this.hosts.get(online.alias)?.languageEndpoint(serverId);
 	}
 
-	/** The state of the machine `serverId`, through the alias in the setting that served it this session. */
-	hostState(serverId: string, states: readonly ReviewGatewayHostState[]): ReturnType<typeof remoteHostState> {
-		const configured = [...this.hosts.values()].find((host) => host.serverId === serverId)?.state.alias;
+	/** The state of machine `serverId`, through the alias that served it, or `last` until that alias attaches. */
+	hostState(serverId: string, states: readonly ReviewGatewayHostState[], last?: string): ReturnType<typeof remoteHostState> {
+		const configured = [...this.hosts.values()].find((host) => host.serverId === serverId)?.state.alias
+			?? (last !== undefined && this.hosts.has(last) && this.hosts.get(last)?.serverId === undefined ? last : undefined);
 		return remoteHostState(serverId, states, configured);
 	}
 

apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopChannel.ts

@@ -58,8 +58,9 @@ export class ReviewDesktopChannel implements IServerChannel {
       return (await this.host.getRemoteLanguageEndpoint(arg)) as T;
     }
     if (command === "getRemoteHostState") {
-      if (typeof arg !== "string") return undefined as T;
-      return (await this.host.getRemoteHostState(arg)) as T;
+      const [serverId, last] = Array.isArray(arg) ? arg : [arg];
+      if (typeof serverId !== "string") return undefined as T;
+      return (await this.host.getRemoteHostState(serverId, typeof last === "string" ? last : undefined)) as T;
     }
     if (command === "retryRemoteHost") {
       if (typeof arg === "string") this.host.retryRemoteHost(arg);

apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopHost.ts

@@ -263,10 +263,10 @@ export class ReviewDesktopHost extends Disposable {
     return states && manager.languageEndpoint(serverId, states).catch(() => undefined);
   }
 
-  async getRemoteHostState(serverId: string) {
+  async getRemoteHostState(serverId: string, last?: string) {
     const manager = this.remoteHosts;
     const states = manager && (await this.remoteHostStates());
-    return states && manager.hostState(serverId, states);
+    return states && manager.hostState(serverId, states, last);
   }
 
   private async remoteHostStates(): Promise<ReviewGatewayHostState[] | undefined> {