From 1304ee17e08a6389cbcc42f15c3bfd43f4cd1aa9 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 16:22:13 -0400 Subject: [PATCH 1/6] Install remote hosts with one shell script packages/review/remote/install.sh probes, installs, updates and uninstalls in one ssh call each. Desktop pipes the copy it bundles, the release publishes it beside the tarballs for hand installs, and the dev install runs it, so all three write Desktop's layout and Desktop updates them. --- .github/workflows/review-desktop-ci.yml | 3 + .github/workflows/review-desktop-preview.yml | 2 + .github/workflows/review-desktop-release.yml | 4 +- .../remote/reviewRemoteHost.test.ts | 5 +- .../electron-main/remote/reviewRemoteHost.ts | 23 +- .../remote/reviewRemoteHosts.test.ts | 12 +- .../electron-main/remote/reviewRemoteHosts.ts | 4 +- .../remote/reviewRemoteInstallScript.ts | 228 +-------- .../reviewRemoteInstaller.containers.test.ts | 68 ++- .../remote/reviewRemoteInstaller.test.ts | 47 +- .../remote/reviewRemoteInstaller.ts | 222 +++------ .../reviewRemoteProbe.containers.test.ts | 7 +- .../remote/reviewRemoteProbe.test.ts | 20 +- .../electron-main/remote/reviewRemoteProbe.ts | 10 +- .../remote/reviewRemoteProbeScript.ts | 79 ---- .../remote/reviewRemoteUninstall.test.ts | 5 +- .../remote/reviewRemoteUninstall.ts | 45 +- .../electron-main/remote/test/fakeSsh.ts | 12 +- .../remote/test/installScript.ts | 9 + .../review/electron-main/reviewDesktopHost.ts | 15 +- .../scripts/build-remote-tarball.mjs | 1 + .../scripts/e2e/journeys/remote-install.mjs | 39 +- .../scripts/e2e/remote/releases.mjs | 11 +- .../scripts/remote-dev-install.mjs | 41 +- .../scripts/stage-review-runtime.mjs | 1 + docs/remote-hosts.md | 9 +- packages/review-protocol/src/contracts.ts | 1 - packages/review/package.json | 1 + packages/review/remote/install.sh | 440 ++++++++++++++++++ .../src/server/review-gateway-hosts.test.ts | 23 +- .../review/src/server/review-gateway-hosts.ts | 8 +- 31 files changed, 750 insertions(+), 645 deletions(-) delete mode 100644 apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbeScript.ts create mode 100644 apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/installScript.ts create mode 100755 packages/review/remote/install.sh diff --git a/.github/workflows/review-desktop-ci.yml b/.github/workflows/review-desktop-ci.yml index a701b76b5..9aedbd363 100644 --- a/.github/workflows/review-desktop-ci.yml +++ b/.github/workflows/review-desktop-ci.yml @@ -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 diff --git a/.github/workflows/review-desktop-preview.yml b/.github/workflows/review-desktop-preview.yml index 77314dfc4..8d9df805b 100644 --- a/.github/workflows/review-desktop-preview.yml +++ b/.github/workflows/review-desktop-preview.yml @@ -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' }} @@ -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: | diff --git a/.github/workflows/review-desktop-release.yml b/.github/workflows/review-desktop-release.yml index 11f1b681b..f77d28a80 100644 --- a/.github/workflows/review-desktop-release.yml +++ b/.github/workflows/review-desktop-release.yml @@ -378,6 +378,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 # 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 @@ -385,7 +386,7 @@ jobs: 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: @@ -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: | 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 0937b3ca8..d166d075f 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 @@ -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"; @@ -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 ?? @@ -774,7 +776,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" }); @@ -983,7 +984,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); @@ -1069,7 +1069,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); 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 d524cc31c..d0be87cf0 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 @@ -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 { +export function runSsh(spawn: SpawnSsh, env: NodeJS.ProcessEnv, args: string[], timeout: number, input?: string, onStdout?: (chunk: string) => void): Promise { return new Promise((resolve) => { let child: SshChildProcess; try { @@ -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 })); @@ -229,7 +232,7 @@ export function runSsh(spawn: SpawnSsh, env: NodeJS.ProcessEnv, args: string[], export type ReviewRemoteInstallMode = "ask" | "always" | "never"; -export type ReviewRemoteInstallRunInput = Pick; +export type ReviewRemoteInstallRunInput = Pick; export interface ReviewRemoteInstallFlow { mode(): ReviewRemoteInstallMode; @@ -239,6 +242,8 @@ export interface ReviewRemoteInstallFlow { run(input: ReviewRemoteInstallRunInput): Promise; origin(): string | undefined; artifact(target: ReviewRemoteTarget, version: string): Promise; + /** packages/review/remote/install.sh, which probes and installs. */ + script(): Promise; } type InstallStep = NonNullable["step"]; @@ -315,7 +320,6 @@ export class ReviewRemoteHost { private detecting: Promise | undefined; private declined = false; private installFailure: string | undefined; - private target: ReviewRemoteTarget | undefined; private installing: AbortController | undefined; private removing = false; serverId: string | undefined; @@ -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; @@ -691,6 +695,7 @@ export class ReviewRemoteHost { target, version, artifact, + script: await flow.script(), spawn: this.options.spawn, env, signal: abort.signal, @@ -828,8 +833,8 @@ export class ReviewRemoteHost { return { languageFeatures: true }; } - private facts(): Pick { - return { ...(this.declined && { declined: true as const }), ...(this.installFailure && { installFailure: this.installFailure }), ...(this.target && { target: this.target }) }; + private facts(): Pick { + return { ...(this.declined && { declined: true as const }), ...(this.installFailure && { installFailure: this.installFailure }) }; } private fail(problem: Problem): void { @@ -842,7 +847,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, () => { 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 10595f9ef..9c060f609 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 @@ -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"; @@ -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 }), }; @@ -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 }), }; @@ -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 }), }; @@ -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; @@ -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)); @@ -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; 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 97f788f4a..609f0bdab 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 @@ -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; diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstallScript.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstallScript.ts index 34df3340a..88523aa84 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstallScript.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstallScript.ts @@ -3,7 +3,10 @@ * Licensed under the MIT License. See LICENSE in the repository root for license information. *--------------------------------------------------------------------------------------------*/ -import { REVIEW_REMOTE_INSTALL_LOCK, REVIEW_REMOTE_LOCK_STALE_SECONDS, REVIEW_REMOTE_VERSION, REVIEW_REMOTE_WRAPPER_MARK } from "../../common/reviewProtocol.js"; +import { readFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; + +import { REVIEW_REMOTE_LOCK_STALE_SECONDS, REVIEW_REMOTE_VERSION, REVIEW_REMOTE_WRAPPER_MARK } from "../../common/reviewProtocol.js"; export { REVIEW_REMOTE_LOCK_STALE_SECONDS, REVIEW_REMOTE_VERSION, REVIEW_REMOTE_WRAPPER_MARK }; @@ -17,225 +20,12 @@ export function shellQuote(value: string, lines = false): string { return `'${value.replace(/'/g, `'\\''`)}'`; } -export interface ReviewRemoteInstallContext { - readonly home: string; - readonly root: string; - readonly token: string; -} - -export const REVIEW_REMOTE_ROOT_SCRIPT = `base=$(printf '%s' "\${DEV_REVIEW_HOME-}" | sed 's/^[[:space:]]*//; s/[[:space:]]*$//') -root= -case "$base" in -'') root=$HOME/.dev/whiteboard-remote ;; -*//*|*/./*|*/../*|*/.|*/..|*/./|*/../) ;; -/*) root=\${base%/}/whiteboard-remote ;; -esac -[ "$(printf '%s' "\${DEV_REVIEW_HOME-}" | tr -d '\\001-\\037\\177')" = "\${DEV_REVIEW_HOME-}" ] || root= -`; - export const reviewRemoteVersionDir = (root: string, version: string) => `${root}/versions/${version}`; -const reviewRemoteWrapperPath = (home: string) => `${home}/.local/bin/whiteboard`; - -const versionPart = (context: ReviewRemoteInstallContext, version: string) => `${reviewRemoteVersionDir(context.root, version)}.${context.token}.part`; - -function prelude(context: ReviewRemoteInstallContext): string { - if (!/^[0-9a-f]{8,64}$/.test(context.token)) throw new Error("The install token is not hex."); - return `LC_ALL=C -export LC_ALL -umask 022 -trap '' PIPE -exec 3>&1 -root=${shellQuote(context.root)} -lock="$root/${REVIEW_REMOTE_INSTALL_LOCK}" -token=${context.token} -say() { printf '\\n%s %s\\n' ${REVIEW_REMOTE_INSTALL_SAY} "$*" >&3; } -fail() { say FAIL "$*"; exit 3; } -# A rename, so a reader never sees "started" half written. -stamp() { date +%s > "$lock/started.$token" && mv -f "$lock/started.$token" "$lock/started"; } -own() { - [ "$(cat "$lock/token" 2>/dev/null)" = "$token" ] || fail this install no longer holds the install lock - stamp -} -guard() { - if command -v setsid >/dev/null 2>&1; then setsid "$@" & else "$@" & fi - pid=$! - while kill -0 "$pid" 2>/dev/null; do - printf '.\\n' >&3 2>/dev/null || { kill -TERM -"$pid" 2>/dev/null || kill "$pid" 2>/dev/null; exit 3; } - [ "$(cat "$lock/token" 2>/dev/null)" = "$token" ] && stamp - sleep 1 - done - wait "$pid" -} -`; -} - -export function lockScript(context: ReviewRemoteInstallContext, owner: string, staleSeconds = REVIEW_REMOTE_LOCK_STALE_SECONDS): string { - const staleMinutes = Math.ceil(staleSeconds / 60); - return `${prelude(context)}mkdir -p "$root" || fail cannot create "$root" -take() { - mkdir "$lock" 2>/dev/null || return 1 - printf '%s\\n' "$token" > "$lock/token" - printf '%s\\n' ${shellQuote(owner)} > "$lock/owner" - stamp - say LOCKED - exit 0 -} -take -# The token first: a takeover completed between the two reads leaves "started" fresh. -held=$(cat "$lock/token" 2>/dev/null) -started=$(cat "$lock/started" 2>/dev/null) -case "$started" in -''|*[!0-9]*) - # Never written (a holder that died right after mkdir): by the directory's age. Unreadable: fresh. - if [ ! -e "$lock/started" ] && [ -n "$(find "$lock" -prune -mmin +${staleMinutes} 2>/dev/null)" ]; then started=0; else started=$(date +%s); fi ;; -esac -if [ $(( $(date +%s) - started )) -ge ${staleSeconds} ]; then - stale="$lock.$token.stale" - if mv "$lock" "$stale" 2>/dev/null; then - # Another install may have taken it over first: give that one back. - if [ "$(cat "$stale/token" 2>/dev/null)" = "$held" ] || [ -e "$lock" ]; then rm -rf "$stale"; else mv "$stale" "$lock"; fi - fi - take -fi -say BUSY "$(cat "$lock/owner" 2>/dev/null)" -`; -} - -export function releaseScript(context: ReviewRemoteInstallContext): string { - return `${prelude(context)}if [ "$(cat "$lock/token" 2>/dev/null)" = "$token" ] && mv "$lock" "$lock.$token.done" 2>/dev/null; then - rm -rf "$lock.$token.done" -fi -say RELEASED -`; -} - -export function prepareScript(context: ReviewRemoteInstallContext, input: { version: string; sha256: string }): string { - return `${prelude(context)}own -rm -rf "$root"/versions/*.part "$lock".*.stale "$lock".*.done -mkdir -p "$root/versions" || fail cannot create "$root/versions" -${markerCheck(context, input)} -p=${shellQuote(versionPart(context, input.version))} -if [ -n "$complete" ]; then - say COMPLETE - say MARKER "$(cat "$m")" -elif [ -e "$v" ]; then - mv "$v" "$p" && rm -rf "$p" || fail cannot remove "$v" -fi -for dir in "$root"/versions/*; do - [ -n "$(completeSha256 "$dir/${REVIEW_REMOTE_INSTALL_MARKER}")" ] && say HAVE "\${dir##*/}" && continue - # A version whose removal was cut short (its marker went first). - case "\${dir##*/}" in [0-9]*.[0-9]*.[0-9]*) rm -rf "$dir" ;; esac -done -[ -n "$complete" ] || mkdir "$p" || fail cannot create "$p" -say PREPARED -`; -} - -export const REVIEW_REMOTE_COMPLETE_SHA256 = `completeSha256() { - cs=$(sed -n 's/.*"sha256":"\\([^"][^"]*\\)".*/\\1/p' "$1" 2>/dev/null) - cn=$(sed -n 's/.*"node":"\\([^"]*\\)".*/\\1/p' "$1" 2>/dev/null) - cc=$(sed -n 's/.*"cli":"\\([^"]*\\)".*/\\1/p' "$1" 2>/dev/null) - [ -n "$cs" ] && [ -x "$cn" ] && [ -f "$cc" ] && printf '%s' "$cs" -}`; -function markerCheck(context: ReviewRemoteInstallContext, input: { version: string; sha256: string }): string { - return `v=${shellQuote(reviewRemoteVersionDir(context.root, input.version))} -m="$v/${REVIEW_REMOTE_INSTALL_MARKER}" -${REVIEW_REMOTE_COMPLETE_SHA256} -complete= -[ -f "$m" ] && [ "$(completeSha256 "$m")" = ${shellQuote(input.sha256)} ] && complete=1`; -} - -export function completeScript(context: ReviewRemoteInstallContext, input: { version: string; sha256: string }): string { - return `${prelude(context)}${markerCheck(context, input)} -[ -n "$complete" ] || exit 0 -say COMPLETE -say MARKER "$(cat "$m")" -`; -} +/** packages/review/remote/install.sh, beside the review server Desktop runs. */ +export const reviewRemoteInstallScriptPath = (serverEntry: string) => join(dirname(serverEntry), "..", "..", "remote", "install.sh"); -const packageTarball = (context: ReviewRemoteInstallContext, version: string) => `${versionPart(context, version)}/whiteboard-remote.tar.xz`; +export const readRemoteInstallScript = (serverEntry: string) => readFile(reviewRemoteInstallScriptPath(serverEntry), "utf8"); -const DOWNLOAD = { - curl: `guard curl -fsSL --connect-timeout 20 --max-time 900 -o "$f" "$url"`, - wget: `hsts= -wget --help 2>&1 | grep -q -- --no-hsts && hsts=--no-hsts -limit= -command -v timeout >/dev/null 2>&1 && limit="timeout 900" -guard $limit wget -q $hsts -t 2 -T 30 -O "$f" "$url"`, -}; - -export function unpackScript( - context: ReviewRemoteInstallContext, - input: { version: string; sha256: string; url: string; downloader: "curl" | "wget" }, -): string { - if (!/^[0-9a-f]{64}$/.test(input.sha256)) throw new Error("The tarball checksum is not a sha256."); - return `${prelude(context)}own -p=${shellQuote(versionPart(context, input.version))} -f=${shellQuote(packageTarball(context, input.version))} -url=${shellQuote(input.url)} -${DOWNLOAD[input.downloader]} || { s=$?; rm -rf "$p"; fail cannot download "$url": ${input.downloader} exited with "$s"; } -sum=$(sha256sum "$f" 2>/dev/null || openssl dgst -sha256 -r "$f" 2>/dev/null) -sum=\${sum%% *} -[ "$sum" = ${input.sha256} ] || { rm -rf "$p"; say MISMATCH "$sum"; exit 3; } -guard tar -xJf "$f" -C "$p" --strip-components=1 || { rm -rf "$p"; fail cannot unpack the tarball; } -rm -f "$f" -[ -x "$p/bin/node" ] && [ -f "$p/dist/cli.js" ] && [ -x "$p/whiteboard" ] || { rm -rf "$p"; fail the tarball holds no whiteboard; } -out=$(DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 DEV_FAST_REVIEW_TELEMETRY_DISABLED=1 "$p/bin/node" "$p/dist/cli.js" version --json /dev/null | tail -n 1) -say VERSION "$out" -say UNPACKED -`; -} - -export function finishScript( - context: ReviewRemoteInstallContext, - input: { version: string; marker: string; wrapper?: string }, -): string { - const wrapper = input.wrapper - ? `w=${shellQuote(reviewRemoteWrapperPath(context.home))} -if { [ ! -e "$w" ] && [ ! -L "$w" ]; } || { [ -f "$w" ] && [ ! -L "$w" ] && grep -qxF ${shellQuote(REVIEW_REMOTE_WRAPPER_MARK)} "$w"; }; then - if mkdir -p "\${w%/*}" && printf '%s' ${shellQuote(input.wrapper, true)} > "$w.$token.part" && chmod 755 "$w.$token.part" && mv -f "$w.$token.part" "$w"; then - say WRAPPER written - else - rm -f "$w.$token.part" - say WRAPPER failed - fi -else - say WRAPPER foreign -fi -` - : ""; - return `${prelude(context)}own -p=${shellQuote(versionPart(context, input.version))} -v=${shellQuote(reviewRemoteVersionDir(context.root, input.version))} -printf '%s\\n' ${shellQuote(input.marker)} > "$p/${REVIEW_REMOTE_INSTALL_MARKER}" || fail cannot write the marker -[ -e "$v" ] && { rm -rf "$p"; fail "$v" appeared during the install; } -mv "$p" "$v" || fail cannot move the version into place -say COMPLETE -${wrapper}rm -rf "$root"/versions/*.part -say FINISHED -`; -} - -export function cleanupScript(context: ReviewRemoteInstallContext, input: { candidates: readonly string[]; room: number }): string { - const names = input.candidates.map((name) => shellQuote(name)).join(" "); - return `${prelude(context)}own -# Read once so a grep below never finds itself; launchers run the CLI by absolute path. -if [ -d /proc/self ]; then - procs=$(for f in /proc/[0-9]*/cmdline; do tr '\\000' ' ' < "$f" 2>/dev/null; echo; done) -else - procs=$(ps -eo args= 2>/dev/null) -fi -running() { printf '%s\\n' "$procs" | grep -qF -- "$root/versions/$1/"; } -kept=0 -for v in ${names}; do running "$v" && kept=$((kept + 1)); done -for v in ${names}; do - if running "$v"; then say IN-USE "$v"; continue; fi - if [ "$kept" -lt ${input.room} ]; then kept=$((kept + 1)); continue; fi - d="$root/versions/$v" - [ -f "$d/${REVIEW_REMOTE_INSTALL_MARKER}" ] || continue - rm -f "$d/${REVIEW_REMOTE_INSTALL_MARKER}" && rm -rf "$d" && say REMOVED "$v" -done -say CLEANED -`; -} +/** The script as `sh -s` input, with its arguments set first. */ +export const remoteInstallScriptInput = (script: string, args: readonly string[]) => `set -- ${args.map((arg) => shellQuote(arg)).join(" ")}\n${script}`; diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.containers.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.containers.test.ts index a41912dbb..80f082c46 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.containers.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.containers.test.ts @@ -17,11 +17,13 @@ import type { SpawnSsh } from "./reviewRemoteHost.js"; import { installRemote, type ReviewRemoteInstallInput, type ReviewRemoteInstallProgress } from "./reviewRemoteInstaller.js"; import { judgeRemote, probeRemote, type ReviewRemoteProbe, type ReviewRemoteTarget } from "./reviewRemoteProbe.js"; import { REVIEW_SSH_CONFIG_ENV, reviewSshSession, sshCheckArgs, sshCloseArgs, sshMasterArgs, type ReviewSshSession } from "./reviewSshCommand.js"; +import { INSTALL_SCRIPT } from "./test/installScript.js"; const run = promisify(execFile); const checkout = resolve(import.meta.dirname, "../../../../../../../.."); const harness = join(checkout, "apps/review-desktop/scripts/e2e/remote/remote.mjs"); const VERSION: string = JSON.parse(await readFile(join(checkout, "packages/review/package.json"), "utf8")).version; +const NEXT = VERSION.replace(/^(\d+)\.(\d+)\.(\d+).*$/, (_, major, minor, patch) => `${major}.${minor}.${Number(patch) + 1}`); function skipReason(): string | undefined { if (process.env.WB_TEST_CONTAINERS !== "1") return "set WB_TEST_CONTAINERS=1 to install into containers"; @@ -43,7 +45,7 @@ const realSsh: SpawnSsh = (args, options) => spawn("ssh", args, options); let started = false; let controlDirectory = ""; -let releases: { url: string; stage(target: ReviewRemoteTarget): Promise; close(): Promise } | undefined; +let releases: { url: string; stage(target: ReviewRemoteTarget, version?: string): Promise; close(): Promise } | undefined; const masters: ReviewSshSession[] = []; const found = new Map(); const artifacts = new Map(); @@ -71,7 +73,7 @@ before( for (const [host, flags] of Object.entries(hosts)) await remote("up", host, ...flags); for (const host of Object.keys(hosts) as Host[]) { const session = await connect(host); - const probed = await probeRemote({ session, spawn: realSsh, env: sshEnv }); + const probed = await probeRemote({ session, spawn: realSsh, env: sshEnv, script: INSTALL_SCRIPT }); assert.ok("probe" in probed, "error" in probed ? probed.error : ""); const judged = judgeRemote(probed.probe); assert.ok(judged.supported); @@ -110,6 +112,7 @@ function input(host: Host, change: Partial = {}) { onProgress: (step) => progress.push(step), signal: new AbortController().signal, artifact: artifacts.get(target)!, + script: INSTALL_SCRIPT, spawn: realSsh, env: sshEnv, ...change, @@ -233,3 +236,64 @@ test("with three versions installed and one running, an install leaves the runni assert.equal(await inContainer("fresh", "ls ~/.dev/whiteboard-remote/versions | tr '\\n' ' '"), `${VERSION} ${VERSION}-old.1`); }); + +/** NEXT's tarball for the host, built and served once. */ +async function nextArtifact(host: Host): Promise { + const { target } = found.get(host)!; + await releases!.stage(target, NEXT); + return remoteArtifact(target, NEXT, { pin: undefined, releases: releases!.url }); +} + +const launcherTarget = (host: Host) => inContainer(host, "sed -n 's/^exec .\\(.*\\)\\/whiteboard. .*/\\1/p' ~/.local/bin/whiteboard"); + +test("an update installs the new version beside the old one, and the launcher moves to it", { skip, timeout: 30 * 60_000 }, async () => { + await reset("fresh"); + await installRemote(input("fresh").value); + const next = await nextArtifact("fresh"); + + const { value, progress } = input("fresh", { version: NEXT, artifact: next }); + const result = await installRemote(value); + + const updated = `/home/dev/.dev/whiteboard-remote/versions/${NEXT}`; + assert.deepEqual(result, { nodePath: `${updated}/bin/node`, cliPath: `${updated}/dist/cli.js` }); + assert.deepEqual(progress.map((p) => p.step), ["package", "done"]); + assert.equal(await version("fresh"), NEXT); + assert.equal(await launcherTarget("fresh"), updated); + assert.equal(await inContainer("fresh", "ls ~/.dev/whiteboard-remote/versions | tr '\\n' ' '"), `${VERSION} ${NEXT}`); + + const probed = await probeRemote({ session: found.get("fresh")!.session, spawn: realSsh, env: sshEnv, script: INSTALL_SCRIPT }); + assert.ok("probe" in probed); + assert.deepEqual(probed.probe.installed.map((entry) => entry.version).sort(), [VERSION, NEXT].sort()); +}); + +test("a hand install through the published install.sh is one Desktop recognises and updates", { skip, timeout: 30 * 60_000 }, async () => { + await reset("fresh"); + const base = releases!.url.replace(/\/$/, ""); + + await inContainer("fresh", `curl -fsSL ${base}/${VERSION}/remote/install.sh | WHITEBOARD_RELEASES=${base} sh -s -- --version ${VERSION}`); + + assert.equal(await version("fresh"), VERSION); + const probed = await probeRemote({ session: found.get("fresh")!.session, spawn: realSsh, env: sshEnv, script: INSTALL_SCRIPT }); + assert.ok("probe" in probed); + assert.deepEqual(probed.probe.installed.map((entry) => entry.version), [VERSION]); + assert.equal(probed.probe.pathCli, null); + + await installRemote(input("fresh", { version: NEXT, artifact: await nextArtifact("fresh") }).value); + assert.equal(await version("fresh"), NEXT); +}); + +test("the old hand-install layout is adopted: its link moves to the install and its tree goes", { skip, timeout: 10 * 60_000 }, async () => { + await reset("fresh"); + const { target } = found.get("fresh")!; + const tarball = `${releases!.url}whiteboard-remote-${VERSION}-${target}.tar.xz`; + await inContainer( + "fresh", + `mkdir -p ~/.dev/whiteboard-remote ~/.local/bin && curl -fsSL ${tarball} | tar -xJ -C ~/.dev/whiteboard-remote && ln -sf ~/.dev/whiteboard-remote/whiteboard-remote/whiteboard ~/.local/bin/whiteboard`, + ); + + await installRemote(input("fresh").value); + + assert.equal(await launcherTarget("fresh"), installed); + assert.equal(await inContainer("fresh", "test -e ~/.dev/whiteboard-remote/whiteboard-remote && echo kept || echo gone"), "gone"); + assert.equal(await version("fresh"), VERSION); +}); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts index e2b6159d8..6bc83b86e 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts @@ -14,10 +14,11 @@ import test from "node:test"; import type { SpawnSsh } from "./reviewRemoteHost.js"; import { probeRemote } from "./reviewRemoteProbe.js"; import { uninstallRemote } from "./reviewRemoteUninstall.js"; -import { compareVersions, installRemote, type ReviewRemoteInstallInput, type ReviewRemoteInstallProgress } from "./reviewRemoteInstaller.js"; -import { cleanupScript, lockScript, REVIEW_REMOTE_INSTALL_MARKER, REVIEW_REMOTE_WRAPPER_MARK, shellQuote } from "./reviewRemoteInstallScript.js"; +import { installRemote, type ReviewRemoteInstallInput, type ReviewRemoteInstallProgress } from "./reviewRemoteInstaller.js"; +import { REVIEW_REMOTE_INSTALL_MARKER, REVIEW_REMOTE_WRAPPER_MARK, remoteInstallScriptInput, shellQuote } from "./reviewRemoteInstallScript.js"; import type { ReviewRemoteProbe } from "./reviewRemoteProbe.js"; import { reviewSshSession } from "./reviewSshCommand.js"; +import { INSTALL_SCRIPT } from "./test/installScript.js"; const VERSION = "9.9.9"; const PATH = `${dirname(process.execPath)}:/usr/bin:/bin:/usr/sbin:/sbin`; @@ -91,6 +92,7 @@ async function fixture(t: test.TestContext, cli = CLI) { onProgress: (step) => progress.push(step), signal: new AbortController().signal, artifact, + script: INSTALL_SCRIPT, spawn: localRemote(home, env), env: {}, owner: "laptop-test", @@ -128,9 +130,9 @@ test("a DEV_REVIEW_HOME the remote cannot normalise is refused by the probe and const f = await fixture(t); for (const home of ["relative/home", `${f.root}/a/../b`, `${f.root}//b`, `${f.root}/./b`, `${f.root}/a\nb`, `${f.root}/a\tb`]) { const spawn = localRemote(f.home, { ...f.env, DEV_REVIEW_HOME: home }); - const probed = await probeRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {} }); + const probed = await probeRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {}, script: INSTALL_SCRIPT }); assert.match("error" in probed ? probed.error : "", /DEV_REVIEW_HOME there is not an absolute, normalised path/, home); - await assert.rejects(uninstallRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {} }), /DEV_REVIEW_HOME there is not an absolute, normalised path/); + await assert.rejects(uninstallRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {}, script: INSTALL_SCRIPT }), /DEV_REVIEW_HOME there is not an absolute, normalised path/); } assert.deepEqual(await readdir(f.home), []); }); @@ -141,7 +143,7 @@ test("DEV_REVIEW_HOME moves the probe's versions, the install, its launcher and const moved = join(f.root, "moved home"); const spawn = localRemote(f.home, { ...f.env, DEV_REVIEW_HOME: ` ${moved}/ ` }); const probed = async () => { - const result = await probeRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {} }); + const result = await probeRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); return result.probe; }; @@ -154,7 +156,7 @@ test("DEV_REVIEW_HOME moves the probe's versions, the install, its launcher and assert.ok(result.cliPath.startsWith(`${root}/versions/${VERSION}/`), result.cliPath); assert.equal(run(`${root}/versions/${VERSION}/whiteboard`, "version", "--json"), `{"event":"version","version":"${VERSION}"}`); assert.deepEqual((await probed()).installed, [{ version: VERSION, sha256: f.pack.sha256 }]); - await uninstallRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {} }); + await uninstallRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn, env: {}, script: INSTALL_SCRIPT }); assert.deepEqual(await readdir(f.home), [".local"]); }); @@ -187,7 +189,7 @@ test("a download that fails on the host fails the install with curl's exit statu const f = await fixture(t); const spawn = localRemote(f.home, await onPath(f.root, "curl", "exit 22\n")); - await assert.rejects(installRemote(f.input({ spawn })), new RegExp(`while downloading and unpacking .*: cannot download ${RELEASE_URL.replaceAll(".", "\\.")}: curl exited with 22\\.$`)); + await assert.rejects(installRemote(f.input({ spawn })), new RegExp(`failed: cannot download ${RELEASE_URL.replaceAll(".", "\\.")}: curl exited with 22\\.$`)); assert.deepEqual(await f.versions(), []); assert.deepEqual((await readdir(f.remoteRoot)).sort(), ["versions"]); @@ -228,7 +230,7 @@ test("a host where no checksum tool answers fails the install and says so", asyn test("a tarball whose whiteboard reports another version fails the install", async (t) => { const f = await fixture(t, cliFor("1.2.3")); - await assert.rejects(installRemote(f.input()), /The tarball installed on devbox reports version 1\.2\.3, not 9\.9\.9/); + await assert.rejects(installRemote(f.input()), /Installing on devbox failed: the tarball reports version 1\.2\.3, not 9\.9\.9/); assert.ok(!(await f.versions()).includes(VERSION)); }); @@ -430,11 +432,19 @@ test("shellQuote survives quotes and refuses control characters", async () => { assert.throws(() => shellQuote("a\u0007b"), /control character/); }); -test("versions order as semver", () => { - const sorted = ["0.1.10", "0.1.7", "0.1.7-preview.20261003.10", "0.1.6", "0.1.7-preview.20261003.2", "0.2.0-preview.1"].sort(compareVersions); - assert.deepEqual(sorted, ["0.1.6", "0.1.7-preview.20261003.2", "0.1.7-preview.20261003.10", "0.1.7", "0.1.10", "0.2.0-preview.1"]); +test("prereleases order by number, so cleanup keeps the newest one", async (t) => { + const f = await fixture(t); + for (const version of ["9.9.10-preview.2", "9.9.10-preview.10", "9.9.9-preview.3"]) await earlierVersion(f.remoteRoot, version); + + await installRemote(f.input()); + + assert.deepEqual((await f.versions()).sort(), ["9.9.10-preview.10", VERSION].sort()); }); +/** Runs the script's install by hand, as a second laptop would, giving up at once on a held lock. */ +const busyInstall = (home: string) => + remoteInstallScriptInput(INSTALL_SCRIPT, ["install", "--version", VERSION, "--sha256", "0".repeat(64), "--url", RELEASE_URL, "--token", "00112233aabbccdd", "--owner", "stealer", "--lock-wait", "0"]); + test("a lock taken over between the stealer's two reads is left to its new holder", async (t) => { const f = await fixture(t); const lock = join(f.remoteRoot, "install.lock"); @@ -465,7 +475,7 @@ esac child.stdout.setEncoding("utf8").on("data", (chunk: string) => (text += chunk)); child.once("error", reject); child.once("close", () => resolve(text)); - child.stdin.end(lockScript({ home: f.home, root: f.remoteRoot, token: "00112233aabbccdd" }, "stealer")); + child.stdin.end(busyInstall(f.home)); }); assert.match(out, /WHITEBOARD-INSTALL BUSY winner/); @@ -511,22 +521,21 @@ test("a root with a quote is refused before anything runs", async (t) => { test("a read of the lock's start time during refreshes never finds it empty", async (t) => { const f = await fixture(t); - const context = { home: f.home, root: f.remoteRoot, token: "00112233aabbccdd" }; - assert.match(await sh(f.home, lockScript(context, "me")), /LOCKED/); - const started = join(f.remoteRoot, "install.lock", "started"); - const refreshes = sh(f.home, cleanupScript(context, { candidates: [], room: 0 }).replace("own\n", "i=0\nwhile [ $i -lt 400 ]; do own; i=$((i + 1)); done\n")); + const installing = installRemote(f.input({ spawn: localRemote(f.home, await slowTar(f.root, "sleep 4")) })); let done = false; - void refreshes.then(() => (done = true)); + void installing.then(() => (done = true)); + const started = join(f.remoteRoot, "install.lock", "started"); let reads = 0; const bad: string[] = []; while (!done) { const text = await readFile(started, "utf8").catch((error: NodeJS.ErrnoException) => error.code ?? "error"); + if (text === "ENOENT") continue; reads++; if (!/^\d+\n$/.test(text)) bad.push(JSON.stringify(text)); } - assert.match(await refreshes, /CLEANED/); + await installing; assert.ok(reads > 50, `${reads} reads`); assert.deepEqual(bad, []); }); @@ -540,6 +549,6 @@ test("a lock whose start time cannot be read counts as fresh", async (t) => { await writeFile(join(lock, "started"), ""); execFileSync("touch", ["-t", "202001010000", lock]); - assert.match(await sh(f.home, lockScript({ home: f.home, root: f.remoteRoot, token: "00112233aabbccdd" }, "me")), /BUSY busy-laptop/); + assert.match(await sh(f.home, busyInstall(f.home)), /BUSY busy-laptop/); assert.equal(await readFile(join(lock, "token"), "utf8"), "0123456789abcdef\n"); }); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts index eb27645ca..ed3b653e0 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts @@ -8,23 +8,8 @@ import { hostname } from "node:os"; import type { ReviewRemoteArtifact } from "./reviewRemoteArtifacts.js"; import { lastLine, runSsh, type RunResult, type SpawnSsh, type SshChildProcess } from "./reviewRemoteHost.js"; -import { - cleanupScript, - completeScript, - finishScript, - lockScript, - prepareScript, - releaseScript, - REVIEW_REMOTE_INSTALL_SAY, - REVIEW_REMOTE_LOCK_STALE_SECONDS, - REVIEW_REMOTE_VERSION, - REVIEW_REMOTE_WRAPPER_MARK, - reviewRemoteVersionDir, - shellQuote, - unpackScript, - type ReviewRemoteInstallContext, -} from "./reviewRemoteInstallScript.js"; -import { missingInstallTools, type ReviewRemoteProbe, type ReviewRemoteTarget } from "./reviewRemoteProbe.js"; +import { REVIEW_REMOTE_INSTALL_SAY, REVIEW_REMOTE_LOCK_STALE_SECONDS, REVIEW_REMOTE_VERSION, remoteInstallScriptInput } from "./reviewRemoteInstallScript.js"; +import type { ReviewRemoteProbe, ReviewRemoteTarget } from "./reviewRemoteProbe.js"; import { sshExecArgs, type ReviewSshSession } from "./reviewSshCommand.js"; export interface ReviewRemoteInstallProgress { @@ -32,7 +17,6 @@ export interface ReviewRemoteInstallProgress { } const REVIEW_REMOTE_INSTALL_TIMEOUTS = { - step: 30_000, fetch: 25 * 60_000, lockWait: REVIEW_REMOTE_LOCK_STALE_SECONDS * 1000, lockStale: REVIEW_REMOTE_LOCK_STALE_SECONDS * 1000, @@ -48,6 +32,8 @@ export interface ReviewRemoteInstallInput { onProgress(progress: ReviewRemoteInstallProgress): void; readonly signal: AbortSignal; readonly artifact: ReviewRemoteArtifact; + /** packages/review/remote/install.sh. */ + readonly script: string; readonly spawn: SpawnSsh; readonly env: NodeJS.ProcessEnv; readonly owner?: string; @@ -73,12 +59,19 @@ export async function installRemote(input: ReviewRemoteInstallInput): Promise String(ms / 1000); + const args = [ + "install", + ...["--version", input.version, "--sha256", sha256, "--url", artifact.url, "--owner", owner, "--token", token], + ...(probe.downloader ? ["--downloader", probe.downloader] : []), + ...["--lock-wait", seconds(timeouts.lockWait), "--lock-poll", seconds(timeouts.lockPoll), "--lock-stale", String(Math.ceil(timeouts.lockStale / 1000))], + ]; const children = new Set(); - const tracked: SpawnSsh = (args, options) => { - const child = input.spawn(args, options); + const tracked: SpawnSsh = (spawnArgs, options) => { + const child = input.spawn(spawnArgs, options); children.add(child); child.once("close", () => children.delete(child)); return child; @@ -92,131 +85,70 @@ export async function installRemote(input: ReviewRemoteInstallInput): Promise => { - signal.throwIfAborted(); - const result = await runSsh(tracked, input.env, sshExecArgs(session, input.env), timeout, script); - signal.throwIfAborted(); - return answer(alias, what, result, timeout); + let pending = ""; + const told = new Set(); + const onStdout = (chunk: string) => { + const lines = (pending + chunk).split("\n"); + pending = lines.pop() ?? ""; + for (const line of lines) { + const step = line.startsWith(`${REVIEW_REMOTE_INSTALL_SAY} STEP `) ? line.slice(REVIEW_REMOTE_INSTALL_SAY.length + 6).trim() : undefined; + if ((step === "waiting-for-lock" || step === "package") && !told.has(step)) { + told.add(step); + input.onProgress({ step }); + } + } }; - let locking = false; + const timeout = timeouts.lockWait + timeouts.fetch + 60_000; + let result: RunResult; try { - let result = await alreadyComplete(); - if (!result) { - const missing = missingInstallTools(probe, alias); - if (missing || !probe.downloader) throw new Error(missing); - locking = true; - await takeLock(); - result = await install(probe.downloader); - } - input.onProgress({ step: "done" }); - return result; + result = await runSsh(tracked, input.env, sshExecArgs(session, input.env), timeout, remoteInstallScriptInput(input.script, args), onStdout); } finally { signal.removeEventListener("abort", onAbort); - if (locking) await runSsh(input.spawn, input.env, sshExecArgs(session, input.env), timeouts.release, releaseScript(context)); } - - async function alreadyComplete(): Promise { - const checked = await run("checking the installed version", completeScript(context, { version: input.version, sha256 })).catch((error: unknown) => { - if (signal.aborted) throw error; - return undefined; - }); - return checked?.has("COMPLETE") ? readMarker(checked.get("MARKER")) : undefined; + const says = answer(alias, result, timeout); + if (signal.aborted || result.timedOut) { + await runSsh(input.spawn, input.env, sshExecArgs(session, input.env), timeouts.release, remoteInstallScriptInput(input.script, ["release", "--token", token])); + signal.throwIfAborted(); } - - async function takeLock(): Promise { - const deadline = Date.now() + timeouts.lockWait; - let told = false; - for (;;) { - const says = await run("taking the install lock", lockScript(context, owner, Math.ceil(timeouts.lockStale / 1000))); - if (says.has("LOCKED")) return; - const holder = says.get("BUSY"); - if (holder === undefined) throw says.failure(); - if (Date.now() >= deadline) { - throw new Error(`Another install on ${alias} (${holder || "unknown"}) held the lock for ${timeouts.lockWait / 60_000} minutes.`); - } - if (!told) input.onProgress({ step: "waiting-for-lock" }); - told = true; - await delay(timeouts.lockPoll, signal); - } + const done = readMarker(says.get("DONE")); + if (done) { + input.onProgress({ step: "done" }); + return done; } - - async function install(downloader: "curl" | "wget"): Promise { - const prepared = await run("preparing", prepareScript(context, { version: input.version, sha256 })); - if (!prepared.has("PREPARED")) throw prepared.failure(); - if (prepared.has("COMPLETE")) { - const marker = readMarker(prepared.get("MARKER")); - if (!marker) throw new Error(`The install marker on ${alias} for ${input.version} cannot be read.`); - return marker; - } - - input.onProgress({ step: "package" }); - const unpacked = await run( - `downloading and unpacking ${artifact.name}`, - unpackScript(context, { version: input.version, sha256, url: artifact.url, downloader }), - timeouts.fetch, - ); - const mismatch = unpacked.get("MISMATCH"); - if (mismatch !== undefined) { - throw new Error(`The tarball on ${alias} does not match its pinned checksum${mismatch ? "" : " (no checksum tool answered)"}; it was removed.`); - } - if (!unpacked.has("UNPACKED")) throw unpacked.failure(); - const reported = parseVersion(unpacked.get("VERSION")); - if (reported !== input.version) { - throw new Error(`The tarball installed on ${alias} reports version ${reported ?? "nothing"}, not ${input.version}.`); - } - - const dir = reviewRemoteVersionDir(probe.root, input.version); - const result = { nodePath: `${dir}/bin/node`, cliPath: `${dir}/dist/cli.js` }; - const have = prepared.all("HAVE").filter((name) => REVIEW_REMOTE_VERSION.test(name)); - const newest = [input.version, ...have].sort(compareVersions).at(-1); - const finished = await run( - "finishing", - finishScript(context, { - version: input.version, - marker: JSON.stringify({ version: input.version, sha256, node: result.nodePath, cli: result.cliPath, installedAt: Math.floor(Date.now() / 1000) }), - wrapper: newest === input.version ? `#!/bin/sh\n${REVIEW_REMOTE_WRAPPER_MARK}\nexec ${shellQuote(`${dir}/whiteboard`)} "$@"\n` : undefined, - }), - ); - if (!finished.has("COMPLETE")) throw finished.failure(); - - const keep = new Set([input.version, newest]); - const candidates = have.filter((name) => !keep.has(name)).sort(compareVersions).reverse(); - if (candidates.length) { - await run("removing old versions", cleanupScript(context, { candidates, room: 2 - keep.size })).catch((error: unknown) => { - if (signal.aborted) throw error; - }); - } - return result; + const mismatch = says.get("MISMATCH"); + if (mismatch !== undefined) { + throw new Error(`The tarball on ${alias} does not match its pinned checksum${mismatch ? "" : " (no checksum tool answered)"}; it was removed.`); } + const holder = says.get("BUSY"); + if (holder !== undefined) { + throw new Error(`Another install on ${alias} (${holder || "unknown"}) held the lock for ${timeouts.lockWait / 60_000} minutes.`); + } + throw says.failure(); } interface Answer { - has(word: string): boolean; get(word: string): string | undefined; - all(word: string): string[]; failure(): Error; } -function answer(alias: string, what: string, result: RunResult, timeout: number): Answer { +function answer(alias: string, result: RunResult, timeout: number): Answer { const says = result.stdout.split("\n").flatMap((line) => { if (!line.startsWith(`${REVIEW_REMOTE_INSTALL_SAY} `)) return []; const rest = line.slice(REVIEW_REMOTE_INSTALL_SAY.length + 1); const space = rest.indexOf(" "); return [space < 0 ? [rest, ""] : [rest.slice(0, space), rest.slice(space + 1)]]; }); - const all = (word: string) => says.filter(([said]) => said === word).map(([, value]) => value); + const get = (word: string) => says.filter(([said]) => said === word).at(-1)?.[1]; return { - has: (word) => all(word).length > 0, - get: (word) => all(word).at(-1), - all, + get, failure() { - const reason = all("FAIL").at(-1); - if (reason) return new Error(`Installing on ${alias} failed while ${what}: ${reason}.`); - if (result.timedOut) return new Error(`Installing on ${alias} failed: ${what} took longer than ${timeout / 1000} seconds.`); - if (result.error) return new Error(`Installing on ${alias} failed while ${what}: ${result.error.message}`); + const reason = get("FAIL"); + if (reason) return new Error(`Installing on ${alias} failed: ${reason}.`); + if (result.timedOut) return new Error(`Installing on ${alias} failed: it took longer than ${timeout / 1000} seconds.`); + if (result.error) return new Error(`Installing on ${alias} failed: ${result.error.message}`); const stderr = lastLine(result.stderr); - return new Error(`Installing on ${alias} failed while ${what}: exit ${result.code}${stderr ? `: ${stderr}` : ""}.`); + return new Error(`Installing on ${alias} failed: exit ${result.code}${stderr ? `: ${stderr}` : ""}.`); }, }; } @@ -229,51 +161,3 @@ function readMarker(text: string | undefined): ReviewRemoteInstallResult | undef } return undefined; } - -function parseVersion(text: string | undefined): string | undefined { - try { - const value = JSON.parse(text ?? "") as { version?: unknown }; - return typeof value.version === "string" ? value.version : undefined; - } catch { - return undefined; - } -} - -export function compareVersions(a: string, b: string): number { - const [coreA, preA] = split(a); - const [coreB, preB] = split(b); - for (let i = 0; i < 3; i++) if (coreA[i] !== coreB[i]) return coreA[i] - coreB[i]; - if (!preA.length || !preB.length) return preB.length - preA.length; - for (let i = 0; i < Math.max(preA.length, preB.length); i++) { - const x = preA[i]; - const y = preB[i]; - if (x === undefined || y === undefined) return x === undefined ? -1 : 1; - if (x === y) continue; - const nx = /^\d+$/.test(x); - const ny = /^\d+$/.test(y); - if (nx && ny) return Number(x) - Number(y); - if (nx !== ny) return nx ? -1 : 1; - return x < y ? -1 : 1; - } - return 0; -} - -function split(version: string): [number[], string[]] { - const dash = version.indexOf("-"); - const core = (dash < 0 ? version : version.slice(0, dash)).split(".").map(Number); - return [core, dash < 0 ? [] : version.slice(dash + 1).split(".")]; -} - -function delay(ms: number, signal: AbortSignal): Promise { - return new Promise((resolve, reject) => { - const timer = setTimeout(() => { - signal.removeEventListener("abort", stop); - resolve(); - }, ms); - const stop = () => { - clearTimeout(timer); - reject(signal.reason); - }; - signal.addEventListener("abort", stop, { once: true }); - }); -} diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.containers.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.containers.test.ts index 31af1554b..2eee01a30 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.containers.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.containers.test.ts @@ -14,7 +14,8 @@ import { promisify } from "node:util"; import type { SpawnSsh } from "./reviewRemoteHost.js"; import { judgeRemote, parseRemoteProbe, probeRemote, REVIEW_REMOTE_PROBE_TIMEOUT } from "./reviewRemoteProbe.js"; -import { reviewRemoteProbeScript } from "./reviewRemoteProbeScript.js"; +import { remoteInstallScriptInput } from "./reviewRemoteInstallScript.js"; +import { INSTALL_SCRIPT } from "./test/installScript.js"; import { REVIEW_SSH_CONFIG_ENV, reviewSshSession } from "./reviewSshCommand.js"; const run = promisify(execFile); @@ -58,7 +59,7 @@ const RELEASES = "https://install.dev.fast/"; async function probe(host: Host, releases = RELEASES) { const began = Date.now(); - const result = await probeRemote({ session: reviewSshSession(aliasOf(host), controlDirectory, "test"), spawn: realSsh, env: sshEnv(host), releases }); + const result = await probeRemote({ session: reviewSshSession(aliasOf(host), controlDirectory, "test"), spawn: realSsh, env: sshEnv(host), script: INSTALL_SCRIPT, releases }); const elapsed = Date.now() - began; assert.ok("probe" in result, "error" in result ? result.error : ""); assert.ok(elapsed < REVIEW_REMOTE_PROBE_TIMEOUT); @@ -114,7 +115,7 @@ test("the script also runs under bash", { skip }, async () => { { timeout: REVIEW_REMOTE_PROBE_TIMEOUT }, (error, out) => (error ? reject(error) : resolvePromise({ stdout: out })), ); - child.stdin?.end(reviewRemoteProbeScript(RELEASES)); + child.stdin?.end(remoteInstallScriptInput(INSTALL_SCRIPT, ["probe", "--releases", RELEASES])); }); const parsed = parseRemoteProbe(stdout); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.test.ts index def278025..cf6f7f7ac 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.test.ts @@ -12,8 +12,8 @@ import test from "node:test"; import type { SpawnSsh } from "./reviewRemoteHost.js"; import { REVIEW_REMOTE_INSTALL_MARKER, REVIEW_REMOTE_WRAPPER_MARK } from "./reviewRemoteInstallScript.js"; -import { judgeRemote, parseRemoteProbe, probeRemote, type ReviewRemoteProbe } from "./reviewRemoteProbe.js"; -import { REVIEW_REMOTE_PROBE_BEGIN, REVIEW_REMOTE_PROBE_END, REVIEW_REMOTE_PROBE_PATH_CLI } from "./reviewRemoteProbeScript.js"; +import { judgeRemote, parseRemoteProbe, probeRemote, REVIEW_REMOTE_PROBE_BEGIN, REVIEW_REMOTE_PROBE_END, REVIEW_REMOTE_PROBE_PATH_CLI, type ReviewRemoteProbe } from "./reviewRemoteProbe.js"; +import { INSTALL_SCRIPT } from "./test/installScript.js"; import { reviewSshSession } from "./reviewSshCommand.js"; const SHA256 = "a".repeat(64); @@ -111,7 +111,7 @@ test("the script finds the complete installed versions and the host's tools, and const { home, env } = await fakeRemote(t); const before = await tree(home); - const result = await probeRemote({ session, spawn: localShell(env), env: {} }); + const result = await probeRemote({ session, spawn: localShell(env), env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); const { probe } = result; @@ -134,7 +134,7 @@ test("Desktop's wrapper and a link into the install root are not a CLI on PATH: await rm(join(bin, "whiteboard")); await executable(join(local, "whiteboard"), `${REVIEW_REMOTE_WRAPPER_MARK}\necho 0.1.6`); const probe = async () => { - const result = await probeRemote({ session, spawn: localShell(env), env: {} }); + const result = await probeRemote({ session, spawn: localShell(env), env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); return result.probe.pathCli; }; @@ -153,7 +153,7 @@ test("a tree unpacked by hand under the install root is a CLI on PATH; one under await rm(join(home, "bin/whiteboard")); const probe = async (dir: string) => { await executable(join(dir, "whiteboard"), 'if [ "$1" = --version ]; then echo 1.2.3; fi'); - const result = await probeRemote({ session, spawn: localShell({ ...env, PATH: `${dir}:${env.PATH}` }), env: {} }); + const result = await probeRemote({ session, spawn: localShell({ ...env, PATH: `${dir}:${env.PATH}` }), env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); return result.probe.pathCli; }; @@ -170,7 +170,7 @@ test("a home with quotes and backslashes reaches the parser intact", async (t) = const home = join(parent, 'a "quoted" \\ home'); await mkdir(home); - const result = await probeRemote({ session, spawn: localShell({ HOME: home, PATH: "/usr/bin:/bin" }), env: {} }); + const result = await probeRemote({ session, spawn: localShell({ HOME: home, PATH: "/usr/bin:/bin" }), env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); assert.equal(result.probe.home, home); @@ -181,7 +181,7 @@ test("a home that does not exist is refused as unwritable, not as a malformed an t.after(() => rm(parent, { recursive: true, force: true })); const home = join(parent, "missing"); - const result = await probeRemote({ session, spawn: localShell({ HOME: home, PATH: "/usr/bin:/bin" }), env: {} }); + const result = await probeRemote({ session, spawn: localShell({ HOME: home, PATH: "/usr/bin:/bin" }), env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); assert.equal(result.probe.freeBytes, 0); @@ -193,7 +193,7 @@ test("a home that does not exist is refused as unwritable, not as a malformed an test("a probe that does not answer in time is ended and reported", async () => { const started = Date.now(); - const result = await probeRemote({ session, spawn: localShell({}, "/bin/sleep", ["30"]), env: {}, timeout: 300 }); + const result = await probeRemote({ session, spawn: localShell({}, "/bin/sleep", ["30"]), env: {}, script: INSTALL_SCRIPT, timeout: 300 }); assert.deepEqual(result, { error: "The probe of devbox did not answer within 0.3 seconds." }); assert.ok(Date.now() - started < 5000); @@ -201,7 +201,7 @@ test("a probe that does not answer in time is ended and reported", async () => { test("a probe that runs out of time after its answer has no PATH CLI, and is not an error", async () => { const printed = answer({ ...supported, pathCli: undefined }); - const result = await probeRemote({ session, spawn: localShell({}, "/bin/sh", ["-c", `cat >/dev/null; printf '%s' '${printed}'; exec sleep 30`]), env: {}, timeout: 500 }); + const result = await probeRemote({ session, spawn: localShell({}, "/bin/sh", ["-c", `cat >/dev/null; printf '%s' '${printed}'; exec sleep 30`]), env: {}, script: INSTALL_SCRIPT, timeout: 500 }); assert.deepEqual(result, { probe: { ...supported, pathCli: null } }); }); @@ -216,7 +216,7 @@ test("a login shell that ignores SIGTERM is killed in time, and finds no CLI", { await executable(join(home, "bin/curl"), "exit 7"); const started = Date.now(); - const result = await probeRemote({ session, spawn: localShell({ HOME: home, SHELL: shell, PATH: `${join(home, "bin")}:${timeoutDir}:/usr/bin:/bin` }), env: {} }); + const result = await probeRemote({ session, spawn: localShell({ HOME: home, SHELL: shell, PATH: `${join(home, "bin")}:${timeoutDir}:/usr/bin:/bin` }), env: {}, script: INSTALL_SCRIPT }); assert.ok("probe" in result, "error" in result ? result.error : ""); assert.equal(result.probe.pathCli, null); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts index 4f323463d..616ebc918 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts @@ -4,9 +4,13 @@ *--------------------------------------------------------------------------------------------*/ import { lastLine, runSsh, type SpawnSsh } from "./reviewRemoteHost.js"; -import { REVIEW_REMOTE_PROBE_BEGIN, REVIEW_REMOTE_PROBE_END, REVIEW_REMOTE_PROBE_PATH_CLI, reviewRemoteProbeScript } from "./reviewRemoteProbeScript.js"; +import { remoteInstallScriptInput } from "./reviewRemoteInstallScript.js"; import { sshExecArgs, type ReviewSshSession } from "./reviewSshCommand.js"; +export const REVIEW_REMOTE_PROBE_BEGIN = "WHITEBOARD-PROBE-BEGIN"; +export const REVIEW_REMOTE_PROBE_END = "WHITEBOARD-PROBE-END"; +export const REVIEW_REMOTE_PROBE_PATH_CLI = "WHITEBOARD-PROBE-PATH-CLI"; + export interface ReviewRemoteProbe { os: string; arch: string; @@ -66,12 +70,14 @@ export async function probeRemote(input: { session: ReviewSshSession; spawn: SpawnSsh; env: NodeJS.ProcessEnv; + /** packages/review/remote/install.sh. */ + script: string; releases?: string; timeout?: number; }): Promise { const timeout = input.timeout ?? REVIEW_REMOTE_PROBE_TIMEOUT; const alias = input.session.alias; - const result = await runSsh(input.spawn, input.env, sshExecArgs(input.session, input.env), timeout, reviewRemoteProbeScript(input.releases)); + const result = await runSsh(input.spawn, input.env, sshExecArgs(input.session, input.env), timeout, remoteInstallScriptInput(input.script, ["probe", ...(input.releases ? ["--releases", input.releases] : [])])); if (result.error) return { error: `The probe of ${alias} could not start: ${result.error.message}` }; const parsed = parseRemoteProbe(result.stdout); if ("probe" in parsed) return parsed; diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbeScript.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbeScript.ts deleted file mode 100644 index fd0ff73bb..000000000 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbeScript.ts +++ /dev/null @@ -1,79 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) dev.fast. All rights reserved. - * Licensed under the MIT License. See LICENSE in the repository root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { REVIEW_REMOTE_FIND_CLI } from "./reviewRemoteAttachScript.js"; -import { REVIEW_REMOTE_COMPLETE_SHA256, REVIEW_REMOTE_INSTALL_MARKER, REVIEW_REMOTE_ROOT_SCRIPT, REVIEW_REMOTE_WRAPPER_MARK, shellQuote } from "./reviewRemoteInstallScript.js"; - -export const REVIEW_REMOTE_PROBE_BEGIN = "WHITEBOARD-PROBE-BEGIN"; -export const REVIEW_REMOTE_PROBE_END = "WHITEBOARD-PROBE-END"; -export const REVIEW_REMOTE_PROBE_PATH_CLI = "WHITEBOARD-PROBE-PATH-CLI"; - -export const reviewRemoteProbeScript = (releases?: string) => `LC_ALL=C -export LC_ALL -str() { printf '"%s"' "$(printf '%s' "$1" | tr -d '\\000-\\037' | sed 's/\\\\/\\\\\\\\/g; s/"/\\\\"/g')"; } -strOrNull() { if [ -n "$1" ]; then str "$1"; else printf null; fi; } - -os=$(uname -s 2>/dev/null) -arch=$(uname -m 2>/dev/null) -glibc=$(getconf GNU_LIBC_VERSION 2>/dev/null | sed -n 's/^glibc \\([0-9][0-9.]*\\)$/\\1/p') -if [ -z "$glibc" ]; then - glibc=$(ldd --version 2>&1 | head -n 1 | grep -i -e glibc -e 'gnu libc' | sed -n 's/.* \\([0-9][0-9]*\\.[0-9][0-9]*\\)$/\\1/p') -fi -home=$HOME -writable=false -[ -d "$home" ] && [ -w "$home" ] && writable=true -free=$(df -Pk "$home" 2>/dev/null | awk 'NR == 2 { printf "%.0f", $4 * 1024 }') - -${REVIEW_REMOTE_ROOT_SCRIPT}remote=$root - -${REVIEW_REMOTE_COMPLETE_SHA256} -installed= -for dir in "$remote"/versions/*; do - [ -d "$dir" ] || continue - case "$dir" in *.part) continue ;; esac - sha256=$(completeSha256 "$dir/${REVIEW_REMOTE_INSTALL_MARKER}") - [ -n "$sha256" ] || continue - installed="$installed\${installed:+,}{\\"version\\":$(str "\${dir##*/}"),\\"sha256\\":$(str "$sha256")}" -done - -tools= -for tool in tar xz sha256sum openssl; do - command -v "$tool" >/dev/null 2>&1 && tools="$tools\${tools:+,}\\"$tool\\"" -done - -releases=${releases ? shellQuote(releases) : ""} -downloader= -reachable=false -if command -v curl >/dev/null 2>&1; then - downloader=curl - [ -n "$releases" ] && curl -fsI --max-time 3 -o /dev/null "$releases" >/dev/null 2>&1 && reachable=true -elif command -v wget >/dev/null 2>&1; then - downloader=wget - hsts= - tries= - wget --help 2>&1 | grep -q -- --no-hsts && hsts=--no-hsts - wget --help 2>&1 | grep -q -- --tries && tries="-t 1" - limit= - command -v timeout >/dev/null 2>&1 && limit="timeout 3" - [ -n "$releases" ] && $limit wget -q $hsts $tries -T 3 --spider "$releases" >/dev/null 2>&1 && reachable=true -fi - -echo -echo ${REVIEW_REMOTE_PROBE_BEGIN} -printf '{"os":%s,"arch":%s,"glibc":%s,"home":%s,"root":%s,"homeWritable":%s,"freeBytes":%s,' \\ - "$(str "$os")" "$(str "$arch")" "$(strOrNull "$glibc")" "$(str "$home")" "$(str "$remote")" \\ - "$writable" "\${free:-0}" -printf '"installed":[%s],"downloader":%s,"releasesReachable":%s,"tools":[%s]}\\n' \\ - "$installed" "$(strOrNull "$downloader")" "$reachable" "$tools" -echo ${REVIEW_REMOTE_PROBE_END} - -${REVIEW_REMOTE_FIND_CLI}# Desktop's own installs are judged by their markers, not by a version on PATH. -grep -qxF ${shellQuote(REVIEW_REMOTE_WRAPPER_MARK)} "$wb" 2>/dev/null && wb= -case "$(readlink -f "$wb" 2>/dev/null)" in "$(readlink -f "$remote" 2>/dev/null || echo "$remote")"/versions/*) wb= ;; esac -if [ -n "$wb" ]; then - wbVersion=$(PATH="\${wb%/*}:$PATH" DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 DEV_FAST_REVIEW_TELEMETRY_DISABLED=1 bounded 2 "$wb" --version /dev/null | head -n 1) - printf '%s {"path":%s,"version":%s}\\n' ${REVIEW_REMOTE_PROBE_PATH_CLI} "$(str "$wb")" "$(strOrNull "$wbVersion")" -fi -`; diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.test.ts index 1a6962f77..426bf87f9 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.test.ts @@ -14,6 +14,7 @@ import type { SpawnSsh } from "./reviewRemoteHost.js"; import { REVIEW_REMOTE_INSTALL_MARKER } from "./reviewRemoteInstallScript.js"; import { uninstallRemote } from "./reviewRemoteUninstall.js"; import { reviewSshSession } from "./reviewSshCommand.js"; +import { INSTALL_SCRIPT } from "./test/installScript.js"; const localRemote = (home: string): SpawnSsh => @@ -28,9 +29,9 @@ async function fixture(t: test.TestContext) { const dir = join(home, ".dev/whiteboard-remote/versions", name); await mkdir(dir, { recursive: true }); await writeFile(join(dir, "cli.js"), cli); - await writeFile(join(dir, REVIEW_REMOTE_INSTALL_MARKER), JSON.stringify({ version: name, node: process.execPath, cli: join(dir, "cli.js") })); + await writeFile(join(dir, REVIEW_REMOTE_INSTALL_MARKER), JSON.stringify({ version: name, sha256: "0".repeat(64), node: process.execPath, cli: join(dir, "cli.js") })); }; - const uninstall = () => uninstallRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn: localRemote(home), env: {} }); + const uninstall = () => uninstallRemote({ session: reviewSshSession("devbox", tmpdir(), "test"), spawn: localRemote(home), env: {}, script: INSTALL_SCRIPT }); return { home, ran, version, uninstall }; } diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts index ed35cb9db..aa71a95b3 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts @@ -5,53 +5,30 @@ import { plainText } from "./reviewRemoteAgents.js"; import { lastLine, runSsh, type SpawnSsh } from "./reviewRemoteHost.js"; -import { compareVersions } from "./reviewRemoteInstaller.js"; -import { REVIEW_REMOTE_INSTALL_MARKER, REVIEW_REMOTE_INSTALL_SAY, REVIEW_REMOTE_ROOT_SCRIPT, REVIEW_REMOTE_VERSION, shellQuote } from "./reviewRemoteInstallScript.js"; +import { REVIEW_REMOTE_INSTALL_SAY, remoteInstallScriptInput } from "./reviewRemoteInstallScript.js"; import { sshExecArgs, type ReviewSshSession } from "./reviewSshCommand.js"; export const REVIEW_REMOTE_UNINSTALL_TIMEOUT = 60_000; -const LIST = `${REVIEW_REMOTE_ROOT_SCRIPT}printf '%s ROOT %s\\n' ${REVIEW_REMOTE_INSTALL_SAY} "$root" -for d in "$root"/versions/*; do - [ -f "$d/${REVIEW_REMOTE_INSTALL_MARKER}" ] && printf '%s HAVE %s\\n' ${REVIEW_REMOTE_INSTALL_SAY} "\${d##*/}" -done -printf '%s LISTED\\n' ${REVIEW_REMOTE_INSTALL_SAY} -`; - -function uninstallScript(root: string, version: string): string { - return `m=${shellQuote(`${root}/versions/${version}/${REVIEW_REMOTE_INSTALL_MARKER}`)} -node=$(sed -n 's/.*"node":"\\([^"]*\\)".*/\\1/p' "$m") -cli=$(sed -n 's/.*"cli":"\\([^"]*\\)".*/\\1/p' "$m") -DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 "$node" "$cli" remote uninstall --keep-reviews --json { const { session, spawn, env } = input; const timeout = input.timeout ?? REVIEW_REMOTE_UNINSTALL_TIMEOUT; const failed = (detail: string) => new Error(`Could not remove Whiteboard from ${session.alias}: ${detail}`); - - const listed = await runSsh(spawn, env, sshExecArgs(session, env), timeout, LIST); - const said = (word: string) => - listed.stdout - .split("\n") - .filter((line) => line.startsWith(`${REVIEW_REMOTE_INSTALL_SAY} ${word}`)) - .map((line) => line.slice(REVIEW_REMOTE_INSTALL_SAY.length + word.length + 2).trim()); - if (!said("LISTED").length) throw failed(sshProblem(listed)); - const root = said("ROOT")[0]; - if (!root?.startsWith("/")) throw failed("DEV_REVIEW_HOME there is not an absolute, normalised path."); - const newest = said("HAVE") - .filter((name) => REVIEW_REMOTE_VERSION.test(name)) - .sort(compareVersions) - .at(-1); - if (!newest) throw failed("Whiteboard Desktop installed nothing there."); - - const ran = await runSsh(spawn, env, sshExecArgs(session, env), timeout, uninstallScript(root, newest)); + const ran = await runSsh(spawn, env, sshExecArgs(session, env), timeout, remoteInstallScriptInput(input.script, ["uninstall"])); + const refused = ran.stdout + .split("\n") + .filter((line) => line.startsWith(`${REVIEW_REMOTE_INSTALL_SAY} FAIL `)) + .at(-1) + ?.slice(REVIEW_REMOTE_INSTALL_SAY.length + 6); + if (refused) throw failed(`${refused}.`); const result = ran.stdout .split("\n") .map((line) => { diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/fakeSsh.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/fakeSsh.ts index 33bfd6e18..c4f638cb9 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/fakeSsh.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/fakeSsh.ts @@ -9,8 +9,7 @@ import type { AddressInfo } from "node:net"; import { PassThrough } from "node:stream"; import type { TestContext } from "node:test"; import type { ReviewRemoteClock, SpawnSsh, SshChildProcess } from "../reviewRemoteHost.js"; -import { REVIEW_REMOTE_INSTALL_SAY } from "../reviewRemoteInstallScript.js"; -import { REVIEW_REMOTE_PROBE_BEGIN, REVIEW_REMOTE_PROBE_END, REVIEW_REMOTE_PROBE_PATH_CLI } from "../reviewRemoteProbeScript.js"; +import { REVIEW_REMOTE_PROBE_BEGIN, REVIEW_REMOTE_PROBE_END, REVIEW_REMOTE_PROBE_PATH_CLI } from "../reviewRemoteProbe.js"; class FakeChild extends EventEmitter { static nextPid = 1000; @@ -167,7 +166,8 @@ export function fakeSsh(remotes: Record, clock?: { now(): nu } else if (kind === "exec") { const answer = () => { entry.input = child.input; - if (child.input.includes(REVIEW_REMOTE_PROBE_BEGIN)) { + const command = /^set -- '([a-z]+)'/.exec(child.input)?.[1]; + if (command === "probe") { entry.kind = "probe"; return child.finish(0, { stdout: probeOutput(remote.probe) }); } @@ -181,11 +181,7 @@ export function fakeSsh(remotes: Record, clock?: { now(): nu const answer = remote.connect ?? { code: 0, stdout: connectedOutput(child.input) }; return child.finish(answer.code, answer); } - if (child.input.includes(" LISTED\\n")) { - entry.kind = "uninstall"; - return child.finish(0, { stdout: `${REVIEW_REMOTE_INSTALL_SAY} ROOT /home/dev/.dev/whiteboard-remote\n${REVIEW_REMOTE_INSTALL_SAY} HAVE 0.1.6\n${REVIEW_REMOTE_INSTALL_SAY} LISTED\n` }); - } - if (child.input.includes("remote uninstall")) { + if (command === "uninstall") { entry.kind = "uninstall"; const done = () => child.finish(0, { stdout: `${JSON.stringify({ event: "remote.uninstall", ok: remote.uninstall?.ok ?? true, reason: "refused" })}\n` }); return void (remote.uninstall?.after ?? Promise.resolve()).then(done); diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/installScript.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/installScript.ts new file mode 100644 index 000000000..568622070 --- /dev/null +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/test/installScript.ts @@ -0,0 +1,9 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) dev.fast. All rights reserved. + * Licensed under the MIT License. See LICENSE in the repository root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { readFileSync } from "node:fs"; + +/** packages/review/remote/install.sh, as Desktop sends it. */ +export const INSTALL_SCRIPT = readFileSync(new URL("../../../../../../../../../packages/review/remote/install.sh", import.meta.url), "utf8"); 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 47fe9b11d..73567f3f2 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 @@ -22,7 +22,7 @@ import { NullTelemetryService } from "../../platform/telemetry/common/telemetryU import { IUpdateService } from "../../platform/update/common/update.js"; import { UtilityProcess } from "../../platform/utilityProcess/electron-main/utilityProcess.js"; import { IWindowsMainService } from "../../platform/windows/electron-main/windows.js"; -import type { ReviewDesktopConnection } from "../common/reviewDesktopBootstrap.js"; +import { resolveReviewServerEntry, type ReviewDesktopConnection } from "../common/reviewDesktopBootstrap.js"; import type { ReviewGatewayHostState, ReviewRemoteAgent, @@ -60,6 +60,7 @@ import { reviewRemoteInstallConsentPath, } from "./remote/reviewRemoteInstallConsent.js"; import { installRemote } from "./remote/reviewRemoteInstaller.js"; +import { readRemoteInstallScript } from "./remote/reviewRemoteInstallScript.js"; import { createSshAskpass } from "./remote/reviewSshAskpass.js"; import { reviewSshConfigPath, @@ -305,7 +306,8 @@ export class ReviewDesktopHost extends Disposable { } private remoteInstallFlow(): ReviewRemoteInstallFlow { - const { userDataPath } = this.environmentMainService; + const { userDataPath, appRoot, isBuilt } = this.environmentMainService; + let script: Promise | undefined; const artifactsOptions = { pin: this.productService.whiteboardRemote, releases: process.env[REVIEW_REMOTE_RELEASES_ENV], @@ -339,6 +341,15 @@ export class ReviewDesktopHost extends Disposable { origin: () => remoteReleasesOrigin(artifactsOptions), artifact: (target, version) => remoteArtifact(target, version, artifactsOptions), + script: () => + (script ??= readRemoteInstallScript( + resolveReviewServerEntry({ + isBuilt, + appRoot, + serverEntryOverride: process.env["DEV_FAST_REVIEW_SERVER_ENTRY"], + join, + }), + )), }; } diff --git a/apps/review-desktop/scripts/build-remote-tarball.mjs b/apps/review-desktop/scripts/build-remote-tarball.mjs index f3e3a7c0a..b8df8eac9 100644 --- a/apps/review-desktop/scripts/build-remote-tarball.mjs +++ b/apps/review-desktop/scripts/build-remote-tarball.mjs @@ -59,6 +59,7 @@ const PACKAGE_DATA = [ "THIRD_PARTY_NOTICES.md", "onboarding.md", "instructions", + "remote", ]; const reviewRequire = createRequire(path.join(reviewPackage, "package.json")); diff --git a/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs b/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs index a381a2fba..5c3ba9a19 100644 --- a/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs +++ b/apps/review-desktop/scripts/e2e/journeys/remote-install.mjs @@ -662,16 +662,9 @@ async function steps(ctx, until, watch, timings, manifestPath) { assert.equal(declined.state, "not-installed", declined.detail); assert.equal(declined.declined, true); - const denyTarget = { x86_64: "linux-x64", aarch64: "linux-arm64" }[ - await onRemote(deny, "uname -m") - ]; - assert.equal( declined.installCommand, - [ - `mkdir -p ${ROOT} && rm -rf ${ROOT}/whiteboard-remote && curl -fsSL https://install.dev.fast/releases/${version}/remote/whiteboard-remote-${version}-${denyTarget}.tar.xz | tar -xJ -C ${ROOT}`, - "mkdir -p ~/.local/bin && ln -sf ~/.dev/whiteboard-remote/whiteboard-remote/whiteboard ~/.local/bin/whiteboard", - ].join("\n"), + `curl -fsSL https://install.dev.fast/releases/${version}/remote/install.sh | sh -s -- --version ${version}`, ); await denySection .getByRole("button", { name: `Install ${deny}` }) @@ -894,18 +887,14 @@ async function steps(ctx, until, watch, timings, manifestPath) { ); const hand = alias("hand"); - const [unpack, link] = declined.installCommand.split("\n"); - const served = `${releases.url}whiteboard-remote-${next}-linux-x64.tar.xz`; + const base = releases.url.replace(/\/$/, ""); - await onRemote( - hand, - [ - "set -e", - unpack.replace(/curl -fsSL \S+/, `curl -fsSL ${served}`), - link, - ].join("\n"), - { timeout: 5 * MINUTE }, - ); + const handCommand = declined.installCommand + .replaceAll(version, next) + .replace("https://install.dev.fast/releases", base) + .replace("| sh ", `| WHITEBOARD_RELEASES=${base} sh `); + + await onRemote(hand, handCommand, { timeout: 5 * MINUTE }); assert.equal(await onRemote(hand, "bash -lc 'whiteboard --version'"), next); const handAdded = Date.now(); @@ -935,12 +924,12 @@ async function steps(ctx, until, watch, timings, manifestPath) { assert.equal(handSettled.state, "online", handSettled.detail); assert.equal( - await onRemote(hand, `ls -d ${ROOT}/versions 2>/dev/null || true`), - "", + await onRemote(hand, `ls ${ROOT}/versions`), + next, ); assert.match( - await onRemote(hand, `readlink ${WHITEBOARD}`), - /\/\.dev\/whiteboard-remote\/whiteboard-remote\/whiteboard$/, + await onRemote(hand, `cat ${WHITEBOARD}`), + new RegExp(`/versions/${next.replaceAll(".", "\\.")}/whiteboard`), ); const handServer = JSON.parse( @@ -953,11 +942,11 @@ async function steps(ctx, until, watch, timings, manifestPath) { hand, `cat /proc/${handServer.serverPid}/cmdline | tr '\\0' ' '`, ), - /\/\.dev\/whiteboard-remote\/whiteboard-remote\//, + new RegExp(`/versions/${next.replaceAll(".", "\\.")}/`), ); assert.deepEqual(await promptsFor(ctx, hand), []); ctx.check( - `11. ${hand}: ${served} unpacked and linked by Settings' two lines; online without a question, its server runs from the unpacked tree, no versions/ was written and ${WHITEBOARD} is still the hand symlink`, + `11. ${hand}: Settings' install.sh line installed ${next}; online without a question, as Desktop's own install, and its server runs from versions/${next}`, ); ctx.check( diff --git a/apps/review-desktop/scripts/e2e/remote/releases.mjs b/apps/review-desktop/scripts/e2e/remote/releases.mjs index b02eec0b0..740dfab8d 100644 --- a/apps/review-desktop/scripts/e2e/remote/releases.mjs +++ b/apps/review-desktop/scripts/e2e/remote/releases.mjs @@ -1,9 +1,10 @@ -/** This checkout's remote tarballs, served over plain HTTP as install.dev.fast serves a release's, for DEV_FAST_REVIEW_REMOTE_RELEASES. */ +/** This checkout's remote tarballs and install.sh, served over plain HTTP as install.dev.fast serves a release's: flat for DEV_FAST_REVIEW_REMOTE_RELEASES, and under /remote/ for a hand install's WHITEBOARD_RELEASES. */ import { createReadStream } from "node:fs"; import { copyFile, mkdtemp, rm, stat } from "node:fs/promises"; import { createServer } from "node:http"; import { networkInterfaces, tmpdir } from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; import { buildDevRemoteTarball } from "../../build-remote-tarball.mjs"; @@ -24,8 +25,13 @@ function laptopAddress() { return address; } +const INSTALL_SCRIPT = fileURLToPath( + new URL("../../../../../packages/review/remote/install.sh", import.meta.url), +); + export async function serveReleases() { const dir = await mkdtemp(path.join(tmpdir(), "wb-releases-")); + await copyFile(INSTALL_SCRIPT, path.join(dir, "install.sh")); const server = createServer(async (request, response) => { let name; @@ -40,8 +46,7 @@ export async function serveReleases() { const found = name === "" || - (name === path.basename(name) && - (await stat(file).then((s) => s.isFile(), () => false))); + (await stat(file).then((s) => s.isFile(), () => false)); response.writeHead(found ? 200 : 404); diff --git a/apps/review-desktop/scripts/remote-dev-install.mjs b/apps/review-desktop/scripts/remote-dev-install.mjs index be3dcf77e..0eff0c460 100644 --- a/apps/review-desktop/scripts/remote-dev-install.mjs +++ b/apps/review-desktop/scripts/remote-dev-install.mjs @@ -14,6 +14,10 @@ import { const ROOT = "~/.dev/whiteboard-remote"; +const INSTALL_SCRIPT = fileURLToPath( + new URL("../../../packages/review/remote/install.sh", import.meta.url), +); + const BUCKET = "dev-fast-desktop-artifacts"; const RELEASES = "https://install.dev.fast/releases"; @@ -44,7 +48,7 @@ function hostTarget(alias, { sshConfig } = {}) { return target; } -/** Unpacks this checkout's tarball in ~/.dev/whiteboard-remote, links ~/.local/bin/whiteboard, and returns its `whiteboard version --json`; `commit` rewrites its VS Code server's commit. */ +/** Installs this checkout's tarball with packages/review/remote/install.sh, as Desktop does, and returns its `whiteboard version --json`; `commit` rewrites its VS Code server's commit. */ export async function remoteDevInstall( alias, { target, sshConfig, version, commit, stdio = "inherit" } = {}, @@ -53,12 +57,7 @@ export async function remoteDevInstall( const built = await buildDevRemoteTarball({ target, version, stdio }); const copy = `/tmp/whiteboard-remote-${randomBytes(4).toString("hex")}.tar.xz`; - - const removeCopy = () => { - try { - ssh(alias, sshConfig, `rm -f ${copy}`); - } catch {} - }; + const installed = `${ROOT}/versions/${built.version}`; try { execFileSync( @@ -67,28 +66,34 @@ export async function remoteDevInstall( { stdio: ["ignore", "inherit", "inherit"] }, ); + const args = ["install", "--version", built.version, "--sha256", built.sha256, "--file", copy]; + + const said = execFileSync("ssh", [...sshArgs(sshConfig), alias, "sh", "-s"], { + encoding: "utf8", + input: `set -- ${args.map((arg) => `'${arg}'`).join(" ")}\n${await readFile(INSTALL_SCRIPT, "utf8")}`, + stdio: ["pipe", "pipe", "inherit"], + }); + + if (!/^WHITEBOARD-INSTALL DONE /m.test(said)) + throw new Error(`install.sh did not install on ${alias}:\n${said.trim()}`); + return ssh( alias, sshConfig, [ "set -e", - `trap 'rm -f ${copy}' EXIT`, - `mkdir -p ${ROOT}`, - `rm -rf ${ROOT}/whiteboard-remote`, - `tar -xJ -C ${ROOT} < ${copy}`, ...(commit ? [ - `sed -i 's/"commit": "[0-9a-f]*"/"commit": "${commit}"/' ${ROOT}/whiteboard-remote/vscode-server/product.json`, + `sed -i 's/"commit": "[0-9a-f]*"/"commit": "${commit}"/' ${installed}/vscode-server/product.json`, ] : []), - "mkdir -p ~/.local/bin", - `ln -sf ${ROOT}/whiteboard-remote/whiteboard ~/.local/bin/whiteboard`, - "~/.local/bin/whiteboard version --json", + `${installed}/whiteboard version --json`, ].join("\n"), ); - } catch (error) { - removeCopy(); - throw error; + } finally { + try { + ssh(alias, sshConfig, `rm -f ${copy}`); + } catch {} } } diff --git a/apps/review-desktop/scripts/stage-review-runtime.mjs b/apps/review-desktop/scripts/stage-review-runtime.mjs index a1a43ce29..e5a854fde 100644 --- a/apps/review-desktop/scripts/stage-review-runtime.mjs +++ b/apps/review-desktop/scripts/stage-review-runtime.mjs @@ -49,6 +49,7 @@ export const REQUIRED_RUNTIME_ENTRIES = [ // The build's commit; without it the server reports `commit: null`. "dist/build-info.json", "instructions/authoring.md", + "remote/install.sh", "node_modules", ]; diff --git a/docs/remote-hosts.md b/docs/remote-hosts.md index 43ed541a2..49dd484a3 100644 --- a/docs/remote-hosts.md +++ b/docs/remote-hosts.md @@ -59,6 +59,7 @@ never stored. Desktop first checks the host and writes nothing. If the host lacks Desktop's version, Desktop asks once, then installs it as your user. The host downloads it from `install.dev.fast` and checks its pinned checksum before unpacking. +Desktop runs the same `install.sh` you can run by hand. You do not need to start anything. Desktop starts the review server when it connects. On a remote with no Desktop, `whiteboard api` starts one too, so an @@ -77,7 +78,8 @@ instead of `~/.dev`. The review server keeps reviews in `~/.dev`. ### Updates -When Desktop updates, it installs its new version without asking. It restarts +When Desktop updates, it installs its new version without asking, also over a +hand install. It restarts the review server it started. It keeps the previous version and any version still running. It leaves a server you started yourself running. @@ -97,11 +99,10 @@ not have its version: On the remote, with `` set to Desktop's version: ```sh -mkdir -p ~/.dev/whiteboard-remote && rm -rf ~/.dev/whiteboard-remote/whiteboard-remote && curl -fsSL https://install.dev.fast/releases//remote/whiteboard-remote--linux-x64.tar.xz | tar -xJ -C ~/.dev/whiteboard-remote -mkdir -p ~/.local/bin && ln -sf ~/.dev/whiteboard-remote/whiteboard-remote/whiteboard ~/.local/bin/whiteboard +curl -fsSL https://install.dev.fast/releases//remote/install.sh | sh -s -- --version ``` -On ARM64, use `linux-arm64` in place of `linux-x64`. +It installs where Desktop does, so Desktop finds it. ### Developing diff --git a/packages/review-protocol/src/contracts.ts b/packages/review-protocol/src/contracts.ts index 6da1c4d3a..e0020c251 100644 --- a/packages/review-protocol/src/contracts.ts +++ b/packages/review-protocol/src/contracts.ts @@ -912,7 +912,6 @@ export const ReviewGatewayHostSchema = z.strictObject({ declined: z.literal(true).optional(), asking: requiredString.optional(), installFailure: stringAllowEmpty.optional(), - target: z.enum(["linux-x64", "linux-arm64"]).optional(), installing: z .strictObject({ step: z.enum(REVIEW_REMOTE_INSTALL_STEPS), diff --git a/packages/review/package.json b/packages/review/package.json index 2790b98b8..4636b4f7b 100644 --- a/packages/review/package.json +++ b/packages/review/package.json @@ -18,6 +18,7 @@ "onboarding.md", "dist", "instructions", + "remote", "src", "!src/**/*.test.ts", "!src/**/*test-utils*", diff --git a/packages/review/remote/install.sh b/packages/review/remote/install.sh new file mode 100755 index 000000000..a7f8a3513 --- /dev/null +++ b/packages/review/remote/install.sh @@ -0,0 +1,440 @@ +#!/bin/sh +# Installs Whiteboard's remote CLI on a Linux host. Desktop pipes this file over ssh; +# the same file is published next to each release's tarballs for hand installs: +# curl -fsSL https://install.dev.fast/releases//remote/install.sh | sh -s -- --version +# +# install --version V [--sha256 H (--url U | --file F)] [--downloader curl|wget] [--owner NAME] [--token HEX] +# [--lock-wait S] [--lock-poll S] [--lock-stale S] (the default command) +# probe [--releases URL] prints what Desktop needs to judge the host; writes nothing +# release --token HEX drops the install lock if that install still holds it +# uninstall runs the newest installed version's `whiteboard remote uninstall` +# +# Older and newer Desktops and `whiteboard remote uninstall` share this layout; change it +# only with a migration: +# $root/versions// one version, complete once .whiteboard-install.json names +# its sha256, an executable node and its cli +# $root/install.lock/ token, owner and started (epoch seconds); stale after 15 minutes +# ~/.local/bin/whiteboard launcher for the newest version, replaced only when it carries +# the mark below or is the old hand-install symlink +# $root is $DEV_REVIEW_HOME/whiteboard-remote, or ~/.dev/whiteboard-remote. +# Desktop reads stdout lines "WHITEBOARD-INSTALL ". + +LC_ALL=C +export LC_ALL +umask 022 +trap '' PIPE +exec 3>&1 + +MARKER=.whiteboard-install.json +MARK='# Written by Whiteboard Desktop, which replaces it with each install.' +RELEASES=${WHITEBOARD_RELEASES:-https://install.dev.fast/releases} +hand= + +say() { [ -n "$hand" ] || printf '\n%s %s\n' WHITEBOARD-INSTALL "$*" >&3; } +tell() { [ -z "$hand" ] || printf '%s\n' "$*" >&2; } +fail() { + say FAIL "$*" + tell "Whiteboard was not installed: $*" + exit 3 +} +q() { printf "'%s'" "$(printf '%s' "$1" | sed "s/'/'\\\\''/g")"; } + +base=$(printf '%s' "${DEV_REVIEW_HOME-}" | sed 's/^[[:space:]]*//; s/[[:space:]]*$//') +root= +case "$base" in +'') root=$HOME/.dev/whiteboard-remote ;; +*//* | */./* | */../* | */. | */..) ;; +/*) root=${base%/}/whiteboard-remote ;; +esac +[ "$(printf '%s' "${DEV_REVIEW_HOME-}" | tr -d '\001-\037\177')" = "${DEV_REVIEW_HOME-}" ] || root= +lock=$root/install.lock + +completeSha256() { + cs=$(sed -n 's/.*"sha256":"\([^"][^"]*\)".*/\1/p' "$1" 2>/dev/null) + cn=$(sed -n 's/.*"node":"\([^"]*\)".*/\1/p' "$1" 2>/dev/null) + cc=$(sed -n 's/.*"cli":"\([^"]*\)".*/\1/p' "$1" 2>/dev/null) + [ -n "$cs" ] && [ -x "$cn" ] && [ -f "$cc" ] && printf '%s' "$cs" +} + +# Semver precedence, newest first. +newestFirst() { + awk ' + function core(v, i) { i = index(v, "-"); return i ? substr(v, 1, i - 1) : v } + function pre(v, i) { i = index(v, "-"); return i ? substr(v, i + 1) : "" } + function cmp(a, b, x, y, n, m, i, p, r, nx, ny) { + split(core(a), x, "."); split(core(b), y, ".") + for (i = 1; i <= 3; i++) if (x[i] + 0 != y[i] + 0) return x[i] + 0 < y[i] + 0 ? -1 : 1 + p = pre(a); r = pre(b) + if (p == "" || r == "") return p == r ? 0 : (p == "" ? 1 : -1) + n = split(p, x, "."); m = split(r, y, ".") + for (i = 1; i <= n && i <= m; i++) { + if (x[i] == y[i]) continue + nx = x[i] ~ /^[0-9]+$/; ny = y[i] ~ /^[0-9]+$/ + if (nx && ny) return x[i] + 0 < y[i] + 0 ? -1 : 1 + if (nx != ny) return nx ? -1 : 1 + return x[i] < y[i] ? -1 : 1 + } + return n == m ? 0 : (n < m ? -1 : 1) + } + NF { v[++c] = $0; for (j = c; j > 1 && cmp(v[j], v[j - 1]) > 0; j--) { t = v[j]; v[j] = v[j - 1]; v[j - 1] = t } } + END { for (j = 1; j <= c; j++) print v[j] }' +} + +bounded() { + if timeout -k 1 1 true >/dev/null 2>&1; then timeout -k 1 "$@"; else shift; "$@"; fi +} + +probe() { + releases= + while [ $# -gt 0 ]; do + case "$1" in + --releases) releases=$2; shift 2 ;; + *) shift ;; + esac + done + str() { printf '"%s"' "$(printf '%s' "$1" | tr -d '\000-\037' | sed 's/\\/\\\\/g; s/"/\\"/g')"; } + strOrNull() { if [ -n "$1" ]; then str "$1"; else printf null; fi; } + os=$(uname -s 2>/dev/null) + arch=$(uname -m 2>/dev/null) + glibc=$(getconf GNU_LIBC_VERSION 2>/dev/null | sed -n 's/^glibc \([0-9][0-9.]*\)$/\1/p') + if [ -z "$glibc" ]; then + glibc=$(ldd --version 2>&1 | head -n 1 | grep -i -e glibc -e 'gnu libc' | sed -n 's/.* \([0-9][0-9]*\.[0-9][0-9]*\)$/\1/p') + fi + writable=false + [ -d "$HOME" ] && [ -w "$HOME" ] && writable=true + free=$(df -Pk "$HOME" 2>/dev/null | awk 'NR == 2 { printf "%.0f", $4 * 1024 }') + installed= + for dir in "$root"/versions/*; do + [ -d "$dir" ] || continue + case "$dir" in *.part) continue ;; esac + sha256=$(completeSha256 "$dir/$MARKER") + [ -n "$sha256" ] || continue + installed="$installed${installed:+,}{\"version\":$(str "${dir##*/}"),\"sha256\":$(str "$sha256")}" + done + tools= + for tool in tar xz sha256sum openssl; do + command -v "$tool" >/dev/null 2>&1 && tools="$tools${tools:+,}\"$tool\"" + done + downloader= + reachable=false + if command -v curl >/dev/null 2>&1; then + downloader=curl + [ -n "$releases" ] && curl -fsI --max-time 3 -o /dev/null "$releases" >/dev/null 2>&1 && reachable=true + elif command -v wget >/dev/null 2>&1; then + downloader=wget + hsts= + tries= + wget --help 2>&1 | grep -q -- --no-hsts && hsts=--no-hsts + wget --help 2>&1 | grep -q -- --tries && tries="-t 1" + limit= + command -v timeout >/dev/null 2>&1 && limit="timeout 3" + # shellcheck disable=SC2086 # empty options must vanish + [ -n "$releases" ] && $limit wget -q $hsts $tries -T 3 --spider "$releases" >/dev/null 2>&1 && reachable=true + fi + echo + echo WHITEBOARD-PROBE-BEGIN + printf '{"os":%s,"arch":%s,"glibc":%s,"home":%s,"root":%s,"homeWritable":%s,"freeBytes":%s,' \ + "$(str "$os")" "$(str "$arch")" "$(strOrNull "$glibc")" "$(str "$HOME")" "$(str "$root")" \ + "$writable" "${free:-0}" + printf '"installed":[%s],"downloader":%s,"releasesReachable":%s,"tools":[%s]}\n' \ + "$installed" "$(strOrNull "$downloader")" "$reachable" "$tools" + echo WHITEBOARD-PROBE-END + + # The same lookup as Desktop's attach script. + wb=$(command -v whiteboard 2>/dev/null) + case "$wb" in /*) ;; *) wb= ;; esac + if [ -z "$wb" ] && [ -x "$HOME/.local/bin/whiteboard" ]; then wb="$HOME/.local/bin/whiteboard"; fi + if [ -z "$wb" ] && [ -n "$SHELL" ]; then + wb=$(bounded 3 "$SHELL" -lic 'command -v whiteboard' /dev/null | tr -d '\r' | grep '^/.*/whiteboard$' | tail -n 1) + fi + [ -n "$wb" ] && [ -x "$wb" ] || wb= + # Desktop's own installs are judged by their markers, not by a version on PATH. + grep -qxF "$MARK" "$wb" 2>/dev/null && wb= + case "$(readlink -f "$wb" 2>/dev/null)" in "$(readlink -f "$root" 2>/dev/null || echo "$root")"/versions/*) wb= ;; esac + if [ -n "$wb" ]; then + wbVersion=$(PATH="${wb%/*}:$PATH" DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 DEV_FAST_REVIEW_TELEMETRY_DISABLED=1 bounded 2 "$wb" --version /dev/null | head -n 1) + printf '%s {"path":%s,"version":%s}\n' WHITEBOARD-PROBE-PATH-CLI "$(str "$wb")" "$(strOrNull "$wbVersion")" + fi +} + +# A rename, so a reader never sees "started" half written. +stamp() { date +%s >"$lock/started.$token" && mv -f "$lock/started.$token" "$lock/started"; } +own() { + [ "$(cat "$lock/token" 2>/dev/null)" = "$token" ] || fail this install no longer holds the install lock + stamp +} +release() { + if [ "$(cat "$lock/token" 2>/dev/null)" = "$token" ] && mv "$lock" "$lock.$token.done" 2>/dev/null; then + rm -rf "$lock.$token.done" + fi +} +# Runs a long step, keeping the lock fresh; a heartbeat that cannot be written means Desktop +# went away, so the step is stopped. +guard() { + if command -v setsid >/dev/null 2>&1; then setsid "$@" & else "$@" & fi + pid=$! + while kill -0 "$pid" 2>/dev/null; do + printf '.\n' >&3 2>/dev/null || { + kill -TERM -"$pid" 2>/dev/null || kill "$pid" 2>/dev/null + exit 3 + } + [ "$(cat "$lock/token" 2>/dev/null)" = "$token" ] && stamp + sleep 1 + done + wait "$pid" +} + +# 0 when this install took the lock, 1 when another holds it. +takeLock() { + if mkdir "$lock" 2>/dev/null; then + printf '%s\n' "$token" >"$lock/token" + printf '%s\n' "$owner" >"$lock/owner" + stamp + return 0 + fi + # The token first: a takeover completed between the two reads leaves "started" fresh. + held=$(cat "$lock/token" 2>/dev/null) + started=$(cat "$lock/started" 2>/dev/null) + case "$started" in + '' | *[!0-9]*) + # Never written (a holder that died right after mkdir): by the directory's age. Unreadable: fresh. + if [ ! -e "$lock/started" ] && [ -n "$(find "$lock" -prune -mmin +"$(((lockStale + 59) / 60))" 2>/dev/null)" ]; then started=0; else started=$(date +%s); fi + ;; + esac + [ $(($(date +%s) - started)) -ge "$lockStale" ] || return 1 + stale="$lock.$token.stale" + if mv "$lock" "$stale" 2>/dev/null; then + # Another install may have taken it over first: give that one back. + if [ "$(cat "$stale/token" 2>/dev/null)" = "$held" ] || [ -e "$lock" ]; then rm -rf "$stale"; else mv "$stale" "$lock"; fi + fi + mkdir "$lock" 2>/dev/null || return 1 + printf '%s\n' "$token" >"$lock/token" + printf '%s\n' "$owner" >"$lock/owner" + stamp +} + +fetch() { + if [ "$downloader" = curl ]; then + guard curl -fsSL --connect-timeout 20 --max-time 900 -o "$2" "$1" + else + hsts= + wget --help 2>&1 | grep -q -- --no-hsts && hsts=--no-hsts + limit= + command -v timeout >/dev/null 2>&1 && limit="timeout 900" + # shellcheck disable=SC2086 # empty options must vanish + guard $limit wget -q $hsts -t 2 -T 30 -O "$2" "$1" + fi +} + +install() { + downloader= + version= + sha256= + url= + file= + owner= + token= + lockWait=900 + lockPoll=2 + lockStale=900 + while [ $# -gt 0 ]; do + case "$1" in + --version) version=$2 ;; + --sha256) sha256=$2 ;; + --url) url=$2 ;; + --file) file=$2 ;; + --downloader) downloader=$2 ;; + --owner) owner=$2 ;; + --token) token=$2 ;; + --lock-wait) lockWait=$2 ;; + --lock-poll) lockPoll=$2 ;; + --lock-stale) lockStale=$2 ;; + *) fail "unknown option $1" ;; + esac + shift 2 + done + [ -n "$url$file" ] || hand=1 + printf '%s\n' "$version" | grep -Eqx '[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?' || fail "--version needs a version such as 1.2.3, not \"$version\"" + [ -n "$root" ] || fail DEV_REVIEW_HOME is not an absolute, normalised path + case "$root" in *[\"\\]*) fail "$root holds a quote or backslash" ;; esac + [ -n "$token" ] || token=$(od -An -N8 -tx1 /dev/urandom | tr -d ' \n') + printf '%s\n' "$token" | grep -Eqx '[0-9a-f]{8,64}' || fail the install token is not hex + [ -n "$owner" ] || owner=$(hostname 2>/dev/null || echo unknown) + + [ -n "$downloader" ] || ! command -v curl >/dev/null 2>&1 || downloader=curl + [ -n "$downloader" ] || ! command -v wget >/dev/null 2>&1 || downloader=wget + case "$downloader" in '' | curl | wget) ;; *) fail "--downloader is curl or wget, not $downloader" ;; esac + if [ -n "$hand" ]; then + [ "$(uname -s)" = Linux ] || fail "this host runs $(uname -s); Whiteboard needs Linux" + case "$(uname -m)" in + x86_64) target=linux-x64 ;; + aarch64) target=linux-arm64 ;; + *) fail "this host's CPU is $(uname -m); Whiteboard needs x86_64 or aarch64" ;; + esac + [ -n "$downloader" ] || fail Whiteboard needs curl or wget + url="$RELEASES/$version/remote/whiteboard-remote-$version-$target.tar.xz" + if [ "$downloader" = curl ]; then sha256=$(curl -fsSL "$url.sha256"); else sha256=$(wget -qO- "$url.sha256"); fi + sha256=${sha256%% *} + fi + printf '%s\n' "$sha256" | grep -Eqx '[0-9a-f]{64}' || fail "no sha256 for $version" + + v=$root/versions/$version + m=$v/$MARKER + if [ "$(completeSha256 "$m")" = "$sha256" ]; then + say DONE "$(cat "$m")" + tell "Whiteboard $version is already installed." + exit 0 + fi + [ -n "$file$downloader" ] || fail Whiteboard needs curl or wget + + mkdir -p "$root" || fail cannot create "$root" + polls=$(awk -v w="$lockWait" -v p="$lockPoll" 'BEGIN { n = w / p; printf "%d", (n == int(n) ? n : int(n) + 1) }') + told= + while ! takeLock; do + if [ "$polls" -le 0 ]; then + say BUSY "$(cat "$lock/owner" 2>/dev/null)" + tell "Another install ($(cat "$lock/owner" 2>/dev/null)) holds $lock." + exit 5 + fi + [ -n "$told" ] || say STEP waiting-for-lock + told=1 + polls=$((polls - 1)) + sleep "$lockPoll" + done + trap release EXIT + trap 'exit 3' HUP INT TERM + + rm -rf "$root"/versions/*.part "$lock".*.stale "$lock".*.done + mkdir -p "$root/versions" || fail cannot create "$root/versions" + if [ "$(completeSha256 "$m")" = "$sha256" ]; then + say DONE "$(cat "$m")" + exit 0 + fi + p=$v.$token.part + if [ -e "$v" ] && ! { mv "$v" "$p" && rm -rf "$p"; }; then fail cannot remove "$v"; fi + have= + for dir in "$root"/versions/*; do + [ -n "$(completeSha256 "$dir/$MARKER")" ] && have="$have${dir##*/} +" && continue + # A version whose removal was cut short (its marker went first). + case "${dir##*/}" in [0-9]*.[0-9]*.[0-9]*) rm -rf "$dir" ;; esac + done + mkdir "$p" || fail cannot create "$p" + + say STEP package + tell "Installing Whiteboard $version in $v" + f=$p/whiteboard-remote.tar.xz + if [ -n "$file" ]; then + cp "$file" "$f" || fail cannot read "$file" + else + own + fetch "$url" "$f" || { + s=$? + rm -rf "$p" + fail cannot download "$url": "$downloader" exited with "$s" + } + fi + sum=$(sha256sum "$f" 2>/dev/null || openssl dgst -sha256 -r "$f" 2>/dev/null) + sum=${sum%% *} + if [ "$sum" != "$sha256" ]; then + rm -rf "$p" + say MISMATCH "$sum" + tell "The tarball does not match its checksum; it was removed." + exit 4 + fi + own + guard tar -xJf "$f" -C "$p" --strip-components=1 || { + rm -rf "$p" + fail cannot unpack the tarball + } + rm -f "$f" + [ -x "$p/bin/node" ] && [ -f "$p/dist/cli.js" ] && [ -x "$p/whiteboard" ] || { + rm -rf "$p" + fail the tarball holds no whiteboard + } + out=$(DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 DEV_FAST_REVIEW_TELEMETRY_DISABLED=1 "$p/bin/node" "$p/dist/cli.js" version --json /dev/null | tail -n 1) + reported=$(printf '%s' "$out" | sed -n 's/.*"version":"\([^"]*\)".*/\1/p') + [ "$reported" = "$version" ] || { + rm -rf "$p" + fail "the tarball reports version ${reported:-nothing}, not $version" + } + + own + printf '{"version":"%s","sha256":"%s","node":"%s","cli":"%s","installedAt":%s}\n' \ + "$version" "$sha256" "$v/bin/node" "$v/dist/cli.js" "$(date +%s)" >"$p/$MARKER" || fail cannot write the marker + [ -e "$v" ] && { + rm -rf "$p" + fail "$v appeared during the install" + } + mv "$p" "$v" || fail cannot move the version into place + + newest=$(printf '%s%s\n' "$have" "$version" | newestFirst | head -n 1) + if [ "$newest" = "$version" ]; then + w=$HOME/.local/bin/whiteboard + old=$root/whiteboard-remote + adopt= + [ -L "$w" ] && [ "$(readlink "$w")" = "$old/whiteboard" ] && adopt=1 + if [ -n "$adopt" ] || { [ ! -e "$w" ] && [ ! -L "$w" ]; } || { [ -f "$w" ] && [ ! -L "$w" ] && grep -qxF "$MARK" "$w"; }; then + if mkdir -p "${w%/*}" && printf '#!/bin/sh\n%s\nexec %s "$@"\n' "$MARK" "$(q "$v/whiteboard")" >"$w.$token.part" && chmod 755 "$w.$token.part" && mv -f "$w.$token.part" "$w"; then + say LAUNCHER written + tell "Linked $w" + else + rm -f "$w.$token.part" + say LAUNCHER failed + fi + else + say LAUNCHER foreign + tell "Left $w alone: Whiteboard did not write it. Run $v/whiteboard, or put it on your PATH." + fi + fi + + # Read once so a grep below never finds itself; launchers run the CLI by absolute path. + if [ -d /proc/self ]; then + procs=$(for f in /proc/[0-9]*/cmdline; do tr '\000' ' ' <"$f" 2>/dev/null; echo; done) + else + procs=$(ps -eo args= 2>/dev/null) + fi + running() { printf '%s\n' "$procs" | grep -qF -- "$1/"; } + [ -n "$adopt" ] && ! running "$old" && rm -rf "$old" + room=1 + [ "$newest" = "$version" ] || room=0 + candidates=$(printf '%s' "$have" | grep -vxF -e "$version" -e "$newest" | newestFirst) + kept=0 + for c in $candidates; do running "$root/versions/$c" && kept=$((kept + 1)); done + for c in $candidates; do + if running "$root/versions/$c"; then + say IN-USE "$c" + continue + fi + if [ "$kept" -lt "$room" ]; then + kept=$((kept + 1)) + continue + fi + d=$root/versions/$c + [ -f "$d/$MARKER" ] || continue + rm -f "$d/$MARKER" && rm -rf "$d" && say REMOVED "$c" + done + + say DONE "$(cat "$m")" + tell "Whiteboard $version is installed. Run: whiteboard --version" +} + +uninstall() { + [ -n "$root" ] || fail DEV_REVIEW_HOME there is not an absolute, normalised path + newest=$(for dir in "$root"/versions/*; do [ -n "$(completeSha256 "$dir/$MARKER")" ] && printf '%s\n' "${dir##*/}"; done | + grep -Ex '[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?' | newestFirst | head -n 1) + [ -n "$newest" ] || fail Whiteboard Desktop installed nothing there + completeSha256 "$root/versions/$newest/$MARKER" >/dev/null + DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 exec "$cn" "$cc" remote uninstall --keep-reviews --json - [ - `mkdir -p ~/.dev/whiteboard-remote && rm -rf ~/.dev/whiteboard-remote/whiteboard-remote && curl -fsSL https://install.dev.fast/releases/${v}/remote/whiteboard-remote-${v}-${target}.tar.xz | tar -xJ -C ~/.dev/whiteboard-remote`, - "mkdir -p ~/.local/bin && ln -sf ~/.dev/whiteboard-remote/whiteboard-remote/whiteboard ~/.local/bin/whiteboard", - ].join("\n"); +const handInstall = (v: string) => + `curl -fsSL https://install.dev.fast/releases/${v}/remote/install.sh | sh -s -- --version ${v}`; let root: string; @@ -88,7 +85,7 @@ it("refuses a host on another version, naming both versions, with the install co expect(gateway.online()).toEqual([]); }); -it("names this machine's install command for a host without Whiteboard, for the host's arch", () => { +it("names this machine's install command for a host without Whiteboard", () => { const gateway = hosts("0.1.6"); gateway.set([ @@ -99,14 +96,6 @@ it("names this machine's install command for a host without Whiteboard, for the detail: "Whiteboard is not installed on devbox.", }, }, - { - alias: "graviton", - target: "linux-arm64", - problem: { - state: "not-installed", - detail: "Whiteboard is not installed on graviton.", - }, - }, { alias: "other", problem: { state: "auth-failed", detail: "Permission denied." }, @@ -120,12 +109,6 @@ it("names this machine's install command for a host without Whiteboard, for the detail: "Whiteboard is not installed on devbox.", installCommand: handInstall("0.1.6"), }, - { - alias: "graviton", - state: "not-installed", - detail: "Whiteboard is not installed on graviton.", - installCommand: handInstall("0.1.6", "linux-arm64"), - }, { alias: "other", state: "auth-failed", detail: "Permission denied." }, ]); }); diff --git a/packages/review/src/server/review-gateway-hosts.ts b/packages/review/src/server/review-gateway-hosts.ts index 7a75747f2..2839d7a3f 100644 --- a/packages/review/src/server/review-gateway-hosts.ts +++ b/packages/review/src/server/review-gateway-hosts.ts @@ -60,7 +60,6 @@ interface Host extends GatewayRemote { declined?: true; asking?: string; installFailure?: string; - target?: ReviewGatewayHost["target"]; serverId?: string; status: ReviewGatewayHostState["state"]; detail?: string; @@ -123,10 +122,7 @@ export function createGatewayHosts(input: { ...(detail !== undefined && { detail }), ...(INSTALLS.has(host.status) && host.problem?.state !== "incompatible" && { - installCommand: [ - `mkdir -p ~/.dev/whiteboard-remote && rm -rf ~/.dev/whiteboard-remote/whiteboard-remote && curl -fsSL https://install.dev.fast/releases/${input.version}/remote/whiteboard-remote-${input.version}-${host.target ?? "linux-x64"}.tar.xz | tar -xJ -C ~/.dev/whiteboard-remote`, - "mkdir -p ~/.local/bin && ln -sf ~/.dev/whiteboard-remote/whiteboard-remote/whiteboard ~/.local/bin/whiteboard", - ].join("\n"), + installCommand: `curl -fsSL https://install.dev.fast/releases/${input.version}/remote/install.sh | sh -s -- --version ${input.version}`, }), ...(host.declined && host.status !== "online" && @@ -185,7 +181,6 @@ export function createGatewayHosts(input: { status: "connecting", retryMs: FIRST_RETRY_MS, ...languageOf(given), - ...(given.target && { target: given.target }), }; if (given.endpoint) host.endpoint = given.endpoint; @@ -322,7 +317,6 @@ export function createGatewayHosts(input: { current.languageFeatures = given.languageFeatures; current.languageFeaturesDetail = given.languageFeaturesDetail; current.languageGroups = given.languageGroups; - current.target = given.target; next.push(current); } else { const host = create(given); From 238bf184af419fb3c2e5c3f9b4c2d7d59e5b1eff Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 16:41:54 -0400 Subject: [PATCH 2/6] Update an install Desktop recognises without asking Folded from #1032: a host with a Desktop or install.sh install updates like VS Code's server; a CLI the user put on PATH still gets the question. --- .../electron-main/remote/reviewRemoteHost.test.ts | 7 +++---- .../electron-main/remote/reviewRemoteHost.ts | 4 +++- docs/remote-hosts.md | 15 ++++++++------- 3 files changed, 14 insertions(+), 12 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 d166d075f..8f09df6b7 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 @@ -726,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) => { 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 d0be87cf0..6c3c5c1fc 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 @@ -642,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; } diff --git a/docs/remote-hosts.md b/docs/remote-hosts.md index 49dd484a3..3c460feb7 100644 --- a/docs/remote-hosts.md +++ b/docs/remote-hosts.md @@ -56,8 +56,9 @@ never stored. ## How Desktop installs Whiteboard -Desktop first checks the host and writes nothing. If the host lacks Desktop's -version, Desktop asks once, then installs it as your user. The host downloads +Desktop first checks the host and writes nothing. If the host has no install +Desktop recognises, Desktop asks once, then installs as your user. A host with +one gets Desktop's new version without a question. The host downloads it from `install.dev.fast` and checks its pinned checksum before unpacking. Desktop runs the same `install.sh` you can run by hand. @@ -78,10 +79,10 @@ instead of `~/.dev`. The review server keeps reviews in `~/.dev`. ### Updates -When Desktop updates, it installs its new version without asking, also over a -hand install. It restarts -the review server it started. It keeps the previous version and any version -still running. It leaves a server you started yourself running. +When Desktop updates, it installs its new version without asking, also over an +`install.sh` hand install. It restarts the review server it started. It keeps +the previous version and any version still running. It leaves a server you +started yourself running. ### The setting @@ -90,7 +91,7 @@ not have its version: | Value | What Desktop does | |---|---| -| `ask` (the default) | Asks once per host. | +| `ask` (the default) | Asks once per host, unless it already has an install Desktop recognises. | | `always` | Installs without asking. | | `never` | Never installs, and uses the `whiteboard` on the host's `PATH`. | From 11c612db92e2a41087451e9277a80141785eca40 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 17:08:03 -0400 Subject: [PATCH 3/6] Trim comments in the remote install --- .../src/vs/review/electron-main/remote/reviewRemoteHost.ts | 2 +- .../review/electron-main/remote/reviewRemoteInstaller.ts | 1 - .../vs/review/electron-main/remote/reviewRemoteProbe.ts | 1 - .../review/electron-main/remote/reviewRemoteUninstall.ts | 1 - apps/review-desktop/scripts/e2e/remote/releases.mjs | 2 +- apps/review-desktop/scripts/remote-dev-install.mjs | 2 +- packages/review/remote/install.sh | 7 +++---- 7 files changed, 6 insertions(+), 10 deletions(-) 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 6c3c5c1fc..13242bfb2 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 @@ -242,7 +242,7 @@ export interface ReviewRemoteInstallFlow { run(input: ReviewRemoteInstallRunInput): Promise; origin(): string | undefined; artifact(target: ReviewRemoteTarget, version: string): Promise; - /** packages/review/remote/install.sh, which probes and installs. */ + /** packages/review/remote/install.sh. */ script(): Promise; } diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts index ed3b653e0..d1399e58a 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.ts @@ -32,7 +32,6 @@ export interface ReviewRemoteInstallInput { onProgress(progress: ReviewRemoteInstallProgress): void; readonly signal: AbortSignal; readonly artifact: ReviewRemoteArtifact; - /** packages/review/remote/install.sh. */ readonly script: string; readonly spawn: SpawnSsh; readonly env: NodeJS.ProcessEnv; diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts index 616ebc918..6d8f7bbf6 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteProbe.ts @@ -70,7 +70,6 @@ export async function probeRemote(input: { session: ReviewSshSession; spawn: SpawnSsh; env: NodeJS.ProcessEnv; - /** packages/review/remote/install.sh. */ script: string; releases?: string; timeout?: number; diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts index aa71a95b3..76c54d12a 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteUninstall.ts @@ -15,7 +15,6 @@ export async function uninstallRemote(input: { session: ReviewSshSession; spawn: SpawnSsh; env: NodeJS.ProcessEnv; - /** packages/review/remote/install.sh. */ script: string; timeout?: number; }): Promise { diff --git a/apps/review-desktop/scripts/e2e/remote/releases.mjs b/apps/review-desktop/scripts/e2e/remote/releases.mjs index 740dfab8d..bc308ba5f 100644 --- a/apps/review-desktop/scripts/e2e/remote/releases.mjs +++ b/apps/review-desktop/scripts/e2e/remote/releases.mjs @@ -1,4 +1,4 @@ -/** This checkout's remote tarballs and install.sh, served over plain HTTP as install.dev.fast serves a release's: flat for DEV_FAST_REVIEW_REMOTE_RELEASES, and under /remote/ for a hand install's WHITEBOARD_RELEASES. */ +/** This checkout's remote tarballs and install.sh over plain HTTP, as install.dev.fast serves a release's; any path resolves by file name. */ import { createReadStream } from "node:fs"; import { copyFile, mkdtemp, rm, stat } from "node:fs/promises"; import { createServer } from "node:http"; diff --git a/apps/review-desktop/scripts/remote-dev-install.mjs b/apps/review-desktop/scripts/remote-dev-install.mjs index 0eff0c460..de4326061 100644 --- a/apps/review-desktop/scripts/remote-dev-install.mjs +++ b/apps/review-desktop/scripts/remote-dev-install.mjs @@ -48,7 +48,7 @@ function hostTarget(alias, { sshConfig } = {}) { return target; } -/** Installs this checkout's tarball with packages/review/remote/install.sh, as Desktop does, and returns its `whiteboard version --json`; `commit` rewrites its VS Code server's commit. */ +/** Installs this checkout's tarball with install.sh, as Desktop does; `commit` rewrites its VS Code server's commit. */ export async function remoteDevInstall( alias, { target, sshConfig, version, commit, stdio = "inherit" } = {}, diff --git a/packages/review/remote/install.sh b/packages/review/remote/install.sh index a7f8a3513..f25131574 100755 --- a/packages/review/remote/install.sh +++ b/packages/review/remote/install.sh @@ -1,6 +1,6 @@ #!/bin/sh -# Installs Whiteboard's remote CLI on a Linux host. Desktop pipes this file over ssh; -# the same file is published next to each release's tarballs for hand installs: +# Installs Whiteboard on a Linux host. Desktop pipes this file over ssh; releases publish it +# beside their tarballs for hand installs: # curl -fsSL https://install.dev.fast/releases//remote/install.sh | sh -s -- --version # # install --version V [--sha256 H (--url U | --file F)] [--downloader curl|wget] [--owner NAME] [--token HEX] @@ -168,8 +168,7 @@ release() { rm -rf "$lock.$token.done" fi } -# Runs a long step, keeping the lock fresh; a heartbeat that cannot be written means Desktop -# went away, so the step is stopped. +# Runs a long step and keeps the lock fresh; a heartbeat Desktop no longer reads stops it. guard() { if command -v setsid >/dev/null 2>&1; then setsid "$@" & else "$@" & fi pid=$! From 70432697335b6b011b861871238656d9a1817460 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 22:00:07 +0000 Subject: [PATCH 4/6] Stop an abandoned install's lock wait, and refuse bad script arguments The lock wait ran on the host with nothing written to Desktop, so an install Desktop had abandoned kept polling for up to fifteen minutes, then took the lock and could remove a version before its first heartbeat failed. The wait now beats like `guard` does, and stops at once. The heartbeat is silent on a hand install, which used to print a dot a second on stdout. An option without a value tripped `shift 2`: a bare shell error under dash, an endless loop where sh is bash. `release` without `--token` dropped a lock whose token was never written. Both now fail with a reason, which `fail` always prints on stderr. The dev install reads the version directory off the DONE line instead of assuming the default root, and shows the script's reason when it fails. A test ties the script's layout constants to the protocol's. --- .../remote/reviewRemoteInstaller.test.ts | 84 ++++++++++++++++++- .../scripts/remote-dev-install.mjs | 31 +++++-- .../scripts/remote-dev-install.test.mjs | 10 +++ packages/review/remote/install.sh | 15 ++-- 4 files changed, 125 insertions(+), 15 deletions(-) create mode 100644 apps/review-desktop/scripts/remote-dev-install.test.mjs diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts index 6bc83b86e..82d8d3469 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts @@ -4,13 +4,14 @@ *--------------------------------------------------------------------------------------------*/ import assert from "node:assert/strict"; -import { execFileSync, spawn } from "node:child_process"; +import { execFileSync, spawn, type ChildProcess } from "node:child_process"; import { createHash } from "node:crypto"; import { mkdir, mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import test from "node:test"; +import { REVIEW_REMOTE_INSTALL_LOCK, REVIEW_REMOTE_LOCK_STALE_SECONDS } from "../../common/reviewProtocol.js"; import type { SpawnSsh } from "./reviewRemoteHost.js"; import { probeRemote } from "./reviewRemoteProbe.js"; import { uninstallRemote } from "./reviewRemoteUninstall.js"; @@ -552,3 +553,84 @@ test("a lock whose start time cannot be read counts as fresh", async (t) => { assert.match(await sh(f.home, busyInstall(f.home)), /BUSY busy-laptop/); assert.equal(await readFile(join(lock, "token"), "utf8"), "0123456789abcdef\n"); }); + +/** Runs the script by hand under `shell`, as a hand install or a second laptop would. */ +function script(home: string, args: string[], env: NodeJS.ProcessEnv = {}, onStdout?: (chunk: string, child: ChildProcess) => void, shell = "/bin/sh") { + return new Promise<{ stdout: string; stderr: string; code: number | null }>((resolve, reject) => { + const child = spawn(shell, ["-s"], { env: { HOME: home, PATH, ...env }, stdio: ["pipe", "pipe", "pipe"] }); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf8").on("data", (chunk: string) => { + stdout += chunk; + onStdout?.(chunk, child); + }); + child.stderr.setEncoding("utf8").on("data", (chunk: string) => (stderr += chunk)); + child.once("error", reject); + child.once("close", (code) => resolve({ stdout, stderr, code })); + child.stdin.end(remoteInstallScriptInput(INSTALL_SCRIPT, args)); + }); +} + +test("an install waiting for the lock stops as soon as Desktop is gone", async (t) => { + const f = await fixture(t); + const lock = join(f.remoteRoot, "install.lock"); + await mkdir(lock, { recursive: true }); + await writeFile(join(lock, "token"), "0123456789abcdef\n"); + await writeFile(join(lock, "owner"), "busy-laptop\n"); + await writeFile(join(lock, "started"), `${Math.floor(Date.now() / 1000)}\n`); + const began = Date.now(); + + const ran = await script(f.home, ["install", "--version", VERSION, "--sha256", "0".repeat(64), "--url", RELEASE_URL, "--token", "00112233aabbccdd", "--lock-wait", "5", "--lock-poll", "0.1"], {}, (chunk, child) => { + if (chunk.includes("STEP waiting-for-lock")) child.stdout!.destroy(); + }); + + assert.ok(Date.now() - began < 3000, `the orphan polled for ${Date.now() - began} ms`); + assert.equal(ran.code, 3); + assert.equal(await readFile(join(lock, "token"), "utf8"), "0123456789abcdef\n"); +}); + +test("an option without a value fails with a reason, under sh and bash", async (t) => { + const f = await fixture(t); + for (const shell of ["/bin/sh", ...(spawnText("which", ["bash"]) ? ["bash"] : [])]) { + const ran = await script(f.home, ["install", "--version"], {}, undefined, shell); + assert.equal(ran.code, 3, shell); + assert.match(ran.stderr, /--version needs a value/, shell); + } + const ran = await script(f.home, ["install", "--bogus", "1"]); + assert.equal(ran.code, 3); + assert.match(ran.stderr, /unknown option --bogus/); +}); + +test("a hand install talks on stderr only and lands in Desktop's layout", async (t) => { + const f = await fixture(t); + const env = await onPath(f.root, "curl", `while [ $# -gt 0 ]; do [ "$1" = -o ] && out=$2; url=$1; shift; done +case "$url" in *.sha256) printf '%s tarball\\n' ${f.pack.sha256}; exit 0 ;; esac +exec cp ${shellQuote(f.pack.file)} "$out" +`); + + const ran = await script(f.home, ["--version", VERSION], { ...env, WHITEBOARD_RELEASES: "https://releases.test" }); + + assert.equal(ran.code, 0, ran.stderr); + assert.equal(ran.stdout, ""); + assert.match(ran.stderr, new RegExp(`Whiteboard ${VERSION} is installed`)); + assert.equal(run(join(f.home, ".local/bin/whiteboard"), "version", "--json"), `{"event":"version","version":"${VERSION}"}`); + assert.deepEqual(await installRemote(f.input()), { nodePath: join(f.remoteRoot, "versions", VERSION, "bin/node"), cliPath: join(f.remoteRoot, "versions", VERSION, "dist/cli.js") }); + assert.deepEqual(f.progress, [{ step: "done" }]); +}); + +test("release without a token refuses, so a lock whose token was never written stays", async (t) => { + const f = await fixture(t); + await mkdir(join(f.remoteRoot, "install.lock"), { recursive: true }); + + const ran = await script(f.home, ["release"]); + + assert.equal(ran.code, 3); + assert.match(ran.stderr, /release needs --token/); + assert.ok((await readdir(f.remoteRoot)).includes("install.lock")); +}); + +test("the script's layout constants are the protocol's", () => { + for (const line of [`MARKER=${REVIEW_REMOTE_INSTALL_MARKER}`, `MARK='${REVIEW_REMOTE_WRAPPER_MARK}'`, `lock=$root/${REVIEW_REMOTE_INSTALL_LOCK}`, `lockStale=${REVIEW_REMOTE_LOCK_STALE_SECONDS}`]) { + assert.ok(INSTALL_SCRIPT.split("\n").map((l) => l.trim()).includes(line), line); + } +}); diff --git a/apps/review-desktop/scripts/remote-dev-install.mjs b/apps/review-desktop/scripts/remote-dev-install.mjs index de4326061..c8f3f9c7a 100644 --- a/apps/review-desktop/scripts/remote-dev-install.mjs +++ b/apps/review-desktop/scripts/remote-dev-install.mjs @@ -12,8 +12,6 @@ import { buildDevRemoteTarball, } from "./build-remote-tarball.mjs"; -const ROOT = "~/.dev/whiteboard-remote"; - const INSTALL_SCRIPT = fileURLToPath( new URL("../../../packages/review/remote/install.sh", import.meta.url), ); @@ -48,6 +46,14 @@ function hostTarget(alias, { sshConfig } = {}) { return target; } +/** The version directory the script's DONE line names. */ +export function installedFrom(said) { + const done = /^WHITEBOARD-INSTALL DONE (.*)$/m.exec(said); + const cli = done && JSON.parse(done[1]).cli; + + return cli ? path.posix.dirname(path.posix.dirname(cli)) : undefined; +} + /** Installs this checkout's tarball with install.sh, as Desktop does; `commit` rewrites its VS Code server's commit. */ export async function remoteDevInstall( alias, @@ -57,7 +63,6 @@ export async function remoteDevInstall( const built = await buildDevRemoteTarball({ target, version, stdio }); const copy = `/tmp/whiteboard-remote-${randomBytes(4).toString("hex")}.tar.xz`; - const installed = `${ROOT}/versions/${built.version}`; try { execFileSync( @@ -68,13 +73,21 @@ export async function remoteDevInstall( const args = ["install", "--version", built.version, "--sha256", built.sha256, "--file", copy]; - const said = execFileSync("ssh", [...sshArgs(sshConfig), alias, "sh", "-s"], { - encoding: "utf8", - input: `set -- ${args.map((arg) => `'${arg}'`).join(" ")}\n${await readFile(INSTALL_SCRIPT, "utf8")}`, - stdio: ["pipe", "pipe", "inherit"], - }); + let said; + + try { + said = execFileSync("ssh", [...sshArgs(sshConfig), alias, "sh", "-s"], { + encoding: "utf8", + input: `set -- ${args.map((arg) => `'${arg}'`).join(" ")}\n${await readFile(INSTALL_SCRIPT, "utf8")}`, + stdio: ["pipe", "pipe", "inherit"], + }); + } catch (error) { + said = error.stdout ?? ""; + } + + const installed = installedFrom(said); - if (!/^WHITEBOARD-INSTALL DONE /m.test(said)) + if (!installed) throw new Error(`install.sh did not install on ${alias}:\n${said.trim()}`); return ssh( diff --git a/apps/review-desktop/scripts/remote-dev-install.test.mjs b/apps/review-desktop/scripts/remote-dev-install.test.mjs new file mode 100644 index 000000000..98e3c3a47 --- /dev/null +++ b/apps/review-desktop/scripts/remote-dev-install.test.mjs @@ -0,0 +1,10 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { installedFrom } from "./remote-dev-install.mjs"; + +test("installedFrom reads the version directory off the DONE line, wherever the root is", () => { + const said = `WHITEBOARD-INSTALL STEP package\n.\n.\nWHITEBOARD-INSTALL DONE {"version":"1.2.3","sha256":"ab","node":"/srv/x/whiteboard-remote/versions/1.2.3/bin/node","cli":"/srv/x/whiteboard-remote/versions/1.2.3/dist/cli.js","installedAt":1}\n`; + assert.equal(installedFrom(said), "/srv/x/whiteboard-remote/versions/1.2.3"); + assert.equal(installedFrom("WHITEBOARD-INSTALL FAIL cannot unpack the tarball\n"), undefined); +}); diff --git a/packages/review/remote/install.sh b/packages/review/remote/install.sh index f25131574..6131a6ba9 100755 --- a/packages/review/remote/install.sh +++ b/packages/review/remote/install.sh @@ -32,9 +32,11 @@ hand= say() { [ -n "$hand" ] || printf '\n%s %s\n' WHITEBOARD-INSTALL "$*" >&3; } tell() { [ -z "$hand" ] || printf '%s\n' "$*" >&2; } +# Fails once Desktop stops reading, so an install it abandoned stops too. +beat() { [ -n "$hand" ] || printf '.\n' >&3 2>/dev/null; } fail() { say FAIL "$*" - tell "Whiteboard was not installed: $*" + printf 'Whiteboard was not installed: %s\n' "$*" >&2 exit 3 } q() { printf "'%s'" "$(printf '%s' "$1" | sed "s/'/'\\\\''/g")"; } @@ -88,7 +90,7 @@ probe() { releases= while [ $# -gt 0 ]; do case "$1" in - --releases) releases=$2; shift 2 ;; + --releases) [ $# -ge 2 ] || fail "$1 needs a value"; releases=$2; shift 2 ;; *) shift ;; esac done @@ -173,7 +175,7 @@ guard() { if command -v setsid >/dev/null 2>&1; then setsid "$@" & else "$@" & fi pid=$! while kill -0 "$pid" 2>/dev/null; do - printf '.\n' >&3 2>/dev/null || { + beat || { kill -TERM -"$pid" 2>/dev/null || kill "$pid" 2>/dev/null exit 3 } @@ -250,6 +252,7 @@ install() { --lock-stale) lockStale=$2 ;; *) fail "unknown option $1" ;; esac + [ $# -ge 2 ] || fail "$1 needs a value" shift 2 done [ -n "$url$file" ] || hand=1 @@ -298,6 +301,7 @@ install() { [ -n "$told" ] || say STEP waiting-for-lock told=1 polls=$((polls - 1)) + beat || exit 3 sleep "$lockPoll" done trap release EXIT @@ -423,7 +427,7 @@ uninstall() { newest=$(for dir in "$root"/versions/*; do [ -n "$(completeSha256 "$dir/$MARKER")" ] && printf '%s\n' "${dir##*/}"; done | grep -Ex '[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?' | newestFirst | head -n 1) [ -n "$newest" ] || fail Whiteboard Desktop installed nothing there - completeSha256 "$root/versions/$newest/$MARKER" >/dev/null + completeSha256 "$root/versions/$newest/$MARKER" >/dev/null # for its cn and cc DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 exec "$cn" "$cc" remote uninstall --keep-reviews --json Date: Wed, 14 Oct 2026 18:06:34 -0400 Subject: [PATCH 5/6] Fake a Linux uname in the hand-install test, so it runs on macOS --- .../vs/review/electron-main/remote/reviewRemoteInstaller.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts index 82d8d3469..fc70b6035 100644 --- a/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts +++ b/apps/review-desktop/code-oss/src/vs/review/electron-main/remote/reviewRemoteInstaller.test.ts @@ -603,6 +603,7 @@ test("an option without a value fails with a reason, under sh and bash", async ( test("a hand install talks on stderr only and lands in Desktop's layout", async (t) => { const f = await fixture(t); + await onPath(f.root, "uname", `case "$1" in -s) echo Linux ;; -m) echo aarch64 ;; esac\n`); const env = await onPath(f.root, "curl", `while [ $# -gt 0 ]; do [ "$1" = -o ] && out=$2; url=$1; shift; done case "$url" in *.sha256) printf '%s tarball\\n' ${f.pack.sha256}; exit 0 ;; esac exec cp ${shellQuote(f.pack.file)} "$out" From fdc04b87f00e41f40c9c6079527887320f9a7d6a Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Wed, 14 Oct 2026 22:14:57 +0000 Subject: [PATCH 6/6] Group the conditions shellcheck 0.9 reads as if-then-else --- packages/review/remote/install.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/review/remote/install.sh b/packages/review/remote/install.sh index 6131a6ba9..fd8c2d1bc 100755 --- a/packages/review/remote/install.sh +++ b/packages/review/remote/install.sh @@ -351,7 +351,7 @@ install() { fail cannot unpack the tarball } rm -f "$f" - [ -x "$p/bin/node" ] && [ -f "$p/dist/cli.js" ] && [ -x "$p/whiteboard" ] || { + { [ -x "$p/bin/node" ] && [ -f "$p/dist/cli.js" ] && [ -x "$p/whiteboard" ]; } || { rm -rf "$p" fail the tarball holds no whiteboard } @@ -435,7 +435,7 @@ case "${1-}" in probe) shift && probe "$@" ;; uninstall) uninstall ;; release) - [ "${2-}" = --token ] && [ -n "${3-}" ] || fail "release needs --token HEX" + { [ "${2-}" = --token ] && [ -n "${3-}" ]; } || fail "release needs --token HEX" token=$3 release ;;