diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts index de5aba0ec..8d45c4452 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewCanvasPart.ts +++ b/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): Promise { + 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(), diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewHostFailure.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewHostFailure.ts new file mode 100644 index 000000000..19530ddea --- /dev/null +++ b/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; + +export class ReviewHostDown extends Error { + constructor(readonly host: HostDown) { + super(reviewHostStatus(host).sentence); + } +} + +export function notifyOpenFailure(notifications: INotificationService, retry: (alias: string) => Promise, 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); +} diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.test.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.test.ts index c1b336aa8..fb7c920c6 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.test.ts +++ b/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", () => { diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.ts index af1d21611..9ca0aee1a 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewRemoteEntry.ts +++ b/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; +type RemoteFields = Pick; 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(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 }; } diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.test.ts index 88c5e98c7..0937b3ca8 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.test.ts +++ b/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); }); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.ts index 66cca15b0..d524cc31c 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHost.ts +++ b/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; @@ -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), }); } diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.test.ts index ce6b6cfb1..10595f9ef 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.test.ts +++ b/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 } }); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.ts index db1709ad9..97f788f4a 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.ts +++ b/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 { - 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 { + 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); } diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopChannel.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopChannel.ts index e2621582d..69ea22d73 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopChannel.ts +++ b/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); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopHost.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopHost.ts index a49f7b91b..47fe9b11d 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/reviewDesktopHost.ts +++ b/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 { diff --git a/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.test.ts index d3888d248..898f94f00 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.test.ts @@ -5,23 +5,24 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { Emitter } from "../../../base/common/event.js"; +import { Emitter, Event } from "../../../base/common/event.js"; +import { CommandsRegistry } from "../../../platform/commands/common/commands.js"; import type { IMainProcessService } from "../../../platform/ipc/common/mainProcessService.js"; -import type { INotification, INotificationService } from "../../../platform/notification/common/notification.js"; +import type { INotification, INotificationActions, INotificationService } from "../../../platform/notification/common/notification.js"; import { PersistentConnectionEventType, type PersistentConnectionEvent } from "../../../platform/remote/common/remoteAgentConnection.js"; import type { IStorageService } from "../../../platform/storage/common/storage.js"; import type { IWorkbenchEnvironmentService } from "../../../workbench/services/environment/common/environmentService.js"; import type { IRemoteAgentConnection, IRemoteAgentService } from "../../../workbench/services/remote/common/remoteAgentService.js"; import type { IStatusbarEntry, IStatusbarService } from "../../../workbench/services/statusbar/browser/statusbar.js"; -import type { ReviewGatewayHostState } from "../../common/reviewProtocol.js"; -import { ReviewSourceWindowHostState, showSourceWindowHostState } from "./reviewSourceWindowHostState.js"; +import { RETRY_HOST_COMMAND, ReviewSourceWindowHostState, showSourceWindowHostState } from "./reviewSourceWindowHostState.js"; import type { ReviewWindowHosts } from "./reviewWindowAuthorityResolver.js"; -const OFFLINE = "devbox — offline, reconnecting…"; +const OFFLINE = "devbox offline"; function fakes() { const entries: IStatusbarEntry[] = []; const shown: INotification[] = []; + const retried: string[] = []; let closed = 0; const statusbar = { addEntry: (entry: IStatusbarEntry) => { @@ -31,24 +32,31 @@ function fakes() { } as unknown as IStatusbarService; const notifications = { notify: (notification: INotification) => { - shown.push(notification); - return { close: () => void closed++ }; + const shownAt = shown.push({ ...notification }) - 1; + return { + onDidClose: Event.None, + close: () => void closed++, + updateMessage: (message: string) => void (shown[shownAt] = { ...shown[shownAt], message }), + updateActions: (actions: INotificationActions) => void (shown[shownAt] = { ...shown[shownAt], actions }), + }; }, } as unknown as INotificationService; - let state: ReviewGatewayHostState["state"] = "online"; + let state: { state: string; detail?: string } = { state: "online" }; let polls = 0; - const hosts: ReviewWindowHosts = { endpoint: async () => undefined, state: async () => (polls++, { alias: "devbox", state }) }; + const hosts = { endpoint: async () => undefined, state: async () => (polls++, { alias: "devbox", ...state }) } as unknown as ReviewWindowHosts; const connection = new Emitter(); return { statusbar, notifications, hosts, connection: { onDidStateChange: connection.event } as unknown as IRemoteAgentConnection, + retry: (alias: string) => void retried.push(alias), fire: (type: PersistentConnectionEventType) => connection.fire({ type } as PersistentConnectionEvent), - set: (next: ReviewGatewayHostState["state"]) => void (state = next), + set: (next: string, detail?: string) => void (state = { state: next, detail }), polls: () => polls, last: () => entries.at(-1), shown, + retried, closed: () => closed, }; } @@ -56,19 +64,22 @@ function fakes() { async function until(check: () => boolean) { while (!check()) await new Promise((resolve) => setTimeout(resolve, 1)); } +const labels = (notification: INotification | undefined) => notification?.actions?.primary?.map((action) => action.label); test("shows the alias while online, the offline state within one poll, and restores it", async () => { const f = fakes(); - const watch = showSourceWindowHostState("abc-1", f.hosts, f.statusbar, f.notifications, f.connection, undefined, 5); + const watch = showSourceWindowHostState("abc-1", f.hosts, f.statusbar, f.notifications, f.connection, f.retry, undefined, 5); try { await until(() => f.last()?.text === "devbox"); assert.notEqual(f.last()?.kind, "warning"); + assert.equal(f.last()?.command, undefined); assert.equal(f.shown.length, 0); f.set("offline"); await until(() => f.last()?.text === OFFLINE); assert.equal(f.last()?.kind, "warning"); - assert.deepEqual(f.shown.map((n) => [n.message, n.actions]), [["devbox is offline. The window reconnects when it is back.", undefined]]); + assert.equal(f.last()?.command, RETRY_HOST_COMMAND); + assert.deepEqual(f.shown.map((n) => [n.message, labels(n)]), [["devbox offline.", ["Retry"]]]); f.fire(PersistentConnectionEventType.ConnectionLost); f.set("online"); @@ -82,9 +93,39 @@ test("shows the alias while online, the offline state within one poll, and resto assert.notEqual(f.last()?.kind, "warning"); assert.equal(f.closed(), 1); - f.set("unreachable"); + f.set("unreachable", "ssh: connect to host devbox port 22: Connection refused"); + await until(() => f.shown.length === 2); + assert.equal(f.last()?.text, OFFLINE); + assert.deepEqual(f.shown.map((n) => n.message), ["devbox offline.", "ssh: connect to host devbox port 22: Connection refused"], "a later outage notifies again"); + } finally { + watch.dispose(); + } +}); + +test("Retry in the notice and the status bar retries the host", async () => { + const f = fakes(); + f.set("offline"); + const watch = showSourceWindowHostState("abc-1", f.hosts, f.statusbar, f.notifications, f.connection, f.retry, undefined, 5); + try { + await until(() => f.shown.length === 1); + await f.shown[0]?.actions?.primary?.[0]?.run(); + await CommandsRegistry.getCommand(RETRY_HOST_COMMAND)?.handler(undefined as never); + assert.deepEqual(f.retried, ["devbox", "devbox"]); + } finally { + watch.dispose(); + } +}); + +test("a host that refuses its sign-in is named so, with no Retry, and the open notice follows the state", async () => { + const f = fakes(); + f.set("offline"); + const watch = showSourceWindowHostState("abc-1", f.hosts, f.statusbar, f.notifications, f.connection, f.retry, undefined, 5); + try { await until(() => f.last()?.text === OFFLINE); - assert.equal(f.shown.length, 1, "no second notification in the same window"); + f.set("auth-failed", "Permission denied (publickey)."); + await until(() => f.last()?.text === "Can't sign in to devbox"); + assert.equal(f.last()?.command, undefined); + assert.deepEqual(f.shown.map((n) => [n.message, labels(n)]), [["Permission denied (publickey).", []]]); } finally { watch.dispose(); } @@ -110,8 +151,8 @@ test("a window restored before main knows its host's alias names it by the alias first.contribution.dispose(); const { f, contribution } = open({ state: "offline" }); try { - await until(() => f.last()?.text === "wb-test-a — offline, reconnecting…"); - assert.equal(f.shown[0]?.message, "wb-test-a is offline. The window reconnects when it is back."); + await until(() => f.last()?.text === "wb-test-a offline"); + assert.equal(f.shown[0]?.message, "wb-test-a offline."); } finally { contribution.dispose(); } diff --git a/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.ts b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.ts index 44ec092cc..7c3ba0603 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.ts @@ -3,8 +3,11 @@ * Licensed under the MIT License. See LICENSE in the repository root for license information. *--------------------------------------------------------------------------------------------*/ +import { toAction } from "../../../base/common/actions.js"; +import { Event } from "../../../base/common/event.js"; import { Disposable, DisposableStore, toDisposable, type IDisposable } from "../../../base/common/lifecycle.js"; import Severity from "../../../base/common/severity.js"; +import { CommandsRegistry } from "../../../platform/commands/common/commands.js"; import { IMainProcessService } from "../../../platform/ipc/common/mainProcessService.js"; import { INotificationService, type INotificationHandle } from "../../../platform/notification/common/notification.js"; import { PersistentConnectionEventType } from "../../../platform/remote/common/remoteAgentConnection.js"; @@ -13,46 +16,71 @@ import { IWorkbenchEnvironmentService } from "../../../workbench/services/enviro import { IRemoteAgentService, type IRemoteAgentConnection } from "../../../workbench/services/remote/common/remoteAgentService.js"; import { IStatusbarService, StatusbarAlignment, type IStatusbarEntry, type IStatusbarEntryAccessor } from "../../../workbench/services/statusbar/browser/statusbar.js"; import { IStorageService, StorageScope, StorageTarget } from "../../../platform/storage/common/storage.js"; +import { reviewHostStatus, type ReviewGatewayHostState } from "../../common/reviewProtocol.js"; import { REVIEW_DESKTOP_CHANNEL } from "../../common/reviewDesktopBootstrap.js"; import { isReviewRemoteAuthority, reviewRemoteServerId } from "./reviewRemoteAuthority.js"; import { reviewWindowHosts, type ReviewWindowHosts } from "./reviewWindowAuthorityResolver.js"; export const HOST_STATE_POLL_MS = 2_000; +export const RETRY_HOST_COMMAND = "review.sourceWindow.retryHost"; + const HOST_ALIAS_KEY = "review.sourceWindow.hostAlias"; -/** Shows a Source window's host as main sees it: its alias, or that it is offline and the window is reconnecting. */ +/** Shows a Source window's host as main sees it, with Retry while it is offline. */ export function showSourceWindowHostState( serverId: string, hosts: ReviewWindowHosts, statusbar: IStatusbarService, notifications: INotificationService, connection: Pick | null, + retry: (alias: string) => void, stored?: { get(): string | undefined; set(alias: string): void }, every = HOST_STATE_POLL_MS, ): IDisposable { const store = new DisposableStore(); let mainAlias: string | undefined; - let known = false; - let online = true; + let host: Pick | undefined; let lost = false; let notified = false; let entry: IStatusbarEntryAccessor | undefined; let notice: INotificationHandle | undefined; + const alias = () => mainAlias ?? stored?.get() ?? serverId.slice(0, 8); + store.add(CommandsRegistry.registerCommand(RETRY_HOST_COMMAND, () => retry(alias()))); + const retryAction = toAction({ id: RETRY_HOST_COMMAND, label: "Retry", run: () => retry(alias()) }); const render = () => { - if (!known) return; - const alias = mainAlias ?? stored?.get() ?? serverId.slice(0, 8); - const offline = !online || lost; - const text = offline ? `${alias} — offline, reconnecting…` : alias; - const props: IStatusbarEntry = { name: "Remote Host", text, ariaLabel: text, kind: offline ? "warning" : undefined }; + if (!host) return; + const state = lost && host.state === "online" ? "offline" : host.state; + const status = reviewHostStatus({ alias: alias(), state, detail: host.detail }); + const offline = state !== "online"; + const canRetry = status.action === "retry"; + const props: IStatusbarEntry = { + name: "Remote Host", + text: status.label, + ariaLabel: status.label, + tooltip: offline ? status.sentence : undefined, + kind: offline ? "warning" : undefined, + command: canRetry ? RETRY_HOST_COMMAND : undefined, + }; if (entry) entry.update(props); else entry = store.add(statusbar.addEntry(props, "review.sourceWindow.host", StatusbarAlignment.LEFT, Number.MAX_VALUE)); - if (offline && !notified) { - notified = true; - notice = notifications.notify({ severity: Severity.Warning, message: `${alias} is offline. The window reconnects when it is back.` }); - } else if (!offline) { + if (!offline) { + notified = false; notice?.close(); notice = undefined; + return; + } + const actions = { primary: canRetry ? [retryAction] : [] }; + if (notice) { + notice.updateMessage(status.sentence); + notice.updateActions(actions); + } else if (!notified) { + notified = true; + const shown = notifications.notify({ severity: Severity.Warning, message: status.sentence, actions }); + notice = shown; + Event.once(shown.onDidClose)(() => { + if (notice === shown) notice = undefined; + }); } }; if (connection) { @@ -64,13 +92,12 @@ export function showSourceWindowHostState( let stopped = false; let handle: ReturnType | undefined; const tick = async () => { - const state = await hosts.state(serverId).catch(() => undefined); + const state = await hosts.state(serverId, mainAlias ?? stored?.get()).catch(() => undefined); if (stopped) return; if (state) { if (state.alias && state.alias !== mainAlias) stored?.set(state.alias); mainAlias = state.alias ?? mainAlias; - online = state.state === "online"; - known = true; + host = state; render(); } handle = setTimeout(() => void tick(), every); @@ -97,8 +124,9 @@ export class ReviewSourceWindowHostState extends Disposable implements IWorkbenc super(); const authority = environment.remoteAuthority; if (!authority || !isReviewRemoteAuthority(authority)) return; - const hosts = reviewWindowHosts(mainProcess.getChannel(REVIEW_DESKTOP_CHANNEL)); - this._register(showSourceWindowHostState(reviewRemoteServerId(authority), hosts, statusbar, notifications, remoteAgent.getConnection(), { + const channel = mainProcess.getChannel(REVIEW_DESKTOP_CHANNEL); + const retry = (alias: string) => void channel.call("retryRemoteHost", alias).catch(() => undefined); + this._register(showSourceWindowHostState(reviewRemoteServerId(authority), reviewWindowHosts(channel), statusbar, notifications, remoteAgent.getConnection(), retry, { get: () => storage.get(HOST_ALIAS_KEY, StorageScope.WORKSPACE), set: (alias) => storage.store(HOST_ALIAS_KEY, alias, StorageScope.WORKSPACE, StorageTarget.MACHINE), })); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewWindowAuthorityResolver.ts b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewWindowAuthorityResolver.ts index 25c8ca500..e84366b9d 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewWindowAuthorityResolver.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewWindowAuthorityResolver.ts @@ -22,7 +22,7 @@ export const REMOTE_WINDOW_RESOLVE_WAIT_MS = 60_000; export interface ReviewWindowHosts { endpoint(serverId: string): Promise; - state(serverId: string): Promise<({ alias?: string } & Pick) | undefined>; + state(serverId: string, last?: string): Promise<({ alias?: string } & Pick) | undefined>; } /** Reloads a window that could not reach its host, once the host's forward answers. */ @@ -45,7 +45,7 @@ export function reloadWhenOnline(hosts: ReviewWindowHosts, serverId: string, rel export function reviewWindowHosts(channel: IChannel): ReviewWindowHosts { return { endpoint: (serverId) => channel.call("getRemoteLanguageEndpoint", serverId), - state: (serverId) => channel.call("getRemoteHostState", serverId), + state: (serverId, last) => channel.call("getRemoteHostState", last === undefined ? serverId : [serverId, last]), }; } diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts index a8243ae5b..4f62fd4d7 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts @@ -10,6 +10,7 @@ import type { VSBuffer } from "../../base/common/buffer.js"; import { Event } from "../../base/common/event.js"; import { URI } from "../../base/common/uri.js"; import { ReviewCanvasEditorInput } from "../browser/parts/canvas/reviewCanvasEditorInput.js"; +import { ReviewHostDown } from "../browser/parts/canvas/reviewHostFailure.js"; import { apiSourceUri } from "../common/reviewSourceView.js"; import { ReviewCanvasEditorTabsService } from "./reviewCanvasEditorTabsService.js"; @@ -67,7 +68,7 @@ function sourceTabs(t: TestContext, answer: (url: URL) => object) { { createInstance: (_ctor: unknown, target: never) => new ReviewCanvasEditorInput(target, {} as never) } as never, { onDidCloseEditor: Event.None } as never, {} as never, - { async getConnection() { return { serverUrl: "http://localhost", token: "test" }; } } as never, + { async getConnection() { return { serverUrl: "http://localhost", token: "test" }; }, async readRemoteHosts() { return []; } } as never, { async openWindow(toOpen: never, options: never) { opened.push({ toOpen, options }); } } as never, { warn() {} } as never, { async writeFile(resource: URI, content: VSBuffer) { written.push(`${resource.toString()}=${content.toString()}`); } } as never, @@ -112,3 +113,10 @@ test("a remote diff opens its empty side from a laptop empty file", async (t) => assert.equal(options.remoteAuthority, HOST); assert.equal(options.diffMode, true); }); + +test("a gateway 503 naming a host is that host's error", async (t) => { + const { tabs } = sourceTabs(t, () => remoteAnswer); + t.mock.method(globalThis, "fetch", async () => Response.json({ ok: false, error: "gateway text" }, { status: 503, headers: { "x-review-host": "devbox" } })); + await assert.rejects(tabs.openApiSource({ reviewId: "r1", kind: "current" }, "Fix the parser"), (error: unknown) => + error instanceof ReviewHostDown && error.host.alias === "devbox" && error.host.state === "offline"); +}); diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts index fcf934483..5e5865bbc 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.ts @@ -23,6 +23,7 @@ import { ReviewCanvasEditorInput, type ReviewCanvasEditorTarget, } from "../browser/parts/canvas/reviewCanvasEditorInput.js"; +import { ReviewHostDown } from "../browser/parts/canvas/reviewHostFailure.js"; import { reviewSourceQuery, type ReviewRemoteNavigatorAnswer, type ReviewSourceSelection } from "../common/reviewProtocol.js"; import { REVIEW_LANGUAGE_SOURCE_SCHEME } from "../common/reviewReadonlySource.js"; @@ -187,6 +188,11 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview headers: { "x-review-token": token }, signal: AbortSignal.timeout(60_000), }); + const alias = response.status === 503 && response.headers.get("x-review-host"); + if (alias) { + const host = (await this.desktopConnection.readRemoteHosts().catch(() => [])).find(host => host.alias === alias); + throw new ReviewHostDown(host && host.state !== "online" ? host : { alias, state: "offline" }); + } if (!response.ok) throw await reviewResponseError(response, "Could not open the code navigator."); return response.json(); } diff --git a/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs b/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs index 7f0c1fc92..a71711f4d 100644 --- a/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs +++ b/apps/review-desktop/scripts/e2e/journeys/remote-host.mjs @@ -420,7 +420,9 @@ async function journey(ctx, page, until) { 'button[aria-label="Share review"], button[aria-label="Shared review"], [aria-label="Session views"] button[aria-label="Trace"]', ); - const sourceTree = canvas.locator('button[aria-label="Source tree ↗"]'); + const sourceTree = canvas.locator( + 'button[aria-label="Source tree ↗"]:not([disabled])', + ); await until( async () => diff --git a/apps/review-desktop/scripts/e2e/journeys/remote-source-window.mjs b/apps/review-desktop/scripts/e2e/journeys/remote-source-window.mjs index eed87b1f1..be019eb4c 100644 --- a/apps/review-desktop/scripts/e2e/journeys/remote-source-window.mjs +++ b/apps/review-desktop/scripts/e2e/journeys/remote-source-window.mjs @@ -458,7 +458,7 @@ async function journey(ctx) { await until(() => other.isClosed(), "the second Source window to close"); // 11–12. The host freezes: the window's status bar says so, then names the host again. - const offlineText = `${alias} — offline, reconnecting…`; + const offlineText = `${alias} offline`; let thaw = await freeze(); const paused = Date.now(); diff --git a/docs/remote-hosts.md b/docs/remote-hosts.md index e6832ea4b..43ed541a2 100644 --- a/docs/remote-hosts.md +++ b/docs/remote-hosts.md @@ -226,8 +226,8 @@ login shell (`~/.profile` or your shell's own start-up file is enough): "Open file", the source tree and Ask's file links open a read-only Source window on the host, with search and language features. -When the host stops answering, the status bar says so. The window reconnects -when the host is back. +When the host stops answering, the status bar says so and offers Retry. The +window reconnects when the host is back. Not available in a Source window: diff --git a/packages/review-protocol/scripts/generate-native-source.mjs b/packages/review-protocol/scripts/generate-native-source.mjs index ff78db185..f072630da 100644 --- a/packages/review-protocol/scripts/generate-native-source.mjs +++ b/packages/review-protocol/scripts/generate-native-source.mjs @@ -123,6 +123,7 @@ const MODULE_PATHS = [ path.join(sourceRoot, "structural-diff.ts"), path.join(sourceRoot, "source-alignment.ts"), path.join(sourceRoot, "contracts.ts"), + path.join(sourceRoot, "review-host-status.ts"), path.join(sourceRoot, "code-peek-diff.ts"), path.join(sourceRoot, "review-api-client.ts"), path.join(sourceRoot, "index.ts"), diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index d58b7ea64..ecc0113e5 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -771,6 +771,7 @@ export type ReviewCanvasContent = reviewId: string; version?: number; host?: string; + hostState?: ReviewApiSummary["hostState"]; available?: ReviewApiSummary["available"]; remoteHosts?: ReviewRemoteHostActions; bridge: ReviewCanvasBridge; diff --git a/packages/review-protocol/src/native-source.test.ts b/packages/review-protocol/src/native-source.test.ts index 0e233f5be..7e9f7f487 100644 --- a/packages/review-protocol/src/native-source.test.ts +++ b/packages/review-protocol/src/native-source.test.ts @@ -79,6 +79,7 @@ describe("native Review Protocol source generation", () => { "structural-diff.ts", "source-alignment.ts", "contracts.ts", + "review-host-status.ts", "code-peek-diff.ts", "review-api-client.ts", "index.ts", @@ -166,6 +167,7 @@ describe("native Review Protocol source generation", () => { await writeFile(path.join(sourceRoot, "runtime-value.ts"), ""); await writeFile(path.join(sourceRoot, "json.ts"), ""); await writeFile(path.join(sourceRoot, "trace-contracts.ts"), ""); + await writeFile(path.join(sourceRoot, "review-host-status.ts"), ""); await writeFile(path.join(sourceRoot, "code-peek-diff.ts"), ""); await writeFile(path.join(sourceRoot, "review-api-client.ts"), ""); await writeFile( diff --git a/packages/review-protocol/src/review-api-client.ts b/packages/review-protocol/src/review-api-client.ts index a6c85591f..d95b912e5 100644 --- a/packages/review-protocol/src/review-api-client.ts +++ b/packages/review-protocol/src/review-api-client.ts @@ -51,7 +51,11 @@ export interface ReviewApiSummary { working?: boolean; host?: string; hostState?: Exclude; - available?: { sourceWindows: boolean }; + available?: { + sourceWindows: boolean; + /** Why they are off: the host's sentence, or its language features detail. */ + reason?: string; + }; } /** diff --git a/packages/review/app/src/App.tsx b/packages/review/app/src/App.tsx index 7783ad158..eff65b9f3 100644 --- a/packages/review/app/src/App.tsx +++ b/packages/review/app/src/App.tsx @@ -295,12 +295,21 @@ function ReviewLayoutContent({ // The scratchpad is a document and nothing else: no source tree to browse, // nothing to share, nothing to dismiss. const scratchpad = session.review?.kind === "scratchpad"; - const sourceWindows = session.review?.available?.sourceWindows !== false; + const available = session.review?.available; + // A remote review keeps Source tree, disabled with the reason. + const sourceWindows = available?.sourceWindows !== false; + const sourceTreeShown = sourceWindows || !!session.review?.host; useEffect(() => { if (scratchpad) captureUiEvent(session, "scratchpad_opened"); }, [scratchpad, session]); const discordTooltip = useTooltip("Join our Discord community"); - const sourceTreeTooltip = useTooltip("Open full read-only source"); + + const sourceTreeTooltip = useTooltip( + sourceWindows + ? "Open full read-only source" + : (available?.reason ?? "Source windows are unavailable."), + ); + const panelStore = useReviewPanelStore(); useSuppressPanelMotionOnCanvasResume(appRef); const activePanel = useReviewPanel((state) => state.active); @@ -614,20 +623,21 @@ function ReviewLayoutContent({ shellStyles.topbarContext, )} > - {!scratchpad && sourceWindows && ( + {!scratchpad && sourceTreeShown && ( ) : null} @@ -685,7 +725,7 @@ export function AskPanelContent({ ? "/ for commands, @ for files" : "@ for files", )} - disabled={thread?.status === "failed"} + disabled={thread?.status === "failed" || down !== undefined} canAsk={canAsk} stop={busy && threadId ? stop : undefined} connecting={thread?.status === "starting"} @@ -779,6 +819,25 @@ export function AskReadOnlyThread({ ); } +function AskHostBanner({ + status, + host, + hosts, +}: { + status: ReviewHostStatus; + host?: string; + hosts?: ReviewRemoteHostActions; +}): ReactElement { + const { failure, button } = useHostActionButton(hosts, host, status.action); + const why = failure ?? status.sentence; + + return ( + + {host && why.includes(host) ? why : `${status.label} · ${why}`} + + ); +} + function ToLatestButton({ onClick }: { onClick: () => void }): ReactElement { const label = "Scroll to the latest"; diff --git a/packages/review/app/src/host/review-session-data.ts b/packages/review/app/src/host/review-session-data.ts index f2618882c..96ae6fc93 100644 --- a/packages/review/app/src/host/review-session-data.ts +++ b/packages/review/app/src/host/review-session-data.ts @@ -1,6 +1,7 @@ import type { LoadedAgentTrace } from "@canvas/use-agent-trace"; import type { ReviewApiSummary, + ReviewRemoteHostActions, ReviewStackLayer, } from "@dev.fast/review-protocol"; @@ -9,6 +10,8 @@ export interface ReviewSessionData { /** Absent for a review. The scratchpad hides review-only chrome. */ kind?: "scratchpad"; host?: string; + hostState?: ReviewApiSummary["hostState"]; + hosts?: ReviewRemoteHostActions; available?: ReviewApiSummary["available"]; /** Absent for a document whose references all carry their own pins. */ pins?: { base: string; head: string }; diff --git a/packages/review/app/src/missing-checkout-banner.browser.test.tsx b/packages/review/app/src/missing-checkout-banner.browser.test.tsx new file mode 100644 index 000000000..c914f5055 --- /dev/null +++ b/packages/review/app/src/missing-checkout-banner.browser.test.tsx @@ -0,0 +1,44 @@ +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { expect, it } from "vitest"; + +import { ReviewSessionProvider } from "./host/review-session"; +import { MissingCheckoutBanner } from "./missing-checkout-banner"; +import { ReviewProvider } from "./review-context"; +import { testReviewSession } from "./review-session-test-utils"; + +it("names a remote review's host and shows a failed move to Trash in the banner", async () => { + const session = testReviewSession(); + session.review!.host = "devbox"; + session.review!.dismiss = async () => { + throw new Error("devbox offline."); + }; + + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + + try { + await act(async () => + root.render( + + + + + , + ), + ); + + expect(container.textContent).toContain( + "The checkout on devbox is unavailable.", + ); + await act(async () => container.querySelector("button")!.click()); + expect(container.querySelector('[role="alert"]')?.textContent).toContain( + "devbox offline.", + ); + expect(container.querySelector("button")!.disabled).toBe(false); + } finally { + await act(async () => root.unmount()); + container.remove(); + } +}); diff --git a/packages/review/app/src/missing-checkout-banner.tsx b/packages/review/app/src/missing-checkout-banner.tsx index 5aca6f282..6ad07dddf 100644 --- a/packages/review/app/src/missing-checkout-banner.tsx +++ b/packages/review/app/src/missing-checkout-banner.tsx @@ -5,6 +5,11 @@ import { type ReactElement, useState } from "react"; import { useReviewSession } from "./host/review-session"; import { useReviewActions, useReviewState } from "./review-context"; +export const checkoutUnavailable = (host?: string) => + host + ? `The checkout on ${host} is unavailable.` + : "Local checkout unavailable."; + export function MissingCheckoutBanner({ worktree, }: { @@ -13,7 +18,9 @@ export function MissingCheckoutBanner({ const session = useReviewSession(); const { dismissReview } = useReviewActions(); const { submissionOutcome } = useReviewState(); + const host = useReviewSession().review?.host; const [busy, setBusy] = useState(false); + const [error, setError] = useState(); return ( { setBusy(true); + setError(undefined); try { await dismissReview(); - } catch (error) { - console.error("Review action failed", error); + } catch (failure) { + setError( + `Could not move to Trash: ${failure instanceof Error ? failure.message : String(failure)}`, + ); setBusy(false); } }} @@ -40,8 +50,9 @@ export function MissingCheckoutBanner({ > {worktree ? "This review's worktree was removed." - : "Local checkout unavailable."}{" "} + : checkoutUnavailable(host)}{" "} Showing the source saved with the review. + {error && {error}} ); } diff --git a/packages/review/app/src/remote-host-state.ts b/packages/review/app/src/remote-host-state.ts index 7c37d9b94..9ccfd7e6c 100644 --- a/packages/review/app/src/remote-host-state.ts +++ b/packages/review/app/src/remote-host-state.ts @@ -1,10 +1,13 @@ -import type { - ReviewGatewayHostState, - ReviewHostStatus, - ReviewRemoteHostActions, +import { + type ReviewGatewayHostState, + type ReviewHostStatus, + type ReviewRemoteHostActions, + reviewHostStatus, } from "@dev.fast/review-protocol"; import { type RefObject, useEffect, useState } from "react"; +import type { ReviewSessionData } from "./host/review-session-data"; + const STATES_EVERY_MS = 3000; export function useRemoteHostStates( @@ -54,6 +57,24 @@ export function useRemoteHostState( ); } +/** A remote review's host while it is not online, with the reason read live. */ +export function useReviewHostDown( + review: Pick | undefined, +): ReviewHostStatus | undefined { + const { host, hostState, hosts } = review ?? {}; + + const down = + host !== undefined && hostState !== undefined && hostState !== "online"; + + const live = useRemoteHostState(hosts, host, down); + + if (!down) return undefined; + + return reviewHostStatus( + live && live.state !== "online" ? live : { alias: host, state: hostState }, + ); +} + export const ACTION_WORDS: Record< NonNullable, string diff --git a/packages/review/app/src/review-home-view.tsx b/packages/review/app/src/review-home-view.tsx index e3441c26b..c34af084b 100644 --- a/packages/review/app/src/review-home-view.tsx +++ b/packages/review/app/src/review-home-view.tsx @@ -43,7 +43,7 @@ import { } from "./icons"; import { homeGroupMarker, homeRowMarker } from "./markers.stylex"; import { useNotify } from "./notify"; -import { useRemoteHostState } from "./remote-host-state"; +import { useReviewHostDown } from "./remote-host-state"; import type { ReviewFindHost } from "./review-find"; import { HomeFilterPicker, @@ -1063,13 +1063,7 @@ export function ReviewHome({ ) : null} {error &&

{error}

} - {waiting?.host && waiting.hostState ? ( - - ) : null} + {waiting ? : null} {!visible.length && ( ; + review: ReviewApiSummary; hosts?: ReviewRemoteHostActions; }) { - const live = useRemoteHostState(hosts, alias, true); - const status = reviewHostStatus(live ?? { alias, state }); - const { failure, button } = useHostActionButton(hosts, alias, status.action); + const status = useReviewHostDown({ ...review, hosts }); + + const { failure, button } = useHostActionButton( + hosts, + review.host, + status?.action, + ); + + if (!status) return null; return (

diff --git a/packages/review/app/src/review-lenses.tsx b/packages/review/app/src/review-lenses.tsx index daf31a037..af75b94eb 100644 --- a/packages/review/app/src/review-lenses.tsx +++ b/packages/review/app/src/review-lenses.tsx @@ -32,6 +32,7 @@ import { import { canvasQueryKeys } from "./canvas-query"; import { useReviewSession } from "./host/review-session"; +import { checkoutUnavailable } from "./missing-checkout-banner"; import { useReviewPanel, useReviewPanelStore } from "./review-panel"; import { captureUiEvent } from "./ui-telemetry"; @@ -184,7 +185,7 @@ export function ReviewLensesProvider({ // The latest outcome wins: a later read clears a failed mark. const error = snapshot.sourceUnavailable - ? "Local checkout unavailable." + ? checkoutUnavailable(session.review?.host) : markViewed.error && markViewed.submittedAt > read.dataUpdatedAt ? String(markViewed.error) : read.error diff --git a/packages/review/src/server/review-gateway-list.test.ts b/packages/review/src/server/review-gateway-list.test.ts index fc7d1b6a1..92f3c2559 100644 --- a/packages/review/src/server/review-gateway-list.test.ts +++ b/packages/review/src/server/review-gateway-list.test.ts @@ -85,7 +85,7 @@ it("offers language features and Source windows only for an online host whose De expect(merged.map((review) => [review.host, review.available])).toEqual([ ["a", { sourceWindows: true }], ["b", { sourceWindows: false }], - ["c", { sourceWindows: false }], + ["c", { sourceWindows: false, reason: "c offline." }], ]); }); @@ -184,3 +184,48 @@ it("gives an id to this machine, then to the first host in the setting's order t ]); expect(listedBy(onBoth.reviewId, lists)?.state.alias).toBe("b"); }); + +it("says why Source windows are off: the host's sentence while it is down, else the language features detail", () => { + const merged = mergeLists( + "structural", + [], + source( + [ + { alias: "a", serverId: "A", state: "online", languageFeatures: true }, + { + alias: "b", + serverId: "B", + state: "online", + languageFeatures: false, + languageFeaturesDetail: "The Whiteboard on b has no VS Code server.", + }, + { alias: "c", serverId: "C", state: "offline", languageFeatures: true }, + { + alias: "d", + serverId: "D", + state: "auth-failed", + detail: "Permission denied.", + }, + { alias: "e", serverId: "E", state: "online", detail: "stale" }, + ], + { + A: [entry("on a")], + B: [entry("on b")], + C: [entry("on c")], + D: [entry("on d")], + E: [entry("on e")], + }, + ["A", "B"], + ), + ); + + expect( + merged.map((review) => [review.host, review.available?.reason]), + ).toEqual([ + ["a", undefined], + ["b", "The Whiteboard on b has no VS Code server."], + ["c", "c offline."], + ["d", "Permission denied."], + ["e", "e offline."], + ]); +}); diff --git a/packages/review/src/server/review-gateway-list.ts b/packages/review/src/server/review-gateway-list.ts index ffdc9cef0..9c477a23e 100644 --- a/packages/review/src/server/review-gateway-list.ts +++ b/packages/review/src/server/review-gateway-list.ts @@ -1,6 +1,7 @@ -import type { - ReviewApiSummary, - ReviewGatewayHostState, +import { + type ReviewApiSummary, + type ReviewGatewayHostState, + reviewHostStatus, } from "@dev.fast/review-protocol"; import { LIST_MODES, type ListMode } from "./review-gateway-memory.js"; @@ -18,7 +19,18 @@ function decorate( hostState: NonNullable, ): ReviewApiSummary { const { alias } = state; - const features = hostState === "online" && state.languageFeatures === true; + const online = hostState === "online"; + const features = online && state.languageFeatures === true; + + const reason = online + ? features + ? undefined + : state.languageFeaturesDetail + : reviewHostStatus({ + alias, + state: hostState, + detail: hostState === state.state ? state.detail : undefined, + }).sentence; return { ...entry, @@ -30,7 +42,10 @@ function decorate( }), host: alias, hostState, - available: { sourceWindows: features }, + available: { + sourceWindows: features, + ...(reason && { reason }), + }, }; }