Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .github/workflows/review-desktop-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ jobs:
- name: Lint
run: pnpm lint

- name: Check the remote install script
run: shellcheck -s sh packages/review/remote/install.sh

- name: Check formatting
run: pnpm format:check

Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/review-desktop-preview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ jobs:
for t in linux-x64 linux-arm64; do
node apps/review-desktop/scripts/build-remote-tarball.mjs --target "$t" --out remote-artifacts --version "$VERSION" --commit "$COMMIT"
done
cp packages/review/remote/install.sh remote-artifacts/install.sh
# Payloads before the pin: no Desktop build may pin a tarball R2 lacks.
- name: Upload the remote tarballs to R2
if: ${{ !inputs.dry_run && steps.existing.outputs.published != 'true' }}
Expand All @@ -218,6 +219,7 @@ jobs:
PREFIX="s3://${R2_BUCKET}/releases/${VERSION}/remote/"
aws s3 cp remote-artifacts/ "$PREFIX" --recursive --exclude "*" --include "*.tar.xz" --content-type application/x-xz
aws s3 cp remote-artifacts/ "$PREFIX" --recursive --exclude "*" --include "*.sha256" --content-type text/plain
aws s3 cp remote-artifacts/install.sh "${PREFIX}install.sh" --content-type text/plain
- name: Pin the remote tarballs
id: remote
run: |
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/review-desktop-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -378,14 +378,15 @@ jobs:
for t in linux-x64 linux-arm64; do
node apps/review-desktop/scripts/build-remote-tarball.mjs --target "$t" --out remote-artifacts --version "$VERSION" --commit "$COMMIT"
done
cp packages/review/remote/install.sh remote-artifacts/install.sh
# Release assets, then R2, then the pin: a rerun that finds the tarballs on
# R2 skips both uploads, and no Desktop build may pin a tarball R2 lacks.
- name: Attach the remote tarballs to the GitHub Release
if: ${{ !inputs.dry_run && steps.existing.outputs.published != 'true' }}
env:
GH_TOKEN: ${{ github.token }}
TAG: ${{ needs.version.outputs.tag }}
run: gh release upload "$TAG" --repo "$GITHUB_REPOSITORY" --clobber remote-artifacts/*.tar.xz remote-artifacts/*.sha256
run: gh release upload "$TAG" --repo "$GITHUB_REPOSITORY" --clobber remote-artifacts/*.tar.xz remote-artifacts/*.sha256 remote-artifacts/install.sh
- name: Upload the remote tarballs to R2
if: ${{ !inputs.dry_run && steps.existing.outputs.published != 'true' }}
env:
Expand All @@ -398,6 +399,7 @@ jobs:
PREFIX="s3://${R2_BUCKET}/releases/${VERSION}/remote/"
aws s3 cp remote-artifacts/ "$PREFIX" --recursive --exclude "*" --include "*.tar.xz" --content-type application/x-xz
aws s3 cp remote-artifacts/ "$PREFIX" --recursive --exclude "*" --include "*.sha256" --content-type text/plain
aws s3 cp remote-artifacts/install.sh "${PREFIX}install.sh" --content-type text/plain
- name: Pin the remote tarballs
id: remote
run: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import test from "node:test";

import type { ReviewGatewayHost } from "../../common/reviewProtocol.js";
import { attachOutput, COMMIT, detectOutput, FAKE_SERVER_ID, fakeClock, fakeSsh, flush, healthServer, until, type FakeRemote } from "./test/fakeSsh.js";
import { INSTALL_SCRIPT } from "./test/installScript.js";
import { classifySshFailure, languageCommitMismatch, ReviewRemoteHost, type ReviewRemoteHostOptions, type ReviewRemoteInstallFlow, type ReviewRemoteInstallMode, type ReviewRemoteInstallRunInput } from "./reviewRemoteHost.js";
import { ReviewRemoteNoTarball } from "./reviewRemoteArtifacts.js";
import { openRemoteInstallConsent } from "./reviewRemoteInstallConsent.js";
Expand Down Expand Up @@ -102,6 +103,7 @@ async function installFlow(
if (error) throw error;
return INSTALLED;
},
script: async () => INSTALL_SCRIPT,
origin: () => (options.origin === null ? undefined : (options.origin ?? "https://install.dev.fast")),
artifact:
options.artifact ??
Expand Down Expand Up @@ -724,18 +726,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 Expand Up @@ -774,7 +775,6 @@ test("the version absent, asked and declined: not-installed and declined, and no
alias: "wb-test-a",
problem: { state: "not-installed", detail: "Whiteboard is not installed on wb-test-a. Install Whiteboard 0.1.6 there." },
declined: true,
target: "linux-arm64",
});
assert.equal(clock.pending, 0);
assert.deepEqual(JSON.parse(await readFile(consentFile, "utf8")).aliases, { "wb-test-a": "deny" });
Expand Down Expand Up @@ -983,7 +983,6 @@ test("a server of another version a user started is left, and the host is incomp
state: "incompatible",
detail: "A Whiteboard server 0.1.5 started by a user is running on wb-test-a; stop it to use this Desktop's version.",
},
target: "linux-arm64",
});
assert.equal(clock.pending, 0);
assert.equal(ssh.of("wb-test-a", "exec").length, 1);
Expand Down Expand Up @@ -1069,7 +1068,6 @@ test("a failed upgrade attaches the older version, which reports why, and Retry
endpoint: { url: `http://127.0.0.1:${port}`, token: "remote-token" },
...NO_SERVER,
installFailure: "Installing Whiteboard 0.1.6 on wb-test-a failed while installing the package: The package installed on wb-test-a reports version 0.1.5, not 0.1.6.",
target: "linux-arm64",
});
assert.match(ssh.of("wb-test-a", "exec")[0].input!, /command -v whiteboard/);
assert.equal(clock.pending, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export interface RunResult {
readonly timedOut: boolean;
}

export function runSsh(spawn: SpawnSsh, env: NodeJS.ProcessEnv, args: string[], timeout: number, input?: string): Promise<RunResult> {
export function runSsh(spawn: SpawnSsh, env: NodeJS.ProcessEnv, args: string[], timeout: number, input?: string, onStdout?: (chunk: string) => void): Promise<RunResult> {
return new Promise((resolve) => {
let child: SshChildProcess;
try {
Expand All @@ -215,7 +215,10 @@ export function runSsh(spawn: SpawnSsh, env: NodeJS.ProcessEnv, args: string[],
done({ code: null });
}, timeout);
child.stdout?.setEncoding("utf8");
child.stdout?.on("data", (chunk: string) => (stdout = (stdout + chunk).slice(-OUTPUT_LIMIT)));
child.stdout?.on("data", (chunk: string) => {
stdout = (stdout + chunk).slice(-OUTPUT_LIMIT);
onStdout?.(chunk);
});
child.stderr?.setEncoding("utf8");
child.stderr?.on("data", (chunk: string) => (stderr = (stderr + chunk).slice(0, OUTPUT_LIMIT)));
child.once("error", (error: NodeJS.ErrnoException) => done({ code: null, error }));
Expand All @@ -229,7 +232,7 @@ export function runSsh(spawn: SpawnSsh, env: NodeJS.ProcessEnv, args: string[],

export type ReviewRemoteInstallMode = "ask" | "always" | "never";

export type ReviewRemoteInstallRunInput = Pick<ReviewRemoteInstallInput, "session" | "probe" | "target" | "version" | "onProgress" | "signal" | "spawn" | "env" | "artifact">;
export type ReviewRemoteInstallRunInput = Pick<ReviewRemoteInstallInput, "session" | "probe" | "target" | "version" | "onProgress" | "signal" | "spawn" | "env" | "artifact" | "script">;

export interface ReviewRemoteInstallFlow {
mode(): ReviewRemoteInstallMode;
Expand All @@ -239,6 +242,8 @@ export interface ReviewRemoteInstallFlow {
run(input: ReviewRemoteInstallRunInput): Promise<ReviewRemoteInstallResult>;
origin(): string | undefined;
artifact(target: ReviewRemoteTarget, version: string): Promise<ReviewRemoteArtifact>;
/** packages/review/remote/install.sh. */
script(): Promise<string>;
}

type InstallStep = NonNullable<ReviewGatewayHost["installing"]>["step"];
Expand Down Expand Up @@ -315,7 +320,6 @@ export class ReviewRemoteHost {
private detecting: Promise<ReviewRemoteAgent[] | undefined> | undefined;
private declined = false;
private installFailure: string | undefined;
private target: ReviewRemoteTarget | undefined;
private installing: AbortController | undefined;
private removing = false;
serverId: string | undefined;
Expand Down Expand Up @@ -614,13 +618,13 @@ export class ReviewRemoteHost {
const mode = flow?.mode() ?? "never";
if (!flow || mode === "never") return onPath;
const origin = flow.origin();
const probed = await probeRemote({ session: this.options.session, spawn: this.options.spawn, env, releases: origin && `${origin}/` });
const script = await flow.script();
const probed = await probeRemote({ session: this.options.session, spawn: this.options.spawn, env, script, releases: origin && `${origin}/` });
if (stale()) return;
if ("error" in probed) throw unreachable(probed.error);
const { probe } = probed;
const support = judgeRemote(probe);
if (!support.supported) throw new HostFailure({ state: "unsupported", detail: support.reason });
this.target = support.target;
const version = await this.options.desktopVersion();
const artifact = await flow.artifact(support.target, version).catch((error: Error) => error);
if (stale()) return;
Expand All @@ -638,7 +642,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, an install Desktop recognises is updated without asking; a CLI the user put on PATH 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 Expand Up @@ -691,6 +697,7 @@ export class ReviewRemoteHost {
target,
version,
artifact,
script: await flow.script(),
spawn: this.options.spawn,
env,
signal: abort.signal,
Expand Down Expand Up @@ -828,8 +835,8 @@ export class ReviewRemoteHost {
return { languageFeatures: true };
}

private facts(): Pick<ReviewGatewayHost, "declined" | "installFailure" | "target"> {
return { ...(this.declined && { declined: true as const }), ...(this.installFailure && { installFailure: this.installFailure }), ...(this.target && { target: this.target }) };
private facts(): Pick<ReviewGatewayHost, "declined" | "installFailure"> {
return { ...(this.declined && { declined: true as const }), ...(this.installFailure && { installFailure: this.installFailure }) };
}

private fail(problem: Problem): void {
Expand All @@ -842,7 +849,7 @@ export class ReviewRemoteHost {
this.cancelPending = undefined;
this.connectedAt = undefined;
this.dropMaster();
this.set({ alias: this.alias, problem, ...(this.declined && { declined: true as const }), ...(this.target && { target: this.target }) });
this.set({ alias: this.alias, problem, ...(this.declined && { declined: true as const }) });
if (problem.state !== "unreachable" || this.disposed) return;
const delay = reconnectDelay(attempt);
this.cancelTimer = this.clock.schedule(delay, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import test from "node:test";

import type { ReviewGatewayHost } from "../../common/reviewProtocol.js";
import { attachOutput, COMMIT, detectOutput, FAKE_SERVER_ID, fakeClock, fakeSsh, flush, healthServer, until, type FakeRemote } from "./test/fakeSsh.js";
import { INSTALL_SCRIPT } from "./test/installScript.js";
import type { ReviewRemoteInstallFlow } from "./reviewRemoteHost.js";
import { closeRemoteHostWindows, ReviewRemoteHosts, type ReviewRemoteHostsOptions } from "./reviewRemoteHosts.js";
import { openRemoteInstallConsent } from "./reviewRemoteInstallConsent.js";
Expand Down Expand Up @@ -271,6 +272,7 @@ test("Install on a declined host stores the agreement, installs and attaches", a
runs.push(input.version);
return INSTALLED;
},
script: async () => INSTALL_SCRIPT,
origin: () => "https://install.dev.fast",
artifact: async (target, version) => ({ name: `whiteboard-remote-${version}-${target}.tar.xz`, url: "https://install.dev.fast/x", sha256: INSTALLED_SHA256 }),
};
Expand Down Expand Up @@ -307,6 +309,7 @@ test("two hosts asking at once are asked one after the other", async (t) => {
return true;
},
run: async () => INSTALLED,
script: async () => INSTALL_SCRIPT,
origin: () => "https://install.dev.fast",
artifact: async (target, version) => ({ name: `whiteboard-remote-${version}-${target}.tar.xz`, url: "https://install.dev.fast/x", sha256: INSTALLED_SHA256 }),
};
Expand Down Expand Up @@ -339,6 +342,7 @@ async function promptingFlow(t: test.TestContext) {
runs.push(input.version);
return INSTALLED;
},
script: async () => INSTALL_SCRIPT,
origin: () => "https://install.dev.fast",
artifact: async (target, version) => ({ name: `whiteboard-remote-${version}-${target}.tar.xz`, url: "https://install.dev.fast/x", sha256: INSTALLED_SHA256 }),
};
Expand Down Expand Up @@ -449,13 +453,13 @@ test("removing Whiteboard runs while nothing reconnects, then forgets the answer
const before = ssh.calls.length;
const removed = manager.uninstall("wb-test-a");
assert.equal(manager.uninstall("wb-test-a"), removed);
await until(() => ssh.of("wb-test-a", "uninstall").length === 2);
await until(() => ssh.of("wb-test-a", "uninstall").length === 1);
manager.reattach("wb-test-a");
manager.resume();
manager.retry("wb-test-a");
while (clock.next());
await new Promise((resolve) => setTimeout(resolve, 50));
assert.deepEqual(ssh.calls.slice(before).map((call) => call.kind), ["uninstall", "uninstall"]);
assert.deepEqual(ssh.calls.slice(before).map((call) => call.kind), ["uninstall"]);
assert.ok(ssh.master("wb-test-a")!.alive);
finish();
await removed;
Expand Down Expand Up @@ -508,7 +512,7 @@ test("a setting change that keeps the alias during an uninstall changes nothing
asked[0].answer(true);
await sentUntil((hosts) => hosts[0]?.endpoint !== undefined);
const removed = manager.uninstall("wb-test-a");
await until(() => ssh.of("wb-test-a", "uninstall").length === 2);
await until(() => ssh.of("wb-test-a", "uninstall").length === 1);
manager.update(true, ["wb-test-a"]);
await new Promise((resolve) => setTimeout(resolve, 20));

Expand All @@ -534,7 +538,7 @@ test("an alias removed during a successful uninstall is closed once", async (t)
asked[0].answer(true);
await sentUntil((hosts) => hosts[0]?.endpoint !== undefined);
const removed = manager.uninstall("wb-test-a");
await until(() => ssh.of("wb-test-a", "uninstall").length === 2);
await until(() => ssh.of("wb-test-a", "uninstall").length === 1);
manager.update(true, []);
finish();
await removed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,10 @@ export class ReviewRemoteHosts {
const host = this.hosts.get(alias);
host?.quiesce();
try {
if (!this.options.install) throw new Error("This Desktop cannot install or remove Whiteboard on hosts.");
const env = { ...(await this.options.environment()), ...(await this.askpass()).env(alias) };
await uninstallRemote({ session: reviewSshSession(alias, this.options.controlDirectory, this.options.instance), spawn: this.options.spawn, env });
const script = await this.options.install.script();
await uninstallRemote({ session: reviewSshSession(alias, this.options.controlDirectory, this.options.instance), spawn: this.options.spawn, env, script });
} catch (error) {
this.options.log(`${alias}: Whiteboard was not removed; the host waits for the setting.`);
throw error;
Expand Down
Loading
Loading