From 5c67e94bca555fc7799cda731f638482e76acfb5 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Tue, 6 Oct 2026 16:15:14 -0400 Subject: [PATCH 01/10] Show a Source window's host state, with Retry while offline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status bar and the notice use the host's own words, such as "Can't sign in to alex-box", instead of "offline, reconnecting…" for every state. While the host is offline, both offer Retry. A later outage in the same window notifies again. Agent-Session: 3a3376c5-3f72-46df-930c-a66d5c3bcce5 Agent-Session: 7633c50b-998d-4b1e-8ca2-ffb350a26066 Agent-Session: 9a4d92ed-7a0d-42a1-9169-574bca60095b Agent-Session: 1c093463-922b-435b-a925-664d5d5c3c0a Agent-Session: 2b51b153-90b5-4a72-a052-fccf0beb9f1c --- .../reviewSourceWindowHostState.test.ts | 73 +++++++++++++++---- .../remote/reviewSourceWindowHostState.ts | 60 +++++++++++---- docs/remote-hosts.md | 4 +- .../scripts/generate-native-source.mjs | 1 + .../review-protocol/src/native-source.test.ts | 2 + 5 files changed, 106 insertions(+), 34 deletions(-) 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..b2c2ec88b 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) { @@ -69,8 +97,7 @@ export function showSourceWindowHostState( 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/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/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( From 3081e5a7bad4faccb7b7a24fc162d813aefbf62d Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Tue, 6 Oct 2026 16:21:38 -0400 Subject: [PATCH 02/10] Fail at once when opening source on a host that is down Opening a remote review's source used to wait up to a minute and then show the raw error. When the host is known to be down it now fails at once, with the host's words and Retry. The canvas no longer leaves the rejection unhandled. Agent-Session: 3a3376c5-3f72-46df-930c-a66d5c3bcce5 Agent-Session: 7633c50b-998d-4b1e-8ca2-ffb350a26066 Agent-Session: 9a4d92ed-7a0d-42a1-9169-574bca60095b Agent-Session: 1c093463-922b-435b-a925-664d5d5c3c0a Agent-Session: 2b51b153-90b5-4a72-a052-fccf0beb9f1c --- .../canvas/reviewApiEditorSerializer.test.ts | 5 ++- .../browser/parts/canvas/reviewCanvasPart.ts | 37 +++++++++++++------ .../browser/parts/canvas/reviewHostFailure.ts | 28 ++++++++++++++ .../reviewCanvasEditorTabsService.test.ts | 22 ++++++++++- .../services/reviewCanvasEditorTabsService.ts | 10 +++++ .../reviewEditorResolverService.test.ts | 1 + packages/review/app/src/App.tsx | 8 ++-- 7 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewHostFailure.ts diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts index 8c1635fa0..550a7d54b 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts @@ -28,6 +28,7 @@ test("opening historical source requests its version and opens a separate native { warn() {} } as never, {} as never, {} as never, + { reviews: [] } as never, ); t.after(() => tabs.dispose()); await tabs.openApiSource({ reviewId: "review-a", kind: "version", version: 7 }, "Historical Review"); @@ -63,7 +64,7 @@ test("native group restoration preserves both reviews, order and pinned source v }; const groupService = { groups, mainPart: { activeGroup: undefined as EditorGroupModel | undefined } }; const createTabs = () => - new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never); + new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, { reviews: [] } as never); tabs = createTabs(); const registry = Registry.as(EditorExtensions.EditorFactory); registry.start({ get: () => instantiation } as never); @@ -130,7 +131,7 @@ test("current Source tabs retain identity and main version tabs still restore", }, invokeFunction(fn: (accessor: { get(): ReviewCanvasEditorTabsService }) => unknown) { return fn({ get: () => tabs }); }, }; - tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never); + tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, { reviews: [] } as never); try { const serializer = new ReviewApiEditorSerializer(); const restored = serializer.deserialize(instantiation as never, JSON.stringify({ kind: "api-source", reviewId: "a", title: "A", selection: { reviewId: "a", kind: "current" } })); 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/services/reviewCanvasEditorTabsService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewCanvasEditorTabsService.test.ts index a8243ae5b..a207e1d20 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"; @@ -39,6 +40,7 @@ async function closeWelcome(updateNeeded: boolean): Promise { { warn() {} } as never, {} as never, {} as never, + { reviews: [] } as never, ); try { const welcome = await tabs.openWelcome(true); @@ -59,7 +61,7 @@ const HOST = "whiteboard+c0ffee"; const remote = (path: string) => URI.from({ scheme: "vscode-remote", authority: HOST, path }).toString(); const view = (generation?: string) => ({ reviewId: "r1", version: 2, generation }); -function sourceTabs(t: TestContext, answer: (url: URL) => object) { +function sourceTabs(t: TestContext, answer: (url: URL) => object, reviews: object[] = []) { const opened: { toOpen: { workspaceUri?: URI; fileUri?: URI; label?: string }[]; options: Record }[] = []; const written: string[] = []; t.mock.method(globalThis, "fetch", async (url: string) => Response.json(answer(new URL(url)))); @@ -67,11 +69,12 @@ 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, { cacheHome: URI.file("/laptop/cache") } as never, + { reviews } as never, ); t.after(() => tabs.dispose()); return { tabs, opened, written }; @@ -112,3 +115,18 @@ 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("an offline host's review fails at once with its host's words and asks nothing", async (t) => { + const { tabs } = sourceTabs(t, () => remoteAnswer, [{ reviewId: "r1", host: "devbox", hostState: "offline" }]); + const fetch = globalThis.fetch as unknown as { mock: { callCount(): number } }; + await assert.rejects(tabs.openApiSource({ reviewId: "r1", kind: "current" }, "Fix the parser"), (error: unknown) => + error instanceof ReviewHostDown && error.host.alias === "devbox" && error.message === "devbox offline."); + assert.equal(fetch.mock.callCount(), 0); +}); + +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..5984d0778 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,11 +23,13 @@ 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"; import { sourceLocation, sourceSelectionIdentity, REVIEW_API_SOURCE_SCHEME } from "../common/reviewSourceView.js"; import { reviewRemoteAuthority } from "./remote/reviewRemoteAuthority.js"; +import { IReviewApiCatalogService } from "./reviewApiCatalogService.js"; import { IReviewDesktopConnectionService, reviewResponseError } from "./reviewDesktopConnectionService.js"; type NavigatorAnswer = { workspacePath: string; filePath?: string } | ReviewRemoteNavigatorAnswer; @@ -76,6 +78,7 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview @ILogService private readonly logService: ILogService, @IFileService private readonly files: IFileService, @IEnvironmentService private readonly environment: IEnvironmentService, + @IReviewApiCatalogService private readonly catalog: IReviewApiCatalogService, ) { super(); this._register( @@ -180,6 +183,8 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview } private async navigatorWorkspace(reviewId: string, values: Record): Promise { + const review = this.catalog.reviews.find(review => review.reviewId === reviewId); + if (review?.host && review.hostState && review.hostState !== "online") throw new ReviewHostDown({ alias: review.host, state: review.hostState }); const { serverUrl, token } = await this.desktopConnection.getConnection(); const query = new URLSearchParams(Object.entries(values).filter(([key, value]) => key !== "reviewId" && value !== undefined).map(([key, value]) => [key, String(value)])); const response = await fetch(`${serverUrl}/reviews-api/${encodeURIComponent(reviewId)}/navigator${query.size ? `?${query}` : ""}`, { @@ -187,6 +192,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/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts index 0139f6443..f902bc5ee 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts @@ -30,6 +30,7 @@ test("source tree, selected code, definitions and diffs hand off before creating { warn() {} } as never, {} as never, {} as never, + { reviews: [] } as never, ); const resolver = new ReviewEditorResolverService( { get activeGroup() { throw new Error("Review must not create an editor group"); } } as never, diff --git a/packages/review/app/src/App.tsx b/packages/review/app/src/App.tsx index 7783ad158..3db189608 100644 --- a/packages/review/app/src/App.tsx +++ b/packages/review/app/src/App.tsx @@ -624,10 +624,10 @@ function ReviewLayoutContent({ captureUiEvent(session, "source_tree_opened", { via: "topbar", }); - session.surface.post({ - name: "openSourceTree", - args: {}, - }); + // The desktop already showed why it failed. + session.surface + .post({ name: "openSourceTree", args: {} }) + .catch(() => undefined); }} > From 63ca2dbfdc0cff1f9020cd701d54f6b933ee9ede Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Tue, 6 Oct 2026 19:45:56 -0400 Subject: [PATCH 03/10] Block Ask while a review's host is down The input is disabled and a banner says why, with the step that helps. A question is never queued. A failed request offers Try again, and a host that cannot list its agents for now keeps the last list, so Ask does not disappear. Agent-Session: 7633c50b-998d-4b1e-8ca2-ffb350a26066 Agent-Session: 9a4d92ed-7a0d-42a1-9169-574bca60095b Agent-Session: 1c093463-922b-435b-a925-664d5d5c3c0a Agent-Session: 2b51b153-90b5-4a72-a052-fccf0beb9f1c --- .../parts/canvas/reviewRemoteEntry.test.ts | 2 + .../browser/parts/canvas/reviewRemoteEntry.ts | 6 +- packages/review-protocol/src/contracts.ts | 1 + packages/review/app/src/api-canvas.tsx | 4 + packages/review/app/src/ask-agent-picker.tsx | 54 +++-- packages/review/app/src/ask-panel.test.tsx | 198 +++++++++++++++++- packages/review/app/src/ask-panel.tsx | 105 ++++++++-- .../app/src/host/review-session-data.ts | 3 + packages/review/app/src/remote-host-state.ts | 29 ++- 9 files changed, 360 insertions(+), 42 deletions(-) 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/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/app/src/api-canvas.tsx b/packages/review/app/src/api-canvas.tsx index 84e03918f..1c839f6b6 100644 --- a/packages/review/app/src/api-canvas.tsx +++ b/packages/review/app/src/api-canvas.tsx @@ -321,6 +321,8 @@ export function ApiCanvas({ kind: snapshot.kind, trashedAt: snapshot.trashedAt, host: content.host, + hostState: content.hostState, + hosts: content.remoteHosts, available: content.available, pins: snapshot.pins ? { base: snapshot.pins.base, head: snapshot.pins.head } @@ -376,6 +378,8 @@ export function ApiCanvas({ client, content.reviewId, content.host, + content.hostState, + content.remoteHosts, content.available, data, version, diff --git a/packages/review/app/src/ask-agent-picker.tsx b/packages/review/app/src/ask-agent-picker.tsx index aa8dada21..a289724d8 100644 --- a/packages/review/app/src/ask-agent-picker.tsx +++ b/packages/review/app/src/ask-agent-picker.tsx @@ -37,6 +37,7 @@ import { textStyles } from "./ui/text"; import { fieldStyles } from "./ui/text-field"; import { useAnchoredPopover } from "./use-anchored-popover"; import { useDismissOnOutside } from "./use-dismiss-on-outside"; +import { useTooltip } from "./use-tooltip"; const agentsSchema = z.object({ agents: z.array( @@ -63,26 +64,37 @@ export const logos: Record< // request under way, which the toolbar and the panel share. const knownAgents = new WeakMap(); -const askingAgents = new WeakMap>(); +// Undefined: the host could not say now. +const askingAgents = new WeakMap< + ReviewSession, + Promise +>(); -function askAgentsOf(session: ReviewSession): Promise { +function askAgentsOf( + session: ReviewSession, +): Promise { let request = askingAgents.get(session); if (!request) { request = session .fetch("/ask/agents") - .then(async (response) => - response.ok ? agentsSchema.parse(await response.json()).agents : null, + .then(async (response) => { + // A host without Ask refuses the request; one that is down fails it. + if (response.status >= 500) throw new Error(response.statusText); + + return response.ok + ? agentsSchema.parse(await response.json()).agents + : null; + }) + .then( + (agents) => { + knownAgents.set(session, agents); + + return agents; + }, + () => knownAgents.get(session), ) - // A check that fails keeps the last answer; a host without Ask refuses - // the request instead. - .catch(() => knownAgents.get(session) ?? null) - .then((agents) => { - knownAgents.set(session, agents); - askingAgents.delete(session); - - return agents; - }); + .finally(() => askingAgents.delete(session)); askingAgents.set(session, request); } @@ -90,8 +102,9 @@ function askAgentsOf(session: ReviewSession): Promise { } /** Which local agents can answer, or null where this host has no Ask. Asked - * again each time Ask opens and each time the review comes back into focus, - * so an agent installed during the review shows up. */ + * again each time Ask opens, each time the review comes back into focus and + * each time its host changes state, so an agent installed during the review + * shows up. A host that cannot answer now keeps the last answer. */ export function useAskAgents(session: ReviewSession | null): AskAgent[] | null { const [agents, setAgents] = useState(() => session ? (knownAgents.get(session) ?? null) : null, @@ -103,7 +116,7 @@ export function useAskAgents(session: ReviewSession | null): AskAgent[] | null { const refresh = () => { void askAgentsOf(session).then((value) => { - if (current) setAgents(value); + if (current && value !== undefined) setAgents(value); }); }; @@ -393,25 +406,32 @@ export function AskAgentPicker({ agents, agent, locked, + unavailable, onPick, }: { agents: AskAgent[] | null; agent: AskAgentId | undefined; locked: boolean; + unavailable?: string; onPick: (agent: AskAgentId) => void; }): ReactElement { const [open, setOpen] = useState(false); const anchor = useRef(null); const trigger = useRef(null); const dismiss = useMenuClose(setOpen, trigger); + const hint = useTooltip(unavailable ?? ""); const chosen = agents?.find((candidate) => candidate.id === agent); if ( locked || + unavailable !== undefined || agents?.filter((candidate) => candidate.available).length === 1 ) { return ( - + {agent ? logos[agent]({}) : null} {chosen?.name ?? agent ?? "Agent"} diff --git a/packages/review/app/src/ask-panel.test.tsx b/packages/review/app/src/ask-panel.test.tsx index 0a2b21914..7256bf703 100644 --- a/packages/review/app/src/ask-panel.test.tsx +++ b/packages/review/app/src/ask-panel.test.tsx @@ -10,7 +10,7 @@ import { useShowOpenThread, } from "./ask-delete"; import { AskHistoryProvider, useAskHistory } from "./ask-history"; -import { AskHistoryList } from "./ask-history-list"; +import { AskHistoryControl, AskHistoryList } from "./ask-history-list"; import { AskPanelContent, AskReadOnlyThread } from "./ask-panel"; import { TestCanvasQuery } from "./canvas-query-test-utils"; import { ReviewSessionProvider } from "./host/review-session"; @@ -1501,3 +1501,199 @@ it("shows a saved conversation whose checkout is gone, with nothing to ask", asy vi.unstubAllGlobals(); } }); + +const twoAgents = () => + Response.json({ + agents: [ + { id: "claude", name: "Claude Code", available: true }, + { id: "codex", name: "Codex", available: true }, + ], + }); + +it("asks nothing while a remote review's host is down, and says why with its next step", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const session = testReviewSession(); + + const hosts = { + states: async () => [ + { alias: "wb-a", state: "offline" as const, detail: "ssh timed out." }, + ], + retry: vi.fn<(alias: string) => Promise>(async () => {}), + install: async () => {}, + openSettings: async () => {}, + }; + + session.review = { + ...session.review!, + host: "wb-a", + hostState: "offline", + hosts, + }; + + const fetch = vi + .spyOn(session, "fetch") + .mockImplementation(async (endpoint) => + endpoint === "/ask/agents" + ? twoAgents() + : Response.json({ ok: true }, { status: 404 }), + ); + + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + + try { + await act(async () => + root.render( + + + , + ), + ); + + const textarea = container.querySelector("textarea")!; + + expect(textarea.disabled).toBe(true); + expect(container.querySelector('[role="status"]')?.textContent).toContain( + "wb-a offline · ssh timed out.", + ); + + await type(textarea, "Is this safe?"); + await act(async () => textarea.form!.requestSubmit()); + expect(fetch.mock.calls.some(([called]) => called === "/ask")).toBe(false); + + await act(async () => buttonNamed(container, "Retry")!.click()); + expect(hosts.retry).toHaveBeenCalledWith("wb-a"); + } finally { + await act(async () => root.unmount()); + container.remove(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + } +}); + +it("offers to try again a question that did not go, and sends it again", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const session = testReviewSession(); + let refused = false; + + const fetch = vi + .spyOn(session, "fetch") + .mockImplementation(async (endpoint, init) => { + if (endpoint === "/ask/agents") return twoAgents(); + + if (endpoint === "/ask") { + if (refused) return Response.json({ threadId: "thread" }); + refused = true; + + return Response.json({ error: "Busy." }, { status: 500 }); + } + + if (endpoint === "/ask/watch") return new Response(new ReadableStream()); + + return Response.json({ ok: true }, { status: init?.method ? 200 : 404 }); + }); + + const asked = () => + fetch.mock.calls + .filter(([called]) => called === "/ask") + .map(([, init]) => JSON.parse(String(init?.body)).question); + + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + + try { + await act(async () => + root.render( + + + , + ), + ); + + const textarea = container.querySelector("textarea")!; + + await type(textarea, "Is this safe?"); + await enter(textarea); + expect(container.querySelector('[role="alert"]')?.textContent).toContain( + "Busy.", + ); + + await act(async () => buttonNamed(container, "Try again")!.click()); + expect(asked()).toEqual([ + { text: "Is this safe?" }, + { text: "Is this safe?" }, + ]); + expect(container.querySelector('[role="alert"]')).toBeNull(); + expect(textarea.value).toBe(""); + } finally { + await act(async () => root.unmount()); + container.remove(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + } +}); + +it("keeps Ask while its host cannot list agents, and lets an agent be picked again once the host is back", async () => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + const session = testReviewSession(); + const header = document.createElement("div"); + document.body.append(header); + let hostState: "online" | "offline" = "online"; + + vi.spyOn(session, "fetch").mockImplementation(async (endpoint) => { + if (endpoint === "/ask/agents") + return hostState === "online" + ? twoAgents() + : Response.json({ error: "wb-a is offline." }, { status: 503 }); + + return Response.json({ ok: true }, { status: 404 }); + }); + + const container = document.createElement("div"); + document.body.append(container); + const root = createRoot(container); + + const picker = () => + header.querySelector('button[aria-haspopup="menu"]:not([aria-label])'); + + // The catalog hands the canvas a new session as the host changes state. + const render = async () => + act(async () => + root.render( + + + + + + , + ), + ); + + try { + await render(); + expect(picker()).not.toBeNull(); + + hostState = "offline"; + await render(); + expect(picker()).toBeNull(); + expect(header.textContent).toBe("Claude Code"); + expect(buttonNamed(container, "Saved conversations")).not.toBeNull(); + + hostState = "online"; + await render(); + expect(picker()).not.toBeNull(); + } finally { + await act(async () => root.unmount()); + container.remove(); + header.remove(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + } +}); diff --git a/packages/review/app/src/ask-panel.tsx b/packages/review/app/src/ask-panel.tsx index cf139e683..5671f569b 100644 --- a/packages/review/app/src/ask-panel.tsx +++ b/packages/review/app/src/ask-panel.tsx @@ -1,3 +1,7 @@ +import type { + ReviewHostStatus, + ReviewRemoteHostActions, +} from "@dev.fast/review-protocol"; import type { AgentSelection } from "@review/agent-selection"; import { type AskAgentId, @@ -55,12 +59,17 @@ import { controlStyles } from "./controls-styles"; import { formatRelativeTime } from "./format"; import { useReviewSession } from "./host/review-session"; import { ArrowUpIcon, ImageIcon, LockIcon } from "./icons"; +import { + ACTION_WORDS, + useHostAction, + useReviewHostDown, +} from "./remote-host-state"; import { useAskKey, useOptionalReviewPanelStore } from "./review-panel"; import type { AskPresence, AskReport } from "./review-panel-model"; import { fontSize, radius } from "./scale.stylex"; import type { StyleArg } from "./stylex-props"; import { tokens } from "./tokens.stylex"; -import { IconButton } from "./ui/button"; +import { Button, IconButton } from "./ui/button"; import { Chip } from "./ui/chip"; import { StatusBanner } from "./ui/status-banner"; import { surfaceStyles } from "./ui/surface"; @@ -110,6 +119,8 @@ export function AskPanelContent({ const [threadId, setThreadId] = useState(null); const [sending, setSending] = useState(false); const [requestError, setRequestError] = useState(null); + const [again, setAgain] = useState<() => void>(); + const down = useReviewHostDown(session.review); // Choices for a question not yet asked; a thread says its own. const [picks, setPicks] = useState({}); const [bypassPick, setBypassPick] = useState(); @@ -234,6 +245,7 @@ export function AskPanelContent({ const canAsk = !session.review?.trashedAt && Boolean(agent) && + !down && (!busy || (answering && !sending)); const post = useCallback( @@ -255,6 +267,11 @@ export function AskPanelContent({ [session], ); + const fail = useCallback((message: string, rerun: () => void) => { + setRequestError(message); + setAgain(() => rerun); + }, []); + const ask = async (question: AskQuestion) => { if (!agent || !canAsk || thread?.status === "failed") return false; setSending(true); @@ -291,7 +308,10 @@ export function AskPanelContent({ return true; } catch (error) { - setRequestError(error instanceof Error ? error.message : String(error)); + // The question is back in the composer; sending it again sends that. + fail(error instanceof Error ? error.message : String(error), () => + composer.current?.form?.requestSubmit(), + ); return false; } finally { @@ -319,17 +339,20 @@ export function AskPanelContent({ ); const decide = useCallback( - (permissionId: string, optionId: string) => + function decide(permissionId: string, optionId: string) { void post(`/ask/${threadId}/permission`, { permissionId, optionId, - }).catch((error: Error) => setRequestError(error.message)), - [post, threadId], + }).catch((error: Error) => + fail(error.message, () => decide(permissionId, optionId)), + ); + }, + [post, threadId, fail], ); const stop = () => void post(`/ask/${threadId}/cancel`).catch((error: Error) => - setRequestError(error.message), + fail(error.message, stop), ); const chosen = agents?.find((candidate) => candidate.id === agent); @@ -374,7 +397,7 @@ export function AskPanelContent({ } void post(`/ask/${threadId}/choice`, { kind, value }).catch( - (error: Error) => setRequestError(error.message), + (error: Error) => fail(error.message, () => choose(kind, value)), ); }; @@ -397,7 +420,7 @@ export function AskPanelContent({ } void post(`/ask/${threadId}/permissions`, { bypass: value }).catch( - (error: Error) => setRequestError(error.message), + (error: Error) => fail(error.message, () => permit(value)), ); }; @@ -407,7 +430,7 @@ export function AskPanelContent({ requestError ?? thread?.error ?? (lost - ? `Whiteboard lost its connection to ${thread?.agentName ?? chosenName}.` + ? `Whiteboard lost its connection to ${session.review?.host ?? thread?.agentName ?? chosenName}.` : null); // Saved once its agent started a session; before that, nothing reopens it. @@ -454,6 +477,22 @@ export function AskPanelContent({ }) : undefined; + const errorAction = reconnect + ? { label: "Reconnect", run: reconnect } + : retry + ? { label: "Try again", run: retry } + : startOver + ? { label: "Start a new conversation", run: startOver } + : requestError !== null && again && !down + ? { + label: "Try again", + run: () => { + setRequestError(null); + again(); + }, + } + : undefined; + // Reopening starts the agent and loads its session. Whiteboard's saved // copy shows meanwhile; one saved before that copy waits for the replay. const connecting = @@ -594,6 +633,7 @@ export function AskPanelContent({ agents={agents} agent={agent} locked={threadId !== null || savedThreadId !== undefined} + unavailable={down?.label} onPick={(picked) => { setAgent(picked); setPicks({}); @@ -605,6 +645,13 @@ export function AskPanelContent({ return (
{header ? createPortal(agentPicker, header) : agentPicker} + {down ? ( + + ) : null}
@@ -651,19 +698,15 @@ export function AskPanelContent({ ) : error ? (

{error} - {reconnect || retry || startOver ? ( + {errorAction ? ( <> {" "} ) : null} @@ -685,7 +728,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 +822,34 @@ export function AskReadOnlyThread({ ); } +function AskHostBanner({ + status, + host, + hosts, +}: { + status: ReviewHostStatus; + host?: string; + hosts?: ReviewRemoteHostActions; +}): ReactElement { + const { run, pending, failure } = useHostAction(hosts, host); + const { action } = status; + const why = failure ?? status.sentence; + + return ( + run(action)}> + {ACTION_WORDS[action]} + + ) : null + } + > + {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/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 From 8c90e7a915c1543e2359dcdd0bb9e67ec2910b37 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 7 Oct 2026 08:44:34 -0400 Subject: [PATCH 04/10] Say why SSH could not reach a host in a plain sentence Common OpenSSH failures read as one sentence naming the host: refused, unknown host name, timed out, key not accepted, host key changed, and a connection that stopped answering. Other failures keep OpenSSH's words. The raw output stays in the log. Agent-Session: 7633c50b-998d-4b1e-8ca2-ffb350a26066 Agent-Session: 9a4d92ed-7a0d-42a1-9169-574bca60095b Agent-Session: 1c093463-922b-435b-a925-664d5d5c3c0a Agent-Session: 2b51b153-90b5-4a72-a052-fccf0beb9f1c --- .../remote/reviewRemoteHost.test.ts | 6 ++-- .../electron-main/remote/reviewRemoteHost.ts | 29 +++++++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) 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), }); } From 3c3e810fc25ff7464909b77896ff1eeffe4d97f5 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 7 Oct 2026 10:55:55 -0400 Subject: [PATCH 05/10] Keep Source tree on remote reviews, disabled with the reason The gateway gives a remote review's availability a reason: the host's sentence while it is down, or why its VS Code server is unavailable. Source tree stays in the top bar and is disabled with that reason as its tooltip, instead of disappearing. Missing language extensions do not disable it. --- .../scripts/e2e/journeys/remote-host.mjs | 4 +- .../review-protocol/src/review-api-client.ts | 6 ++- packages/review/app/src/App.tsx | 16 +++++-- packages/review/app/src/api-canvas.test.tsx | 16 ++++++- .../src/server/review-gateway-list.test.ts | 47 ++++++++++++++++++- .../review/src/server/review-gateway-list.ts | 25 ++++++++-- 6 files changed, 102 insertions(+), 12 deletions(-) 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/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 3db189608..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,11 +623,12 @@ function ReviewLayoutContent({ shellStyles.topbarContext, )} > - {!scratchpad && sourceWindows && ( + {!scratchpad && sourceTreeShown && ( - ) : null - } - > + {host && why.includes(host) ? why : `${status.label} · ${why}`} ); 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/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 From 176c2505e5f8f51a21f848384d680db4e6bd805b Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 10:21:51 -0400 Subject: [PATCH 07/10] Match the Source window's offline label in the journey --- .../scripts/e2e/journeys/remote-source-window.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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(); From 85450881e5076615ad66f821a6fae48f1d776591 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 10:39:49 -0400 Subject: [PATCH 08/10] Show a restored Source window's host offline before its alias attaches --- .../electron-main/remote/reviewRemoteHosts.test.ts | 10 ++++++++++ .../review/electron-main/remote/reviewRemoteHosts.ts | 7 ++++--- .../vs/review/electron-main/reviewDesktopChannel.ts | 5 +++-- .../src/vs/review/electron-main/reviewDesktopHost.ts | 4 ++-- .../services/remote/reviewSourceWindowHostState.ts | 2 +- .../services/remote/reviewWindowAuthorityResolver.ts | 4 ++-- 6 files changed, 22 insertions(+), 10 deletions(-) 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..e60de2571 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 the machine `serverId`, through the alias in the setting that served it this session, or `last` while that alias has not attached. */ + 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.ts b/apps/review-desktop/code-oss/src/vs/review/services/remote/reviewSourceWindowHostState.ts index b2c2ec88b..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 @@ -92,7 +92,7 @@ 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); 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]), }; } From e2b64dedcca4d1fd39ea14b3ac9fb5c001ed171f Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 14:12:26 -0400 Subject: [PATCH 09/10] Let the gateway's 503 say a source's host is down The gateway answers a down host's review at once with 503 and its alias, so the navigator no longer checks the catalog first. --- .../parts/canvas/reviewApiEditorSerializer.test.ts | 5 ++--- .../services/reviewCanvasEditorTabsService.test.ts | 12 +----------- .../review/services/reviewCanvasEditorTabsService.ts | 4 ---- .../services/reviewEditorResolverService.test.ts | 1 - 4 files changed, 3 insertions(+), 19 deletions(-) diff --git a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts index 550a7d54b..8c1635fa0 100644 --- a/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/browser/parts/canvas/reviewApiEditorSerializer.test.ts @@ -28,7 +28,6 @@ test("opening historical source requests its version and opens a separate native { warn() {} } as never, {} as never, {} as never, - { reviews: [] } as never, ); t.after(() => tabs.dispose()); await tabs.openApiSource({ reviewId: "review-a", kind: "version", version: 7 }, "Historical Review"); @@ -64,7 +63,7 @@ test("native group restoration preserves both reviews, order and pinned source v }; const groupService = { groups, mainPart: { activeGroup: undefined as EditorGroupModel | undefined } }; const createTabs = () => - new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, { reviews: [] } as never); + new ReviewCanvasEditorTabsService(instantiation as never, editors as never, groupService as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never); tabs = createTabs(); const registry = Registry.as(EditorExtensions.EditorFactory); registry.start({ get: () => instantiation } as never); @@ -131,7 +130,7 @@ test("current Source tabs retain identity and main version tabs still restore", }, invokeFunction(fn: (accessor: { get(): ReviewCanvasEditorTabsService }) => unknown) { return fn({ get: () => tabs }); }, }; - tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never, { reviews: [] } as never); + tabs = new ReviewCanvasEditorTabsService(instantiation as never, { onDidCloseEditor: Event.None } as never, {} as never, {} as never, {} as never, { warn() {} } as never, {} as never, {} as never); try { const serializer = new ReviewApiEditorSerializer(); const restored = serializer.deserialize(instantiation as never, JSON.stringify({ kind: "api-source", reviewId: "a", title: "A", selection: { reviewId: "a", kind: "current" } })); 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 a207e1d20..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 @@ -40,7 +40,6 @@ async function closeWelcome(updateNeeded: boolean): Promise { { warn() {} } as never, {} as never, {} as never, - { reviews: [] } as never, ); try { const welcome = await tabs.openWelcome(true); @@ -61,7 +60,7 @@ const HOST = "whiteboard+c0ffee"; const remote = (path: string) => URI.from({ scheme: "vscode-remote", authority: HOST, path }).toString(); const view = (generation?: string) => ({ reviewId: "r1", version: 2, generation }); -function sourceTabs(t: TestContext, answer: (url: URL) => object, reviews: object[] = []) { +function sourceTabs(t: TestContext, answer: (url: URL) => object) { const opened: { toOpen: { workspaceUri?: URI; fileUri?: URI; label?: string }[]; options: Record }[] = []; const written: string[] = []; t.mock.method(globalThis, "fetch", async (url: string) => Response.json(answer(new URL(url)))); @@ -74,7 +73,6 @@ function sourceTabs(t: TestContext, answer: (url: URL) => object, reviews: objec { warn() {} } as never, { async writeFile(resource: URI, content: VSBuffer) { written.push(`${resource.toString()}=${content.toString()}`); } } as never, { cacheHome: URI.file("/laptop/cache") } as never, - { reviews } as never, ); t.after(() => tabs.dispose()); return { tabs, opened, written }; @@ -116,14 +114,6 @@ test("a remote diff opens its empty side from a laptop empty file", async (t) => assert.equal(options.diffMode, true); }); -test("an offline host's review fails at once with its host's words and asks nothing", async (t) => { - const { tabs } = sourceTabs(t, () => remoteAnswer, [{ reviewId: "r1", host: "devbox", hostState: "offline" }]); - const fetch = globalThis.fetch as unknown as { mock: { callCount(): number } }; - await assert.rejects(tabs.openApiSource({ reviewId: "r1", kind: "current" }, "Fix the parser"), (error: unknown) => - error instanceof ReviewHostDown && error.host.alias === "devbox" && error.message === "devbox offline."); - assert.equal(fetch.mock.callCount(), 0); -}); - 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" } })); 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 5984d0778..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 @@ -29,7 +29,6 @@ import { reviewSourceQuery, type ReviewRemoteNavigatorAnswer, type ReviewSourceS import { REVIEW_LANGUAGE_SOURCE_SCHEME } from "../common/reviewReadonlySource.js"; import { sourceLocation, sourceSelectionIdentity, REVIEW_API_SOURCE_SCHEME } from "../common/reviewSourceView.js"; import { reviewRemoteAuthority } from "./remote/reviewRemoteAuthority.js"; -import { IReviewApiCatalogService } from "./reviewApiCatalogService.js"; import { IReviewDesktopConnectionService, reviewResponseError } from "./reviewDesktopConnectionService.js"; type NavigatorAnswer = { workspacePath: string; filePath?: string } | ReviewRemoteNavigatorAnswer; @@ -78,7 +77,6 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview @ILogService private readonly logService: ILogService, @IFileService private readonly files: IFileService, @IEnvironmentService private readonly environment: IEnvironmentService, - @IReviewApiCatalogService private readonly catalog: IReviewApiCatalogService, ) { super(); this._register( @@ -183,8 +181,6 @@ export class ReviewCanvasEditorTabsService extends Disposable implements IReview } private async navigatorWorkspace(reviewId: string, values: Record): Promise { - const review = this.catalog.reviews.find(review => review.reviewId === reviewId); - if (review?.host && review.hostState && review.hostState !== "online") throw new ReviewHostDown({ alias: review.host, state: review.hostState }); const { serverUrl, token } = await this.desktopConnection.getConnection(); const query = new URLSearchParams(Object.entries(values).filter(([key, value]) => key !== "reviewId" && value !== undefined).map(([key, value]) => [key, String(value)])); const response = await fetch(`${serverUrl}/reviews-api/${encodeURIComponent(reviewId)}/navigator${query.size ? `?${query}` : ""}`, { diff --git a/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts b/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts index f902bc5ee..0139f6443 100644 --- a/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/services/reviewEditorResolverService.test.ts @@ -30,7 +30,6 @@ test("source tree, selected code, definitions and diffs hand off before creating { warn() {} } as never, {} as never, {} as never, - { reviews: [] } as never, ); const resolver = new ReviewEditorResolverService( { get activeGroup() { throw new Error("Review must not create an editor group"); } } as never, From 13f0aefc22e0e3563713852c62e495c660443596 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 16:06:40 -0400 Subject: [PATCH 10/10] Shorten a host state comment --- .../src/vs/review/electron-main/remote/reviewRemoteHosts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e60de2571..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,7 +226,7 @@ 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, or `last` while that alias has not attached. */ + /** 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);