Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
7a4253b
Ask on remote reviews
thesiti92 Oct 7, 2026
5de3fd1
Start the remote server without waiting on the login shell
thesiti92 Oct 14, 2026
7a95e25
Share one fake Ask agent across server tests
thesiti92 Oct 14, 2026
5819baa
Drop Ask tests that other suites already cover
thesiti92 Oct 14, 2026
7867b88
Keep only the Ask tests on what this change adds
thesiti92 Oct 14, 2026
f365a6a
Trim Ask comments
thesiti92 Oct 14, 2026
aa95265
Say a laptop connects to a host in the Ask docs
thesiti92 Oct 14, 2026
912b613
Resolve and configure Source windows for remote hosts
thesiti92 Oct 7, 2026
87b3743
Pass whole host entries to Source windows and trust main's answers
thesiti92 Oct 14, 2026
4811602
Drop wall-clock checks from the window authority tests
thesiti92 Oct 14, 2026
e2156f7
Drop Source window tests a sibling test already covers
thesiti92 Oct 14, 2026
8a15c5e
Open Source windows for remote reviews
thesiti92 Oct 7, 2026
94d941d
Answer a remote caller's empty navigator side with emptySide
thesiti92 Oct 14, 2026
124ca22
Forward the remote navigator's host paths and build URIs in the fork
thesiti92 Oct 14, 2026
058e9a5
Wait on host state changes, not fixed sleeps, in the Source window test
thesiti92 Oct 14, 2026
4147ab3
Serve empty extension channels instead of proxying them in Source win…
thesiti92 Oct 14, 2026
c074a07
Drop duplicate Source window tests and trim comments
thesiti92 Oct 14, 2026
3708ab5
Shorten the Source windows docs
thesiti92 Oct 14, 2026
0c7843e
Let a review client reopen its shared stream at once
thesiti92 Oct 6, 2026
c7e9fe6
Word each remote host state once, with its next step
thesiti92 Oct 6, 2026
679c72d
Show a remote host's state and next step on its reviews
thesiti92 Oct 6, 2026
7b19bbc
Offer Retry to offline hosts in Settings, and show its result
thesiti92 Oct 6, 2026
31991f8
Reuse the host action hook and status wording in Settings
thesiti92 Oct 14, 2026
0fef570
Drop checks for the removed Connection lost banner
thesiti92 Oct 14, 2026
25e2a36
Show a Source window's host state, with Retry while offline
thesiti92 Oct 6, 2026
e394a6a
Fail at once when opening source on a host that is down
thesiti92 Oct 6, 2026
f65423b
Block Ask while a review's host is down
thesiti92 Oct 6, 2026
260a973
Say why SSH could not reach a host in a plain sentence
thesiti92 Oct 7, 2026
3b91de3
Keep Source tree on remote reviews, disabled with the reason
thesiti92 Oct 7, 2026
f9865ff
Name the host when a remote review's checkout is gone
thesiti92 Oct 7, 2026
9a16cbd
Match the Source window's offline label in the journey
thesiti92 Oct 14, 2026
a93bb24
Show a restored Source window's host offline before its alias attaches
thesiti92 Oct 14, 2026
3a90432
Let the gateway's 503 say a source's host is down
thesiti92 Oct 14, 2026
ed83e74
Shorten a host state comment
thesiti92 Oct 14, 2026
691bac1
Update Whiteboard on a host forward without asking when it already ha…
thesiti92 Oct 7, 2026
4ed5b85
Give a dev Desktop a +dev.<checkout state> version, so remote hosts u…
thesiti92 Oct 7, 2026
43a2103
Update only Desktop's own installs without asking
thesiti92 Oct 14, 2026
68dfcf7
Drop the dev Desktop version stamp
thesiti92 Oct 14, 2026
a1824fb
Match the install docs to updating only Desktop's own installs
thesiti92 Oct 14, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/review-desktop/UPSTREAM
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,10 @@ one of these entries:
fatal either: it ends that connection only. Isolation is unconditional, so a
remote Source window gets no upstream reload dialog; it reconnects through
the fork's resolver and shows its host state in the status bar.
- `src/vs/platform/window/common/window.ts` and
`src/vs/platform/native/electron-main/nativeHostMainService.ts`: the
`openWindow` option `reviewReferencesToShow` runs Find References in the
opened window.
- `extensions/package.json` and the extension `package.json`/`package-lock.json`
files: `@vscode/extension-telemetry` is hoisted to the shared extensions
install (github keeps its own ^1.0.0 copy; the shared copy is ^0.9.8). The
Expand Down Expand Up @@ -614,7 +618,8 @@ one of these entries:
profile, extension host status, lifetime and extensions scanner services,
and the `logger`, `userDataProfiles`, `remoteextensionsenvironment`,
`remoteExtensionsScanner`, `remoteFilesystem` and extension host debug
broadcast channels. `telemetry` is a no-op channel and `ITelemetryService`
broadcast channels. `telemetry` is a no-op channel, `extensions` and
`extensionGalleryManifest` list no extensions, and `ITelemetryService`
is `NullTelemetryService`. Cuts requests, telemetry appenders and machine
ids, the gallery and extension installs, language packs, terminals, the
agent host, MCP and the sandbox helper.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ test("opening historical source requests its version and opens a separate native
{ async getConnection() { return { serverUrl: "http://localhost", token: "test" }; } } as never,
{ async openWindow(...args: unknown[]) { opened.push(args); } } as never,
{ warn() {} } as never,
{} as never,
{} as never,
);
t.after(() => tabs.dispose());
await tabs.openApiSource({ reviewId: "review-a", kind: "version", version: 7 }, "Historical Review");
assert.equal(requests[0], "http://localhost/reviews-api/review-a/navigator?version=7");
assert.equal(opened.length, 1);
assert.deepEqual(opened[0]![1], { forceNewWindow: true });
assert.equal((opened[0]![0] as { workspaceUri: { fsPath: string } }[])[0].workspaceUri.fsPath, "/pinned/review.code-workspace");
assert.equal((opened[0]![1] as { remoteAuthority?: string }).remoteAuthority, undefined);
ok = false;
await assert.rejects(tabs.openApiSource({ reviewId: "review-a", kind: "current" }, "Review"));
assert.equal(opened.length, 1);
Expand Down Expand Up @@ -60,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);
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<IEditorFactoryRegistry>(EditorExtensions.EditorFactory);
registry.start({ get: () => instantiation } as never);
Expand Down Expand Up @@ -127,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);
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" } }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@ import type {
ReviewDocumentWidthChoice,
ReviewKeymapChoice,
ReviewReadyNotificationChoice,
ReviewRemoteHostActions,
ReviewRuntimeConfig,
ReviewSurfaceEvent,
ReviewTheme,
ReviewVerbResponse,
TutorialProgressV1,
TutorialStepId,
} from "../../../common/reviewProtocol.js";
Expand All @@ -86,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";
Expand Down Expand Up @@ -459,6 +462,7 @@ export class ReviewCanvasEditorPane extends EditorPane {
kind: "api",
reviewId,
...remoteEntry(this.apiCatalog.reviews.find((review) => review.reviewId === reviewId)),
remoteHosts: this.remoteHostActions(),
structuralDiffEnabled: this.currentStructuralDiffEnabled(),
softwareMapEnabled: this.currentSoftwareMapEnabled(),
documentWidth: this.currentDocumentWidth(),
Expand Down Expand Up @@ -487,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);
},
Expand Down Expand Up @@ -574,11 +576,11 @@ export class ReviewCanvasEditorPane extends EditorPane {
this.reviewTelemetryService.capture("review_restored", { via: "home" });
return this.apiCatalog.lifecycle(uuid, "active");
},
hostStates: () => this.desktopConnection.readRemoteHosts(),
remoteHosts: this.remoteHostActions(),
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;
}
},
Expand Down Expand Up @@ -642,6 +644,32 @@ export class ReviewCanvasEditorPane extends EditorPane {
}
}

private notifyOpenFailure(error: unknown): void {
notifyOpenFailure(this.notificationService, (alias) => this.desktopConnection.retryRemoteHost(alias), error);
}

/** Answers the canvas instead of rejecting, once the failure is shown. */
private async openSource(open: () => Promise<void>): Promise<ReviewVerbResponse> {
try {
await open();
return { ok: true };
} catch (error) {
this.notifyOpenFailure(error);
return { ok: false, error: error instanceof Error ? error.message : String(error) };
}
}

private remoteHostActions(): ReviewRemoteHostActions {
return {
states: () => this.desktopConnection.readRemoteHosts(),
retry: (alias) => this.desktopConnection.retryRemoteHost(alias),
install: (alias) => this.desktopConnection.installRemoteHost(alias),
openSettings: async () => {
await this.tabsService.openSettings(true);
},
};
}

async refreshComparison(): Promise<void> {
if (!(this.input instanceof ReviewCanvasEditorInput) || this.input.target.kind !== "api") return;
this.readyInput = undefined;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) dev.fast. All rights reserved.
* Licensed under the MIT License. See LICENSE in the repository root for license information.
*--------------------------------------------------------------------------------------------*/

import Severity from "../../../../base/common/severity.js";
import type { INotificationService } from "../../../../platform/notification/common/notification.js";
import { reviewHostStatus, type ReviewGatewayHostState } from "../../../common/reviewProtocol.js";

type HostDown = Pick<ReviewGatewayHostState, "alias" | "state" | "detail">;

export class ReviewHostDown extends Error {
constructor(readonly host: HostDown) {
super(reviewHostStatus(host).sentence);
}
}

export function notifyOpenFailure(notifications: INotificationService, retry: (alias: string) => Promise<void>, error: unknown): void {
if (!(error instanceof ReviewHostDown)) {
notifications.error(error instanceof Error ? error : String(error));
return;
}
const { alias } = error.host;
const choices = reviewHostStatus(error.host).action === "retry"
? [{ label: "Retry", run: () => void retry(alias).catch(retryError => notifications.error(retryError)) }]
: [];
notifications.prompt(Severity.Warning, error.message, choices);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import test from "node:test";
import type { ReviewApiSummary } from "../../../common/reviewProtocol.js";
import { remoteEntry, withRemoteEntry } from "./reviewRemoteEntry.js";

const available = { sourceWindows: false, languageFeatures: false };
const available = { sourceWindows: false };
const entry = (host?: string) => ({ reviewId: "r1", title: "Remote", ...(host && { host, available }) }) as ReviewApiSummary;

test("a review a remote opened before its list entry arrived takes the host once the entry does", () => {
Expand All @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type { ReviewApiSummary } from "../../../common/reviewProtocol.js";

type RemoteFields = Pick<ReviewApiSummary, "host" | "available">;
type RemoteFields = Pick<ReviewApiSummary, "host" | "hostState" | "available">;

export const remoteEntry = (review: ReviewApiSummary | undefined): RemoteFields =>
review?.host ? { host: review.host, available: review.available } : {};
review?.host ? { host: review.host, ...(review.hostState && { hostState: review.hostState }), available: review.available } : {};

export function withRemoteEntry<T extends RemoteFields>(content: T, review: ReviewApiSummary | undefined): T | undefined {
const entry = remoteEntry(review);
if (!entry.host || (entry.host === content.host && JSON.stringify(entry.available) === JSON.stringify(content.available))) return undefined;
if (!entry.host || (entry.host === content.host && entry.hostState === content.hostState && JSON.stringify(entry.available) === JSON.stringify(content.available))) return undefined;
return { ...content, ...entry };
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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);
Expand Down Expand Up @@ -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);
});

Expand Down Expand Up @@ -724,18 +724,17 @@ test("the version present: no prompt and no install shown; the installed CLI att
);
});

test("the version present with another sha256 is not installed: the user is asked and the install is shown", async (t) => {
test("the version present with another sha256 is reinstalled without asking, and the install is shown", async (t) => {
const port = await healthServer(t);
const { flow, prompts, runs } = await installFlow(t, "ask", { answers: [true], steps: STEPS });
const { flow, prompts, runs } = await installFlow(t, "ask", { steps: STEPS });
const { host, reports, last } = hostFor(t, { probe: { installed: [at("0.1.5"), at("0.1.6", "b".repeat(64))] } }, port, "wb-test-a", "/tmp/wb-ssh-test", flow);

host.start();
await until(() => last()?.endpoint !== undefined);

assert.equal(prompts.length, 1);
assert.equal(prompts.length, 0);
assert.equal(runs.length, 1);
assert.deepEqual(reports.find((report) => report.installing)?.installing, { step: "preparing" });
assert.equal(await flow.consent.get("wb-test-a"), "allow");
});

test("the version absent with installs always: each step is reported, then the host attaches", async (t) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,28 @@ export function classifySshFailure(stderr: string, promptCancelled: boolean): "a
: "unreachable";
}

const SSH_SENTENCES: [RegExp, (alias: string) => string][] = [
[/REMOTE HOST IDENTIFICATION HAS CHANGED/, (alias) => `${alias}'s host key changed since you last connected. Verify the new fingerprint before replacing its old key in known_hosts.`],
[/Host key verification failed/, (alias) => `${alias}'s host key could not be verified. Run "ssh ${alias}" in a terminal and verify its fingerprint before accepting it.`],
[/Permission denied/, (alias) => `${alias} did not accept your SSH key. Check that "ssh ${alias}" works in a terminal.`],
[/Could not resolve hostname/, (alias) => `${alias}'s host name could not be found. Check its address in your SSH config.`],
[/Connection refused/, (alias) => `${alias} refused the SSH connection. Check that its SSH server is running.`],
[/(Operation|Connection) timed out/, (alias) => `The SSH connection to ${alias} timed out. Check that it is on and reachable from this network.`],
];

function sshFailureSentence(stderr: string, alias: string): string | undefined {
return SSH_SENTENCES.find(([pattern]) => pattern.test(stderr))?.[1](alias);
}

const SSH_DROPS: [RegExp, (alias: string) => string][] = [
[/Timeout, server .* not responding/, (alias) => `${alias} stopped answering over SSH. It may be asleep, or off this network.`],
[/Connection reset|Broken pipe|closed by remote host/, (alias) => `${alias} closed the SSH connection.`],
];

function sshDropSentence(lastLines: string, alias: string): string | undefined {
return SSH_DROPS.find(([pattern]) => pattern.test(lastLines))?.[1](alias);
}

export const OPENSSH_NEEDED = "OpenSSH is needed: no `ssh` command was found on PATH.";

type Problem = NonNullable<ReviewGatewayHost["problem"]>;
Expand Down Expand Up @@ -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),
});
}

Expand Down Expand Up @@ -613,7 +638,9 @@ export class ReviewRemoteHost {
if (artifact instanceof ReviewRemoteNoTarball) return this.cannotInstall(probe, version, noTarballText(this.alias));
if (artifact instanceof Error) return this.cannotInstall(probe, version, installFailureText(this.alias, version, "preparing", artifact.message));
if (!present && !probe.releasesReachable) return this.cannotInstall(probe, version, `${this.alias} cannot reach ${origin}.`);
if (!present && mode === "ask" && !(await this.agreed(flow, probe, version, stale))) {
// Like VS Code's server, Desktop updates its own installs without asking; a CLI the user installed is theirs.
const upgrade = probe.installed.length > 0;
if (!present && !upgrade && mode === "ask" && !(await this.agreed(flow, probe, version, stale))) {
this.declined = true;
return stale() ? undefined : onPath;
}
Expand Down
Loading
Loading