From 257e199199a133f4b824b555ff03cf1129384f8a Mon Sep 17 00:00:00 2001 From: ketan0 Date: Thu, 24 Sep 2026 17:01:22 -0700 Subject: [PATCH] fix: explain read-only macOS installs and pause automatic update retries Agent-Session: 01a0d12e-51ca-7230-9991-0ce7a73018b2 --- .../update/common/darwinUpdateRecovery.ts | 5 + .../electron-main/updateService.darwin.ts | 7 +- .../update/reviewUpdate.contribution.ts | 15 +- .../scripts/darwin-readonly-updates.test.mjs | 185 ++++++++++++++++++ 4 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 apps/review-desktop/scripts/darwin-readonly-updates.test.mjs diff --git a/apps/review-desktop/code-oss/src/vs/platform/update/common/darwinUpdateRecovery.ts b/apps/review-desktop/code-oss/src/vs/platform/update/common/darwinUpdateRecovery.ts index 8263a352e..2e23d9f1e 100644 --- a/apps/review-desktop/code-oss/src/vs/platform/update/common/darwinUpdateRecovery.ts +++ b/apps/review-desktop/code-oss/src/vs/platform/update/common/darwinUpdateRecovery.ts @@ -126,3 +126,8 @@ function parseObject(raw: string | undefined): Record | undefin return undefined; } } + +/** Squirrel's native error for a mounted image or macOS App Translocation. */ +export function isDarwinReadOnlyVolumeError(error: string | undefined): boolean { + return error?.startsWith('Cannot update while running on a read-only volume.') ?? false; +} diff --git a/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/updateService.darwin.ts b/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/updateService.darwin.ts index ae25548d9..06cc4a4db 100644 --- a/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/updateService.darwin.ts +++ b/apps/review-desktop/code-oss/src/vs/platform/update/electron-main/updateService.darwin.ts @@ -23,6 +23,7 @@ import { IApplicationStorageMainService } from '../../storage/electron-main/stor import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { blocksAutomaticDarwinUpdate, + isDarwinReadOnlyVolumeError, DARWIN_FAILED_UPDATE_STORAGE_KEY, DARWIN_UPDATE_ATTEMPT_STORAGE_KEY, DARWIN_UPDATE_OUTCOME_STORAGE_KEY, @@ -42,6 +43,8 @@ function darwinBundleName(): string | undefined { export class DarwinUpdateService extends AbstractUpdateService implements IRelaunchHandler { private feedUrlError: string | undefined; + // A relaunch after moving the app clears this; explicit checks can retry sooner. + private readOnlyVolume = false; @memoize private get onRawError(): Event { return Event.fromNodeEventEmitter(electron.autoUpdater, 'error', (_, message) => message); } @memoize private get onRawCheckingForUpdate(): Event { return Event.fromNodeEventEmitter(electron.autoUpdater, 'checking-for-update'); } @@ -113,6 +116,7 @@ export class DarwinUpdateService extends AbstractUpdateService implements IRelau return; } + this.readOnlyVolume = isDarwinReadOnlyVolumeError(err); this.setState(State.Idle(UpdateType.Archive, err, undefined, 'electron')); } @@ -134,10 +138,11 @@ export class DarwinUpdateService extends AbstractUpdateService implements IRelau } protected doCheckForUpdates(explicit: boolean, pendingCommit?: string): void { - if (!this.quality) { + if (!this.quality || (this.readOnlyVolume && !explicit)) { return; } + this.readOnlyVolume = false; this.setState(State.CheckingForUpdates(explicit)); const internalOrg = this.getInternalOrg(); diff --git a/apps/review-desktop/code-oss/src/vs/review/contrib/update/reviewUpdate.contribution.ts b/apps/review-desktop/code-oss/src/vs/review/contrib/update/reviewUpdate.contribution.ts index 13843c123..ea22331f7 100644 --- a/apps/review-desktop/code-oss/src/vs/review/contrib/update/reviewUpdate.contribution.ts +++ b/apps/review-desktop/code-oss/src/vs/review/contrib/update/reviewUpdate.contribution.ts @@ -19,7 +19,7 @@ import { localize, localize2 } from '../../../nls.js'; import { toAction } from '../../../base/common/actions.js'; import { Disposable } from '../../../base/common/lifecycle.js'; -import { isLinux } from '../../../base/common/platform.js'; +import { isLinux, isMacintosh } from '../../../base/common/platform.js'; import Severity from '../../../base/common/severity.js'; import { Action2, registerAction2 } from '../../../platform/actions/common/actions.js'; import type { ServicesAccessor } from '../../../platform/instantiation/common/instantiation.js'; @@ -29,6 +29,7 @@ import { IProductService } from '../../../platform/product/common/productService import { IStorageService, StorageScope, StorageTarget } from '../../../platform/storage/common/storage.js'; import { darwinFailedUpdateNoticeId, + isDarwinReadOnlyVolumeError, DARWIN_FAILED_UPDATE_STORAGE_KEY, parseDarwinFailedUpdate, shouldAnnounceDarwinFailedUpdate, @@ -60,6 +61,7 @@ class ReviewUpdateNotifications extends Disposable { private readyHandle: INotificationHandle | undefined; /** Which commit `readyHandle` is about. */ private readyCommit: string | undefined; + private announcedReadOnlyVolume = false; constructor( @IUpdateService private readonly updateService: IUpdateService, @@ -154,6 +156,17 @@ class ReviewUpdateNotifications extends Disposable { * would add a download-progress story for a state that clears on its own. */ private onStateChange(state: State): void { + if (isMacintosh && state.type === StateType.Idle && isDarwinReadOnlyVolumeError(state.error)) { + if (!this.announcedReadOnlyVolume) { + this.announcedReadOnlyVolume = true; + this.notificationService.notify({ + severity: Severity.Warning, + sticky: true, + message: localize('review.update.readOnlyVolume', "Whiteboard cannot update from its current location. Quit Whiteboard, use Finder to copy or move Whiteboard to your Applications folder, then open it from there. If you opened Whiteboard from a disk image, eject the disk image after copying the app."), + }); + } + return; + } if (isLinux && state.type === StateType.AvailableForDownload) { if (this.readyCommit === state.update.version && this.readyHandle) { return; } this.readyHandle?.close(); diff --git a/apps/review-desktop/scripts/darwin-readonly-updates.test.mjs b/apps/review-desktop/scripts/darwin-readonly-updates.test.mjs new file mode 100644 index 000000000..181f3b67c --- /dev/null +++ b/apps/review-desktop/scripts/darwin-readonly-updates.test.mjs @@ -0,0 +1,185 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import test from "node:test"; +import vm from "node:vm"; + +import ts from "typescript"; + +const sourceRoot = new URL("../code-oss/src/vs/", import.meta.url); + +const require = createRequire(import.meta.url); + +const noOp = () => {}; + +const decorators = new Proxy({}, { get: () => noOp }); + +// Execute the production methods with Electron/DI boundaries replaced. No +// native updater, user's installation, preferences, or network is touched. +function load(relative, imports = {}, appended = "") { + const filename = new URL(relative, sourceRoot); + + const { outputText, diagnostics } = ts.transpileModule( + readFileSync(filename, "utf8") + appended, + { + compilerOptions: { + module: ts.ModuleKind.CommonJS, + target: ts.ScriptTarget.ES2022, + experimentalDecorators: true, + }, + reportDiagnostics: true, + fileName: filename.pathname, + }, + ); + + assert.equal(diagnostics.length, 0); + const module = { exports: {} }; + vm.runInNewContext( + outputText, + { + module, + exports: module.exports, + process, + require: (name) => { + if (name in imports) return imports[name]; + + if (name.startsWith("node:")) return require(name); + + if (/common\/instantiation.js$/.test(name)) + return { createDecorator: () => noOp }; + + if (/common\/types.js$/.test(name)) return { upcast: (value) => value }; + + return decorators; + }, + }, + { filename: filename.pathname }, + ); + + return module.exports; +} + +const recovery = load("platform/update/common/darwinUpdateRecovery.ts"); + +const update = load("platform/update/common/update.ts"); + +const readOnlyError = + "Cannot update while running on a read-only volume. The application is on a read-only volume. Please move the application and try again."; + +function service() { + let checks = 0; + + const { DarwinUpdateService } = load( + "platform/update/electron-main/updateService.darwin.ts", + { + electron: { + autoUpdater: { + checkForUpdates: () => { + checks++; + }, + }, + }, + "./abstractUpdateService.js": { AbstractUpdateService: Object }, + "../common/update.js": update, + "../common/darwinUpdateRecovery.js": recovery, + "../../../base/common/hash.js": { hash: () => 0 }, + }, + ); + + const instance = Object.create(DarwinUpdateService.prototype); + Object.assign(instance, { + quality: "stable", + productService: { commit: "current" }, + telemetryService: { publicLog2: noOp }, + logService: { trace: noOp, error: noOp }, + meteredConnectionService: { isConnectionMetered: false }, + state: update.State.Idle(update.UpdateType.Archive), + setState(state) { + this.state = state; + }, + getInternalOrg: () => undefined, + getFailedUpdate: () => undefined, + buildUpdateFeedUrl: () => "https://updates.example.test/", + }); + + return { instance, checks: () => checks }; +} + +test("native read-only failure is surfaced once and stops scheduled attempts for this process", () => { + const { instance, checks } = service(); + instance.doCheckForUpdates(false); + assert.equal(checks(), 1); + instance.onError(readOnlyError); + assert.equal(instance.state.error, readOnlyError); + + for (let hour = 0; hour < 3; hour++) instance.doCheckForUpdates(false); + assert.equal(checks(), 1); + assert.equal(instance.state.error, readOnlyError); +}); + +test("manual retry and a fresh process can recover after relocating the app", () => { + const { instance, checks } = service(); + instance.doCheckForUpdates(false); + instance.onError(readOnlyError); + instance.doCheckForUpdates(true); + assert.equal(checks(), 2); + instance.onUpdateNotAvailable(); + instance.doCheckForUpdates(false); + assert.equal(checks(), 3); + const restarted = service(); + restarted.instance.doCheckForUpdates(false); + assert.equal(restarted.checks(), 1); +}); + +test("ordinary update failures remain retryable and late native errors do not clobber Ready", () => { + const { instance, checks } = service(); + instance.doCheckForUpdates(false); + instance.onError("The Internet connection appears to be offline."); + instance.doCheckForUpdates(false); + assert.equal(checks(), 2); + const ready = update.State.Ready({ version: "next" }, false, false); + instance.state = ready; + instance.onError(readOnlyError); + assert.equal(instance.state, ready); + instance.state = update.State.Idle(update.UpdateType.Archive); + instance.doCheckForUpdates(false); + assert.equal(checks(), 3); +}); + +test("recovery notice is sticky, deduplicated, and specific to native macOS read-only failures", () => { + const notices = []; + + const { ReviewUpdateNotifications } = load( + "review/contrib/update/reviewUpdate.contribution.ts", + { + "../../../base/common/lifecycle.js": { Disposable: Object }, + "../../../base/common/platform.js": { isMacintosh: true, isLinux: false }, + "../../../platform/actions/common/actions.js": { + Action2: Object, + registerAction2: noOp, + }, + "../../../platform/update/common/update.js": update, + "../../../platform/update/common/darwinUpdateRecovery.js": recovery, + "../../../workbench/common/contributions.js": { + WorkbenchPhase: {}, + registerWorkbenchContribution2: noOp, + }, + "../../../nls.js": { localize: (_key, text) => text }, + }, + "\nexports.ReviewUpdateNotifications = ReviewUpdateNotifications;", + ); + + const instance = Object.create(ReviewUpdateNotifications.prototype); + instance.notificationService = { notify: (notice) => notices.push(notice) }; + instance.onStateChange( + update.State.Idle(update.UpdateType.Archive, "offline"), + ); + assert.equal(notices.length, 0); + const blocked = update.State.Idle(update.UpdateType.Archive, readOnlyError); + instance.onStateChange(blocked); + instance.onStateChange(blocked); + assert.equal(notices.length, 1); + assert.equal(notices[0].sticky, true); + assert.match(notices[0].message, /Applications folder/); + assert.match(notices[0].message, /disk image/); +});