From 8fa8ae401e25e788cab2e73e8e2a1640ab8172ed Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 2 Sep 2026 00:55:34 -0700 Subject: [PATCH 1/4] fix(desktop): harden update and quit flows --- .github/workflows/ci.yml | 22 ++- .github/workflows/desktop-release.yml | 42 ++++-- apps/desktop/README.md | 2 +- apps/desktop/scripts/ensure-pty-prebuilds.ts | 47 +++++- apps/desktop/src/main/index.ts | 44 +++--- apps/desktop/src/main/updater.test.ts | 134 +++++++++++++++++- apps/desktop/src/main/updater.ts | 117 +++++++++------ apps/desktop/src/main/window.test.ts | 83 +++++++++-- apps/desktop/src/main/window.ts | 83 +++++++---- apps/desktop/src/main/windows.test.ts | 18 ++- apps/desktop/src/main/windows.ts | 7 +- apps/desktop/src/test/electron-mock.ts | 7 + apps/sim/app/_shell/desktop-update-gate.tsx | 11 +- .../api/desktop/update/download/route.test.ts | 9 ++ .../app/api/desktop/update/download/route.ts | 37 +++-- .../update/latest-mac.yml/route.test.ts | 24 +++- .../desktop/update/latest-mac.yml/route.ts | 59 ++++++-- .../settings/components/desktop/desktop.tsx | 35 ++--- .../sidebar-footer/sidebar-footer.test.tsx | 16 ++- .../sidebar-footer/sidebar-footer.tsx | 29 +--- .../w/components/sidebar/sidebar.tsx | 25 ++-- .../hooks/use-desktop-update-state.test.tsx | 90 ++++++++++++ apps/sim/hooks/use-desktop-update-state.ts | 37 +++++ apps/sim/lib/desktop/update-feed.test.ts | 25 ++-- apps/sim/lib/desktop/update-feed.ts | 52 +++++-- packages/desktop-bridge/contract-snapshot.ts | 29 ++-- packages/desktop-bridge/src/index.ts | 20 ++- 27 files changed, 810 insertions(+), 294 deletions(-) create mode 100644 apps/sim/hooks/use-desktop-update-state.test.tsx create mode 100644 apps/sim/hooks/use-desktop-update-state.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bfac30a378a..82f4b3299cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -732,9 +732,7 @@ jobs: # release-only simstudioai/sim-desktop-releases repository. Keeping these # builds out of this source repository prevents its followers from receiving # every internal shell release. Each environment's /api/desktop/update feed - # still offers only its own stream. Unlike stable releases, prereleases build - # before the Apple signing secrets exist — unsigned, so the update pipeline - # remains testable end to end with a manual download. + # still offers only its own signed stream. create-desktop-prerelease: name: Create Desktop Prerelease runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }} @@ -742,7 +740,13 @@ jobs: needs: [detect-desktop-changes, check-desktop-signing] # Requires the signing probe to have actually succeeded (not just "not # cancelled") so a probe failure can't produce a release with no build. - if: ${{ !cancelled() && needs.detect-desktop-changes.outputs.changed == 'true' && needs.check-desktop-signing.result == 'success' }} + if: >- + ${{ + !cancelled() && + needs.detect-desktop-changes.outputs.changed == 'true' && + needs.check-desktop-signing.result == 'success' && + needs.check-desktop-signing.outputs.configured == 'true' + }} permissions: contents: read outputs: @@ -758,13 +762,12 @@ jobs: GH_TOKEN: ${{ github.token }} PRERELEASE_REPOSITORY: simstudioai/sim-desktop-releases SOURCE_REPOSITORY: ${{ github.repository }} - SIGNED: ${{ needs.check-desktop-signing.outputs.configured }} run: | if [ -z "$DESKTOP_RELEASE_TOKEN" ]; then echo "::error::DESKTOP_RELEASE_TOKEN is required to publish desktop prereleases." exit 1 fi - if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNEL=dev; APP_NAME="Sim Dev"; else CHANNEL=staging; APP_NAME="Sim Staging"; fi + if [ "$GITHUB_REF" = "refs/heads/dev" ]; then CHANNEL=dev; else CHANNEL=staging; fi # Prerelease core = next patch after the latest stable release, so # channel builds always outrank the stable they are built on top of # and are always superseded by the next stable. The run-attempt @@ -786,11 +789,6 @@ jobs: IFS='.' read -r MAJOR MINOR PATCH <<< "${LATEST#v}" TAG="v${MAJOR}.${MINOR}.$((PATCH + 1))-${CHANNEL}.${GITHUB_RUN_NUMBER}.${GITHUB_RUN_ATTEMPT}" NOTES="Automated ${CHANNEL}-channel desktop build from ${GITHUB_REF_NAME} @ ${GITHUB_SHA::7}." - if [ "$SIGNED" != "true" ]; then - NOTES="$NOTES - - ⚠️ Unsigned test build (Apple signing secrets not configured). Gatekeeper will quarantine a downloaded copy: right-click → Open, or clear the flag with \`xattr -dr com.apple.quarantine \"/Applications/${APP_NAME}.app\"\`." - fi # Draft until the build uploads its artifacts: drafts are invisible # to the update feed, so a failed or in-flight build can never take # the channel down with an assetless release. The release-only repo @@ -818,7 +816,7 @@ jobs: with: version: ${{ needs.create-desktop-prerelease.outputs.version }} publish: true - sign: ${{ needs.check-desktop-signing.outputs.configured == 'true' }} + sign: true secrets: inherit # The draft only becomes visible to the update feed once its artifacts are diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index b3c51213545..fd9d8584687 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -19,10 +19,8 @@ on: type: boolean default: true sign: - description: Sign and notarize with the Apple Developer identity. When - false (prerelease testing before the signing secrets exist) the build - is packaged unsigned; installed shells detect this and offer manual - downloads instead of Squirrel installs. + description: Sign and notarize with the Apple Developer identity. Unsigned + builds are workflow artifacts only and cannot be published. required: false type: boolean default: true @@ -50,6 +48,7 @@ jobs: build-sign-notarize: name: Build, Sign, Notarize runs-on: macos-26 + timeout-minutes: 60 steps: - name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 @@ -101,8 +100,8 @@ jobs: echo "::error::Manual desktop releases must use a stable source-repository tag." exit 1 fi - if [ "$TOKEN_KIND" = stable ] && [ "$PUBLISH" = true ] && [ "$SIGN" != true ]; then - echo "::error::Stable desktop releases must be signed before publication." + if [ "$PUBLISH" = true ] && [ "$SIGN" != true ]; then + echo "::error::Desktop releases must be signed before publication." exit 1 fi if [ "$TOKEN_KIND" = stable ]; then @@ -206,10 +205,9 @@ jobs: bunx electron-builder --mac --publish never -c.productName="$PRODUCT_NAME" -c.appId="$APP_ID" - # Unsigned prerelease path: no Developer ID, no notarization. The - # binaries are explicitly ad-hoc signed with Hardened Runtime off, which - # runs locally but gets quarantined when downloaded — fine for testing - # the update pipeline without Developer ID credentials. + # Unsigned artifact-only path: no Developer ID or notarization. The bundle + # is ad-hoc signed with Hardened Runtime off for local workflow testing and + # must never be published. - name: Package unsigned if: ${{ !inputs.sign }} working-directory: apps/desktop @@ -274,6 +272,9 @@ jobs: - name: Validate signature and notarization if: ${{ inputs.sign }} env: + APP_ID: ${{ steps.channel.outputs.app_id }} + APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + PRODUCT_NAME: ${{ steps.channel.outputs.name }} VERSION: ${{ inputs.version }} run: | SEMVER="${VERSION#v}" @@ -281,6 +282,25 @@ jobs: ZIP="apps/desktop/release/Sim-${SEMVER}-universal.zip" MOUNT_POINT="$RUNNER_TEMP/sim-dmg" ZIP_DIR="$(mktemp -d "$RUNNER_TEMP/sim-zip.XXXXXX")" + validate_identity() { + local APP_BUNDLE="$1" + local ACTUAL_APP_ID ACTUAL_NAME SIGNATURE + ACTUAL_APP_ID="$(/usr/libexec/PlistBuddy -c 'Print :CFBundleIdentifier' "$APP_BUNDLE/Contents/Info.plist")" + ACTUAL_NAME="$(/usr/libexec/PlistBuddy -c 'Print :CFBundleName' "$APP_BUNDLE/Contents/Info.plist")" + SIGNATURE="$(codesign -dv --verbose=4 "$APP_BUNDLE" 2>&1)" + if [ "$ACTUAL_APP_ID" != "$APP_ID" ] || [ "$ACTUAL_NAME" != "$PRODUCT_NAME" ]; then + echo "::error::Unexpected packaged identity: $ACTUAL_NAME ($ACTUAL_APP_ID)." + exit 1 + fi + if ! grep -Fxq "TeamIdentifier=$APPLE_TEAM_ID" <<< "$SIGNATURE"; then + echo "::error::The app was not signed by the expected Apple team." + exit 1 + fi + if ! grep -Eq 'flags=.*runtime' <<< "$SIGNATURE"; then + echo "::error::The app was not signed with Hardened Runtime." + exit 1 + fi + } mkdir -p "$MOUNT_POINT" hdiutil attach "$DMG" -mountpoint "$MOUNT_POINT" -nobrowse -quiet trap 'hdiutil detach "$MOUNT_POINT" -quiet || true; rm -rf "$ZIP_DIR"' EXIT @@ -292,6 +312,7 @@ jobs: xcrun stapler validate "$APP_BUNDLE" spctl --assess --type execute --verbose "$APP_BUNDLE" codesign --verify --deep --strict "$APP_BUNDLE" + validate_identity "$APP_BUNDLE" unzip -q "$ZIP" -d "$ZIP_DIR" ZIP_APP="$(find "$ZIP_DIR" -maxdepth 2 -name '*.app' -print -quit)" if [ -z "$ZIP_APP" ]; then @@ -301,6 +322,7 @@ jobs: xcrun stapler validate "$ZIP_APP" spctl --assess --type execute --verbose "$ZIP_APP" codesign --verify --deep --strict "$ZIP_APP" + validate_identity "$ZIP_APP" hdiutil detach "$MOUNT_POINT" -quiet rm -rf "$ZIP_DIR" trap - EXIT diff --git a/apps/desktop/README.md b/apps/desktop/README.md index c8deeb969d1..ecbbf33de59 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -171,7 +171,7 @@ Raw local file bytes are never exposed through the preload bridge and cannot be ## Auto-update, channels, rollout, rollback -- `electron-updater` reads the deployment's `/api/desktop/update` feed; production resolves stable releases from `simstudioai/sim`, while dev/staging resolve prereleases from `simstudioai/sim-desktop-releases`. Artifact downloads go directly to GitHub and deltas use `.zip.blockmap`. Install is prompt-based (Restart Now / Later; Later installs on quit) — never forced mid-session. +- `electron-updater` reads the deployment's `/api/desktop/update` feed; production resolves stable releases from `simstudioai/sim`, while dev/staging resolve prereleases from `simstudioai/sim-desktop-releases`. Artifact downloads go directly to GitHub and deltas use `.zip.blockmap`. Sim validates every candidate before starting its download. Install is prompt-based (Restart and update / Later; Later installs on quit) — never forced mid-session. - Streams: production follows stable `X.Y.Z` releases, dev follows `-dev.N`, and staging follows `-staging.N`. The feed still recognizes legacy `-alpha.N`/`-beta.N` releases during migration. - Staged rollout: after publishing, edit `stagingPercentage: 10` into the release's `latest-mac.yml`, then raise as crash metrics stay clean. - Rollback: a pulled release must be superseded by a **higher** version — users on the broken build will not reinstall an equal one. (A blocked-versions kill-switch was removed as unwired dead code; reintroduce it in `updater.ts` if a remote config source ever exists to feed it.) diff --git a/apps/desktop/scripts/ensure-pty-prebuilds.ts b/apps/desktop/scripts/ensure-pty-prebuilds.ts index a41f94cfae8..a08856a51f2 100644 --- a/apps/desktop/scripts/ensure-pty-prebuilds.ts +++ b/apps/desktop/scripts/ensure-pty-prebuilds.ts @@ -14,10 +14,23 @@ * one. That is why this build needs no `x64ArchFiles` rule. */ import { execFileSync } from 'node:child_process' -import { existsSync, mkdirSync, mkdtempSync, renameSync, rmSync, writeFileSync } from 'node:fs' +import { createHash, timingSafeEqual } from 'node:crypto' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from 'node:fs' import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' +import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' + +const logger = createLogger('DesktopPtyPrebuilds') const REQUIRED_ARCHES = ['darwin-arm64', 'darwin-x64'] as const @@ -49,8 +62,30 @@ function packageDir(arch: string): string { return join(workspaceRoot, 'node_modules', '@lydell', `node-pty-${arch}`) } +function expectedIntegrity(arch: string, version: string): string { + const packageName = `@lydell/node-pty-${arch}` + const prefix = `"${packageName}": ["${packageName}@${version}"` + const entry = readFileSync(join(workspaceRoot, 'bun.lock'), 'utf8') + .split('\n') + .find((line) => line.trimStart().startsWith(prefix)) + const integrity = entry ? /,\s*"(sha512-[^"]+)"\],?$/.exec(entry)?.[1] : undefined + if (!integrity) { + throw new Error(`Could not find the pinned integrity for ${packageName}@${version}`) + } + return integrity +} + +function verifyIntegrity(bytes: Buffer, integrity: string, packageName: string): void { + const expected = Buffer.from(integrity.slice('sha512-'.length), 'base64') + const actual = createHash('sha512').update(bytes).digest() + if (expected.length !== actual.length || !timingSafeEqual(expected, actual)) { + throw new Error(`Integrity check failed for ${packageName}`) + } +} + async function fetchPrebuild(arch: string, version: string): Promise { const name = `node-pty-${arch}` + const packageName = `@lydell/${name}` const url = `https://registry.npmjs.org/@lydell/${name}/-/${name}-${version}.tgz` const response = await fetch(url) if (!response.ok) { @@ -60,7 +95,9 @@ async function fetchPrebuild(arch: string, version: string): Promise { const staging = mkdtempSync(join(tmpdir(), 'sim-pty-prebuild-')) try { const tarball = join(staging, 'package.tgz') - writeFileSync(tarball, Buffer.from(await response.arrayBuffer())) + const bytes = Buffer.from(await response.arrayBuffer()) + verifyIntegrity(bytes, expectedIntegrity(arch, version), packageName) + writeFileSync(tarball, bytes) execFileSync('tar', ['-xzf', tarball, '-C', staging], { stdio: 'pipe' }) const target = packageDir(arch) @@ -77,10 +114,10 @@ async function run(): Promise { for (const arch of REQUIRED_ARCHES) { const dir = packageDir(arch) if (existsSync(join(dir, 'prebuilds', arch, 'pty.node'))) { - console.log(`• node-pty prebuild present: ${arch}`) + logger.info('node-pty prebuild present', { arch }) continue } - console.log(`• Fetching node-pty prebuild: ${arch}@${version}`) + logger.info('Fetching node-pty prebuild', { arch, version }) await fetchPrebuild(arch, version) if (!existsSync(join(dir, 'prebuilds', arch, 'pty.node'))) { throw new Error(`Downloaded @lydell/node-pty-${arch} but pty.node is missing`) @@ -89,6 +126,6 @@ async function run(): Promise { } run().catch((error) => { - console.error(error) + logger.error('Could not ensure node-pty prebuilds', { message: getErrorMessage(error) }) process.exit(1) }) diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index fc60d931d18..9cfcc37b401 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -157,7 +157,7 @@ function main(): void { let appSession: Session | null = null let sessionLifecycle: ReturnType | null = null let resumingQuitAfterTeardown = false - let mandatoryRelaunchPending = false + let committedRelaunchPending = false let tray: TrayHandle | null = null let updater: UpdaterHandle | null = null let startupReady: Promise | null = null @@ -383,7 +383,7 @@ function main(): void { preloadPath, isPackaged: app.isPackaged, restorePosition, - isMandatoryRelaunchPending: () => mandatoryRelaunchPending, + isCommittedRelaunchPending: () => committedRelaunchPending, onFullScreenChange: (isFullScreen) => { if (!win.isDestroyed()) { win.webContents.send('desktop:window-state:changed', { isFullScreen }) @@ -418,7 +418,7 @@ function main(): void { } }, allowHttpLocalhost: allowHttpLocalhost(), - isMandatoryRelaunchPending: () => mandatoryRelaunchPending, + isCommittedRelaunchPending: () => committedRelaunchPending, }) attachContextMenu(win.webContents, { isDev: !app.isPackaged, @@ -554,7 +554,7 @@ function main(): void { }, completeDeploymentScopedStateChange: completeDeploymentScopedTeardown, relaunch: () => { - mandatoryRelaunchPending = true + committedRelaunchPending = true relaunchApp() }, }) @@ -585,11 +585,11 @@ function main(): void { if (!resumingQuitAfterTeardown && sessionLifecycle?.isTeardownActive()) { event.preventDefault() void sessionLifecycle.awaitTeardown().then((clean) => { - if (!clean && !mandatoryRelaunchPending) { + if (!clean && !committedRelaunchPending) { logger.error('Quit cancelled because account teardown did not finish safely') return } - if (!mandatoryRelaunchPending && !prepareAccountDataTeardownForQuit()) { + if (!committedRelaunchPending && !prepareAccountDataTeardownForQuit()) { logger.error('Quit cancelled because account-data recovery could not be persisted') return } @@ -603,36 +603,27 @@ function main(): void { }) return } - /** - * A mandatory relaunch is requested only after the server-switch transaction - * has cleared deployment-scoped capabilities and committed the replacement - * origin. The ordinary quit guard must not strand that committed process on - * its old partition; any retained marker is startup retry metadata. - */ - if (!mandatoryRelaunchPending && !prepareAccountDataTeardownForQuit()) { + // A committed relaunch is requested only after its prerequisite teardown has + // succeeded. The ordinary quit guard must not strand that committed process; + // any retained marker is startup retry metadata. + if (!committedRelaunchPending && !prepareAccountDataTeardownForQuit()) { event.preventDefault() logger.error('Quit cancelled because account-data recovery could not be persisted') return } - // Stops the tray's background chat refresh alongside the OS handles. + }) + + app.on('will-quit', () => { + // Renderer unload guards have accepted the quit, so native resources can + // now be released without leaving a cancelled quit in a degraded state. tray?.destroy() tray = null localFilesystem.close() - // Quiesce native pages before publishing the final encrypted descriptor - // set. This prevents a navigation event racing the synchronous quit flush. quiesceBrowserSessions() terminal.dispose() uninstallDocumentationHelpSearch() - flushDesktopChatSessions('before-quit') - // Settings writes coalesce, so a change made in the last moments before - // quit is still pending here. - config.flush() - }) - - app.on('will-quit', () => { - // Final backstop for any descriptor dirtied while Electron was closing - // windows after before-quit. flushDesktopChatSessions('will-quit') + config.flush() }) app.on('activate', () => { @@ -868,6 +859,9 @@ function main(): void { events, appOrigin, autoDownload: () => config.get('autoDownloadUpdates') ?? true, + setRelaunchPending: (pending) => { + committedRelaunchPending = pending + }, beforeInstall: async () => { if (!prepareAccountDataTeardownForQuit()) { throw new Error( diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index a95d2839873..ee70f5b3ce6 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -158,6 +158,7 @@ describe('initUpdater state machine', () => { feedAvailable?: boolean | 'no-release' probeOriginFeed?: (feedUrl: string) => Promise beforeInstall?: () => Promise + setRelaunchPending?: (pending: boolean) => void }) { const states: DesktopUpdateState[] = [] const handle = initUpdater({ @@ -172,6 +173,7 @@ describe('initUpdater state machine', () => { canSelfUpdate: async () => true, platform: 'darwin', beforeInstall: options?.beforeInstall, + setRelaunchPending: options?.setRelaunchPending, }) // Engine selection (signature detection) resolves asynchronously. await vi.advanceTimersByTimeAsync(0) @@ -188,14 +190,14 @@ describe('initUpdater state machine', () => { autoUpdaterMock.quitAndInstall.mockClear() autoUpdaterMock.autoRunAppAfterInstall = false updaterChannel = '' - vi.mocked(dialog.showMessageBox).mockResolvedValue({ response: 1, checkboxChecked: false }) + vi.mocked(dialog.showMessageBox).mockResolvedValue({ response: 0, checkboxChecked: false }) }) afterEach(() => { vi.useRealTimers() }) - it('walks check -> download -> ready and installs only from ready', async () => { + it('walks check -> validated download -> ready and installs only after confirmation', async () => { const { handle, states } = await createUpdater() expect(handle.getState()).toEqual({ status: 'idle' }) @@ -216,12 +218,30 @@ describe('initUpdater state machine', () => { ]) expect(dialog.showMessageBox).not.toHaveBeenCalled() expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + expect(autoUpdaterMock.autoDownload).toBe(false) + expect(autoUpdaterMock.downloadUpdate).toHaveBeenCalledTimes(1) + + handle.install() + await vi.advanceTimersByTimeAsync(0) + expect(dialog.showMessageBox).toHaveBeenCalledWith( + expect.objectContaining({ + buttons: ['Later', 'Restart and update'], + defaultId: 0, + cancelId: 0, + }) + ) + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ + response: 1, + checkboxChecked: false, + }) handle.install() + await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) - it('downloads, installs, and relaunches from one Update action', async () => { + it('downloads from an Update action and waits at ready for an explicit restart', async () => { autoUpdaterMock.autoDownload = false const { handle } = await createUpdater({ autoDownload: false }) @@ -241,11 +261,53 @@ describe('initUpdater state machine', () => { expect(handle.getState()).toEqual({ status: 'downloading', version: '2.0.0', percent: 42 }) emit('update-downloaded', { version: '2.0.0' }) - expect(dialog.showMessageBox).not.toHaveBeenCalled() expect(autoUpdaterMock.autoRunAppAfterInstall).toBe(true) + expect(handle.getState()).toEqual({ status: 'ready', version: '2.0.0' }) + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + }) + + it('keeps one restart confirmation in flight across repeated install requests', async () => { + let resolveConfirmation: (result: { response: number; checkboxChecked: boolean }) => void = + () => { + throw new Error('Restart confirmation did not initialize') + } + vi.mocked(dialog.showMessageBox).mockImplementationOnce( + () => + new Promise((resolve) => { + resolveConfirmation = resolve + }) + ) + const { handle } = await createUpdater() + + handle.check() + await vi.advanceTimersByTimeAsync(0) + emit('update-available', { version: '2.0.0' }) + emit('update-downloaded', { version: '2.0.0' }) + vi.mocked(dialog.showMessageBox).mockClear() + handle.install() + handle.install() + + expect(dialog.showMessageBox).toHaveBeenCalledTimes(1) + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + + resolveConfirmation({ response: 1, checkboxChecked: false }) + await vi.advanceTimersByTimeAsync(0) expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) + it('applies the download preference without enabling unvalidated library downloads', async () => { + const { handle } = await createUpdater() + handle.setAutoDownload(false) + + handle.check() + await vi.advanceTimersByTimeAsync(0) + emit('update-available', { version: '2.0.0' }) + + expect(autoUpdaterMock.autoDownload).toBe(false) + expect(handle.getState()).toEqual({ status: 'available', version: '2.0.0' }) + expect(autoUpdaterMock.downloadUpdate).not.toHaveBeenCalled() + }) + it('surfaces a manually started download failure without installing', async () => { autoUpdaterMock.downloadUpdate.mockRejectedValueOnce(new Error('download failed')) const { handle } = await createUpdater({ autoDownload: false }) @@ -276,6 +338,11 @@ describe('initUpdater state machine', () => { emit('update-available', { version: '2.0.0' }) handle.check() emit('update-downloaded', { version: '2.0.0' }) + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ + response: 1, + checkboxChecked: false, + }) + handle.install() await vi.advanceTimersByTimeAsync(0) expect(beforeInstall).toHaveBeenCalledTimes(1) @@ -286,6 +353,29 @@ describe('initUpdater state machine', () => { expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) + it('bypasses renderer unload guards only after teardown succeeds', async () => { + const setRelaunchPending = vi.fn() + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ + response: 1, + checkboxChecked: false, + }) + const { handle } = await createUpdater({ + beforeInstall: async () => {}, + setRelaunchPending, + }) + + handle.check() + await vi.advanceTimersByTimeAsync(0) + emit('update-available', { version: '2.0.0' }) + emit('update-downloaded', { version: '2.0.0' }) + handle.install() + + expect(setRelaunchPending).not.toHaveBeenCalled() + await vi.advanceTimersByTimeAsync(0) + expect(setRelaunchPending).toHaveBeenCalledWith(true) + expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) + }) + it('does not install when pre-install teardown fails', async () => { const beforeInstall = vi.fn(async () => { throw new Error('flush failed') @@ -296,6 +386,10 @@ describe('initUpdater state machine', () => { await vi.advanceTimersByTimeAsync(0) emit('update-available', { version: '2.0.0' }) emit('update-downloaded', { version: '2.0.0' }) + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ + response: 1, + checkboxChecked: false, + }) handle.install() await vi.advanceTimersByTimeAsync(0) @@ -304,6 +398,22 @@ describe('initUpdater state machine', () => { expect(handle.getState()).toEqual({ status: 'error', version: '2.0.0' }) }) + it('surfaces a staging error after a validated download is ready', async () => { + const { handle } = await createUpdater() + handle.check() + await vi.advanceTimersByTimeAsync(0) + emit('update-available', { version: '2.0.0' }) + emit('update-downloaded', { version: '2.0.0' }) + + emit('error', new Error('native staging failed')) + + expect(autoUpdaterMock.autoInstallOnAppQuit).toBe(false) + expect(handle.getState()).toEqual({ status: 'error', version: '2.0.0' }) + expect(events.record).toHaveBeenCalledWith('update_error', { + message: 'native staging failed', + }) + }) + it('checks from idle and ignores re-entrant checks while busy', async () => { const { handle } = await createUpdater() handle.check() @@ -424,6 +534,7 @@ describe('initUpdater state machine', () => { }) expect(handle.getState()).toEqual({ status: 'idle' }) + expect(autoUpdaterMock.downloadUpdate).not.toHaveBeenCalled() expect(autoUpdaterMock.autoInstallOnAppQuit).toBe(false) expect(events.record).toHaveBeenCalledWith('update_blocked_version', { version: '2.0.0', @@ -981,6 +1092,21 @@ describe('checkForUpdatesInteractive', () => { ) }) + it('opens the restart confirmation when an update is already ready', () => { + const handle: UpdaterHandle = { + setAutoDownload: () => {}, + getState: () => ({ status: 'ready', version: '2.0.0' }), + check: vi.fn(), + install: vi.fn(), + onState: () => () => {}, + } + + checkForUpdatesInteractive({ getWindow: () => null, events, handle }) + + expect(handle.install).toHaveBeenCalledTimes(1) + expect(handle.check).not.toHaveBeenCalled() + }) + it('only explains packaged-build updates when unpackaged', async () => { ;(app as unknown as { isPackaged: boolean }).isPackaged = false checkForUpdatesInteractive({ getWindow: () => null, events, handle: null }) diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index aaa2f3a360e..0de7d007f26 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -238,10 +238,7 @@ export interface UpdaterDeps { loadAutoUpdater?: () => typeof import('electron-updater')['autoUpdater'] /** Test seam: overrides the origin feed availability probe. */ probeOriginFeed?: (feedUrl: string) => Promise - /** - * Test seam: overrides Squirrel self-update capability detection (whether - * the running bundle carries a real Developer ID signature). - */ + /** Test seam: overrides Applications-folder and Developer ID eligibility detection. */ canSelfUpdate?: () => Promise /** Test seam: overrides the manual-mode manifest fetch (body or null). */ fetchManifest?: (url: string) => Promise @@ -249,6 +246,8 @@ export interface UpdaterDeps { platform?: NodeJS.Platform /** Flushes desktop-owned state before Squirrel terminates the process. */ beforeInstall?: () => Promise + /** Bypasses renderer unload guards only after the user confirms a relaunch. */ + setRelaunchPending?: (pending: boolean) => void } export interface UpdaterHandle { @@ -256,8 +255,8 @@ export interface UpdaterHandle { /** Current pipeline state for the renderer update UI. */ getState(): DesktopUpdateState /** - * Renderer-initiated advance: checks for an update, or starts the download - * when one is already known to be available (auto-download off / manual). + * Renderer-initiated advance: checks for an update, downloads an available + * self-update, or opens an available manual installer. */ check(): void /** @@ -285,7 +284,7 @@ export function isNewerVersion(candidateVersion: string, currentVersion: string) return isDowngrade(candidateVersion, currentVersion) } -/** A signed shell may only install a strictly newer build from its own environment stream. */ +/** Accepts only strictly newer builds from the running shell's environment stream. */ function isValidUpdateCandidate(candidateVersion: string, currentVersion: string): boolean { return ( resolveUpdateChannel(candidateVersion) === resolveUpdateChannel(currentVersion) && @@ -294,17 +293,17 @@ function isValidUpdateCandidate(candidateVersion: string, currentVersion: string } /** - * Whether Squirrel.Mac can swap this bundle in place. It validates a - * downloaded update against the running app's code signature, so only builds - * carrying a real Developer ID (a TeamIdentifier) can self-update. Local - * `install:local` builds and pre-signing CI prereleases are ad-hoc signed - * (`TeamIdentifier=not set`) and would fail the swap — those shells get the - * manual pipeline instead. + * Squirrel.Mac can update only an app installed under /Applications whose + * running bundle carries a Developer ID TeamIdentifier. Other packaged builds + * use the manual-download pipeline. */ async function detectSelfUpdateCapability(): Promise { if (process.platform !== 'darwin') { return true } + if (!app.isInApplicationsFolder()) { + return false + } const exe = app.getPath('exe') const bundleEnd = exe.indexOf('.app/') if (bundleEnd < 0) { @@ -342,11 +341,9 @@ interface UpdateEngine { * thirty minutes for production builds, and mirrors pipeline state to the * renderer for the settings update UI and the minimum-shell-version gate. * - * Developer-ID-signed builds use electron-updater (background download, - * then install and relaunch from an explicit Update action). Builds - * that can't self-update (ad-hoc signed: local installs, pre-signing CI - * prereleases) still poll the same feed but surface `available` as a manual - * download link, so the whole pipeline is testable before signing exists. + * Developer-ID-signed builds installed under /Applications use electron-updater. + * Other packaged builds still poll the same feed but surface available updates + * as manual downloads. */ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { if ((deps.platform ?? process.platform) !== 'darwin') { @@ -358,7 +355,6 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { const currentVersion = app.getVersion() let state: DesktopUpdateState = { status: 'idle' } - let installAfterDownload = false const listeners = new Set<(state: DesktopUpdateState) => void>() const setState = (next: DesktopUpdateState) => { state = next @@ -384,7 +380,9 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { autoUpdater.allowDowngrade = false } setChannelWithoutDowngrades(resolveUpdateChannel(currentVersion)) - autoUpdater.autoDownload = deps.autoDownload?.() ?? true + let autoDownloadEnabled = deps.autoDownload?.() ?? true + // Prevents the library from fetching a candidate before Sim validates its asset URLs. + autoUpdater.autoDownload = false // Explicit Update actions must reopen Sim after Squirrel swaps the bundle. autoUpdater.autoRunAppAfterInstall = true // Never install without vetting the downloaded version first. Enabled per @@ -394,18 +392,19 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { autoUpdater.autoInstallOnAppQuit = false autoUpdater.logger = null let installInFlight = false + let installConfirmationInFlight = false const quitAndInstall = () => { if (installInFlight) return - if (!deps.beforeInstall) { - autoUpdater.quitAndInstall() - return - } installInFlight = true void Promise.resolve() .then(() => deps.beforeInstall?.()) - .then(() => autoUpdater.quitAndInstall()) + .then(() => { + deps.setRelaunchPending?.(true) + autoUpdater.quitAndInstall() + }) .catch((error) => { + deps.setRelaunchPending?.(false) autoUpdater.autoInstallOnAppQuit = false installInFlight = false logger.error('Pre-install teardown failed', { @@ -416,6 +415,39 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { }) } + const confirmAndInstall = () => { + if (installInFlight || installConfirmationInFlight || state.status !== 'ready') return + installConfirmationInFlight = true + const version = state.version + const options: Electron.MessageBoxOptions = { + type: 'question', + buttons: ['Later', 'Restart and update'], + defaultId: 0, + cancelId: 0, + message: version ? `Restart to install Sim ${version}?` : 'Restart to update Sim?', + detail: + 'Sim will close all app windows while it updates. Running terminal commands, browser activity, downloads, uploads, and unsaved edits may be interrupted. Choose Later to install the update the next time you quit Sim.', + } + const win = deps.getWindow() + const confirmation = win + ? dialog.showMessageBox(win, options) + : dialog.showMessageBox(options) + void confirmation + .then(({ response }) => { + if (response === 1 && state.status === 'ready' && state.version === version) { + quitAndInstall() + } + }) + .catch((error) => { + logger.warn('Could not show update restart confirmation', { + message: getErrorMessage(error, 'unknown'), + }) + }) + .finally(() => { + installConfirmationInFlight = false + }) + } + let activeProbeId: number | null = null let nextProbeId = 0 let probeTimeout: ReturnType | null = null @@ -452,7 +484,6 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { if (checkId === null) return finishUpdaterCheck(checkId) if (updaterRequestId === checkId) updaterRequestId = null - installAfterDownload = false setState({ status: 'idle' }) }) @@ -468,7 +499,6 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { info.files.every((file) => isReleaseAssetUrl(file.url, info.version, channel))) if (!isValidUpdateCandidate(info.version, currentVersion) || !validOriginAssets) { acceptedUpdateVersion = null - installAfterDownload = false autoUpdater.autoInstallOnAppQuit = false deps.events.record('update_blocked_version', { version: info.version, @@ -479,12 +509,17 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { } acceptedUpdateVersion = info.version deps.events.record('update_check', { available: info.version }) - // With auto-download on, download-progress events follow immediately; - // `available` is the terminal state only when downloads are manual. setState({ - status: autoUpdater.autoDownload ? 'downloading' : 'available', + status: autoDownloadEnabled ? 'downloading' : 'available', version: info.version, }) + if (autoDownloadEnabled) { + void autoUpdater.downloadUpdate().catch((error) => { + logger.warn('Update download failed', { message: getErrorMessage(error, 'unknown') }) + deps.events.record('update_error', { message: getErrorMessage(error, 'unknown') }) + setState({ status: 'error', version: info.version }) + }) + } }) autoUpdater.on('download-progress', (progress) => { @@ -497,13 +532,12 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { }) autoUpdater.on('update-downloaded', (info) => { - if (state.status !== 'downloading' && !installAfterDownload) return + if (state.status !== 'downloading') return if ( acceptedUpdateVersion !== info.version || !isValidUpdateCandidate(info.version, currentVersion) ) { acceptedUpdateVersion = null - installAfterDownload = false autoUpdater.autoInstallOnAppQuit = false deps.events.record('update_blocked_version', { version: info.version }) setState({ status: 'idle' }) @@ -513,10 +547,6 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { autoUpdater.autoInstallOnAppQuit = true deps.events.record('update_downloaded', { version: info.version }) setState({ status: 'ready', version: info.version }) - if (installAfterDownload) { - installAfterDownload = false - quitAndInstall() - } }) autoUpdater.on('error', (error) => { @@ -524,10 +554,12 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { if (checkId !== null) { finishUpdaterCheck(checkId) if (updaterRequestId === checkId) updaterRequestId = null - } else if (state.status !== 'downloading') { + } else if (state.status !== 'downloading' && state.status !== 'ready' && !installInFlight) { return } - installAfterDownload = false + installInFlight = false + deps.setRelaunchPending?.(false) + autoUpdater.autoInstallOnAppQuit = false deps.events.record('update_error', { message: getErrorMessage(error, 'unknown') }) setState({ status: 'error', version: state.version }) }) @@ -662,16 +694,16 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { advance() { setState({ status: 'downloading', version: state.version }) autoUpdater.downloadUpdate().catch((error) => { - installAfterDownload = false logger.warn('Update download failed', { message: getErrorMessage(error, 'unknown') }) + deps.events.record('update_error', { message: getErrorMessage(error, 'unknown') }) setState({ status: 'error', version: state.version }) }) }, install() { - quitAndInstall() + confirmAndInstall() }, setAutoDownload(enabled) { - autoUpdater.autoDownload = enabled + autoDownloadEnabled = enabled }, } } @@ -830,7 +862,6 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { return } if (state.status === 'available') { - installAfterDownload = !state.manual engine.advance() return } @@ -917,7 +948,7 @@ export function checkForUpdatesInteractive( }) return case 'ready': - // The download pipeline already shows its own restart prompt. + handle.install() return case 'error': void showDialog({ diff --git a/apps/desktop/src/main/window.test.ts b/apps/desktop/src/main/window.test.ts index b529d9a37c4..a92e177bc5a 100644 --- a/apps/desktop/src/main/window.test.ts +++ b/apps/desktop/src/main/window.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) -import { BrowserWindow, dialog, systemPreferences } from 'electron' +import { BrowserWindow, dialog, screen, systemPreferences } from 'electron' import type { ConfigStore } from '@/main/config' import type { EventRecorder } from '@/main/observability' import { @@ -10,6 +10,7 @@ import { createMainWindow, createSecureWebPreferences, ensureMicrophoneAccess, + fitBoundsToWorkArea, resolvePermission, sanitizeBounds, setupPermissionHandlers, @@ -24,8 +25,8 @@ describe('resolvePermission', () => { expect(resolvePermission('clipboard-sanitized-write', '', APP)).toBe(false) }) - it('allows clipboard reads from the trusted origin only, so terminal Paste works', () => { - expect(resolvePermission('clipboard-read', APP, APP)).toBe(true) + it('denies clipboard reads, including from the trusted origin', () => { + expect(resolvePermission('clipboard-read', APP, APP)).toBe(false) expect(resolvePermission('clipboard-read', 'https://evil.example', APP)).toBe(false) expect(resolvePermission('clipboard-read', '', APP)).toBe(false) }) @@ -166,13 +167,13 @@ describe('setupPermissionHandlers', () => { expect(systemPreferences.getMediaAccessStatus).not.toHaveBeenCalled() }) - it('answers a clipboard request synchronously', () => { + it('denies a clipboard read request synchronously', () => { const { request } = createSession() const callback = vi.fn() request(null, 'clipboard-read', callback, { requestingUrl: `${APP}/workspace` }) - expect(callback).toHaveBeenCalledWith(true) + expect(callback).toHaveBeenCalledWith(false) }) it('reports microphone as permitted on the check path', () => { @@ -217,6 +218,26 @@ describe('sanitizeBounds', () => { }) }) +describe('fitBoundsToWorkArea', () => { + it('clamps an off-screen window into the matched display work area', () => { + expect( + fitBoundsToWorkArea( + { x: 3000, y: -800, width: 1200, height: 800 }, + { x: 0, y: 25, width: 1440, height: 875 } + ) + ).toEqual({ x: 240, y: 25, width: 1200, height: 800 }) + }) + + it('shrinks oversized bounds to fit the available work area', () => { + expect( + fitBoundsToWorkArea( + { x: -200, y: -100, width: 1800, height: 1200 }, + { x: 0, y: 25, width: 1440, height: 875 } + ) + ).toEqual({ x: 0, y: 25, width: 1440, height: 875 }) + }) +}) + describe('createSecureWebPreferences', () => { it('locks down the renderer', () => { const prefs = createSecureWebPreferences('persist:sim', '/tmp/preload.cjs', true) @@ -246,9 +267,12 @@ describe('createSecureWebPreferences', () => { describe('createMainWindow', () => { beforeEach(() => { vi.clearAllMocks() + vi.mocked(screen.getDisplayMatching).mockReturnValue({ + workArea: { x: 0, y: 0, width: 1440, height: 900 }, + } as never) }) - function createTestWindow(isMandatoryRelaunchPending: () => boolean = () => false) { + function createTestWindow(isCommittedRelaunchPending: () => boolean = () => false) { const config = { filePath: '/tmp/settings.json', getOrigin: vi.fn(() => APP), @@ -268,7 +292,7 @@ describe('createMainWindow', () => { preloadPath: '/tmp/preload.cjs', isPackaged: false, onClosed: vi.fn(), - isMandatoryRelaunchPending, + isCommittedRelaunchPending, }) const contentHandlers = new Map( vi.mocked(win.webContents.on).mock.calls as unknown as Array< @@ -364,7 +388,7 @@ describe('createMainWindow', () => { preloadPath: '/tmp/preload.cjs', isPackaged: false, onClosed: vi.fn(), - isMandatoryRelaunchPending: () => false, + isCommittedRelaunchPending: () => false, platform: 'darwin', }) @@ -431,7 +455,7 @@ describe('createMainWindow', () => { preloadPath: '/tmp/preload.cjs', isPackaged: false, onClosed: vi.fn(), - isMandatoryRelaunchPending: () => false, + isCommittedRelaunchPending: () => false, restorePosition: false, }) @@ -441,5 +465,46 @@ describe('createMainWindow', () => { expect(MockBrowserWindow.lastOptions).toMatchObject({ width: 1200, height: 800 }) expect(MockBrowserWindow.lastOptions?.x).toBeUndefined() expect(MockBrowserWindow.lastOptions?.y).toBeUndefined() + expect(screen.getDisplayMatching).not.toHaveBeenCalled() + }) + + it('restores the first window within the closest connected display', () => { + const config = { + filePath: '/tmp/settings.json', + getOrigin: vi.fn(() => APP), + setOrigin: vi.fn(), + get: vi.fn(() => ({ x: 3000, y: -400, width: 1200, height: 800 })), + set: vi.fn(), + } as unknown as ConfigStore + vi.mocked(screen.getDisplayMatching).mockReturnValue({ + workArea: { x: 1440, y: 25, width: 1440, height: 875 }, + } as never) + + createMainWindow({ + config, + events: { filePath: '/tmp/events.jsonl', record: vi.fn() }, + appOrigin: () => APP, + partition: 'persist:sim', + preloadPath: '/tmp/preload.cjs', + isPackaged: false, + onClosed: vi.fn(), + isCommittedRelaunchPending: () => false, + }) + + const MockBrowserWindow = BrowserWindow as typeof BrowserWindow & { + lastOptions?: Record + } + expect(screen.getDisplayMatching).toHaveBeenCalledWith({ + x: 3000, + y: -400, + width: 1200, + height: 800, + }) + expect(MockBrowserWindow.lastOptions).toMatchObject({ + x: 1680, + y: 25, + width: 1200, + height: 800, + }) }) }) diff --git a/apps/desktop/src/main/window.ts b/apps/desktop/src/main/window.ts index dc9390056a5..90f93e6882c 100644 --- a/apps/desktop/src/main/window.ts +++ b/apps/desktop/src/main/window.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' -import type { Session, WebPreferences } from 'electron' -import { app, BrowserWindow, dialog, nativeTheme, systemPreferences } from 'electron' +import type { Event, Rectangle, Session, WebPreferences } from 'electron' +import { app, BrowserWindow, dialog, nativeTheme, screen, systemPreferences } from 'electron' import { type ConfigStore, isSafeInternalPath, type WindowBounds } from '@/main/config' import { isAppOrigin, isAuthSurfacePath } from '@/main/navigation' import type { EventRecorder } from '@/main/observability' @@ -52,12 +52,10 @@ export function createSecureWebPreferences( } /** - * The permission matrix: clipboard and microphone access for the trusted app - * origin, default-deny for everything else including unknown future - * permissions (camera and screen capture stay denied). + * The permission matrix: sanitized clipboard writes and microphone access for + * the trusted app origin, default-deny for everything else including unknown + * future permissions (clipboard reads, camera, and screen capture stay denied). * - * Clipboard reads are what the terminal's Paste action runs on — xterm has no - * native paste target to fall back to, so a denied read is a Paste that fails. * `media` is what the composer's voice input runs on, and is narrowed to * audio-only requests so a `getUserMedia({ video: true })` still gets nothing. * Both grants are scoped to the app's own origin, which already reaches far @@ -84,7 +82,7 @@ export function resolvePermission( mediaTypes.every((type) => type === 'audio') ) } - return permission === 'clipboard-sanitized-write' || permission === 'clipboard-read' + return permission === 'clipboard-sanitized-write' } /** @@ -190,6 +188,44 @@ export function sanitizeBounds(bounds: WindowBounds | undefined): WindowBounds | return bounds } +/** Keeps restored bounds fully visible within the display Electron matched to them. */ +export function fitBoundsToWorkArea(bounds: WindowBounds, workArea: Rectangle): WindowBounds { + const width = Math.min(bounds.width, workArea.width) + const height = Math.min(bounds.height, workArea.height) + const x = Math.min( + Math.max(bounds.x ?? workArea.x, workArea.x), + workArea.x + workArea.width - width + ) + const y = Math.min( + Math.max(bounds.y ?? workArea.y, workArea.y), + workArea.y + workArea.height - height + ) + return { x, y, width, height } +} + +/** Applies the shared renderer unload decision to main and child windows. */ +export function handleWillPreventUnload( + win: BrowserWindow, + event: Event, + committedRelaunchPending: boolean +): void { + if (committedRelaunchPending) { + event.preventDefault() + return + } + const choice = dialog.showMessageBoxSync(win, { + type: 'question', + buttons: ['Stay', 'Leave'], + defaultId: 0, + cancelId: 0, + message: 'Leave Sim?', + detail: 'Changes you made may not be saved.', + }) + if (choice === 1) { + event.preventDefault() + } +} + export interface CreateMainWindowDeps { config: ConfigStore events: EventRecorder @@ -199,7 +235,7 @@ export interface CreateMainWindowDeps { isPackaged: boolean onClosed: () => void /** A committed process restart must not be cancelled by a renderer's beforeunload handler. */ - isMandatoryRelaunchPending: () => boolean + isCommittedRelaunchPending: () => boolean onFullScreenChange?: (isFullScreen: boolean) => void /** * Restores the persisted screen position for the first window. Secondary @@ -219,13 +255,18 @@ export interface CreateMainWindowDeps { export function createMainWindow(deps: CreateMainWindowDeps): BrowserWindow { const bounds = sanitizeBounds(deps.config.get('windowBounds')) const restorePosition = deps.restorePosition ?? true + let restoredBounds = bounds + if (restorePosition && bounds?.x !== undefined && bounds.y !== undefined) { + const savedRectangle = { x: bounds.x, y: bounds.y, width: bounds.width, height: bounds.height } + restoredBounds = fitBoundsToWorkArea(bounds, screen.getDisplayMatching(savedRectangle).workArea) + } const platform = deps.platform ?? process.platform const win = new BrowserWindow({ title: WINDOW_TITLE, - width: bounds?.width ?? DEFAULT_WIDTH, - height: bounds?.height ?? DEFAULT_HEIGHT, - x: restorePosition ? bounds?.x : undefined, - y: restorePosition ? bounds?.y : undefined, + width: restoredBounds?.width ?? DEFAULT_WIDTH, + height: restoredBounds?.height ?? DEFAULT_HEIGHT, + x: restorePosition ? restoredBounds?.x : undefined, + y: restorePosition ? restoredBounds?.y : undefined, minWidth: MIN_WIDTH, minHeight: MIN_HEIGHT, // No separate title bar: the page renders full-bleed to the window's top @@ -285,21 +326,7 @@ export function createMainWindow(deps: CreateMainWindowDeps): BrowserWindow { }) win.webContents.on('will-prevent-unload', (event) => { - if (deps.isMandatoryRelaunchPending()) { - event.preventDefault() - return - } - const choice = dialog.showMessageBoxSync(win, { - type: 'question', - buttons: ['Stay', 'Leave'], - defaultId: 0, - cancelId: 0, - message: 'Leave Sim?', - detail: 'Changes you made may not be saved.', - }) - if (choice === 1) { - event.preventDefault() - } + handleWillPreventUnload(win, event, deps.isCommittedRelaunchPending()) }) let recoveryDialog: 'crash' | 'hang' | null = null diff --git a/apps/desktop/src/main/windows.test.ts b/apps/desktop/src/main/windows.test.ts index c508f8b47ae..98c11a32518 100644 --- a/apps/desktop/src/main/windows.test.ts +++ b/apps/desktop/src/main/windows.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('electron', () => import('@/test/electron-mock')) import type { WebContents } from 'electron' -import { shell } from 'electron' +import { dialog, shell } from 'electron' import { attachWindowOpenPolicy, isPopupContents, registerPopupContents } from '@/main/windows' const APP = 'https://sim.ai' @@ -29,14 +29,14 @@ describe('attachWindowOpenPolicy', () => { vi.mocked(shell.openExternal).mockClear() }) - function setup(isMandatoryRelaunchPending: () => boolean = () => false) { + function setup(isCommittedRelaunchPending: () => boolean = () => false) { const contents = makeContents() const openAppWindow = vi.fn() attachWindowOpenPolicy(contents as unknown as WebContents, { appOrigin: () => APP, openAppWindow, allowHttpLocalhost: false, - isMandatoryRelaunchPending, + isCommittedRelaunchPending, }) return { contents, openAppWindow } } @@ -98,7 +98,7 @@ describe('attachWindowOpenPolicy', () => { expect(didCreateWindow).toBeDefined() }) - it('allows a mandatory relaunch through a child beforeunload', () => { + it('allows a committed relaunch through a child beforeunload', () => { const { contents } = setup(() => true) const childContents = makeContents() const child = { webContents: childContents } @@ -114,7 +114,7 @@ describe('attachWindowOpenPolicy', () => { expect(event.preventDefault).toHaveBeenCalledOnce() }) - it('leaves child beforeunload untouched during ordinary use', () => { + it('asks before leaving a child window during ordinary use', () => { const { contents } = setup() const childContents = makeContents() const child = { webContents: childContents } @@ -128,6 +128,14 @@ describe('attachWindowOpenPolicy', () => { willPreventUnload?.[1](event) expect(event.preventDefault).not.toHaveBeenCalled() + expect(dialog.showMessageBoxSync).toHaveBeenCalledWith( + child, + expect.objectContaining({ + buttons: ['Stay', 'Leave'], + defaultId: 0, + cancelId: 0, + }) + ) }) }) diff --git a/apps/desktop/src/main/windows.ts b/apps/desktop/src/main/windows.ts index d5e84a971d7..fcb9c41f115 100644 --- a/apps/desktop/src/main/windows.ts +++ b/apps/desktop/src/main/windows.ts @@ -6,6 +6,7 @@ import { openExternalSafe, } from '@/main/navigation' import { scrubUrl } from '@/main/observability' +import { handleWillPreventUnload } from '@/main/window' const logger = createLogger('DesktopWindows') @@ -44,7 +45,7 @@ export interface WindowPolicyDeps { appOrigin: () => string openAppWindow: (url: string) => void allowHttpLocalhost: boolean - isMandatoryRelaunchPending: () => boolean + isCommittedRelaunchPending: () => boolean } /** @@ -81,9 +82,7 @@ export function attachWindowOpenPolicy(contents: WebContents, deps: WindowPolicy registerPopupContents(child.webContents) attachWindowOpenPolicy(child.webContents, deps) child.webContents.on('will-prevent-unload', (event) => { - if (deps.isMandatoryRelaunchPending()) { - event.preventDefault() - } + handleWillPreventUnload(child, event, deps.isCommittedRelaunchPending()) }) const kind = classifyWindowOpen(details.url, details.frameName, deps.appOrigin()) if (kind === 'popup-blank') { diff --git a/apps/desktop/src/test/electron-mock.ts b/apps/desktop/src/test/electron-mock.ts index bb4d81ddc57..2c27d51322f 100644 --- a/apps/desktop/src/test/electron-mock.ts +++ b/apps/desktop/src/test/electron-mock.ts @@ -19,6 +19,7 @@ export const app = { getPath: vi.fn(() => '/tmp/sim-desktop-test'), getAppPath: vi.fn(() => '/tmp/sim-desktop-test/app'), isReady: vi.fn(() => true), + isInApplicationsFolder: vi.fn(() => true), on: vi.fn(), once: vi.fn(), quit: vi.fn(), @@ -74,6 +75,12 @@ export const nativeTheme = { on: vi.fn(), } +export const screen = { + getDisplayMatching: vi.fn(() => ({ + workArea: { x: 0, y: 0, width: 1440, height: 900 }, + })), +} + export const Menu = { buildFromTemplate: vi.fn((template: unknown[]) => ({ popup: vi.fn(), items: template })), setApplicationMenu: vi.fn(), diff --git a/apps/sim/app/_shell/desktop-update-gate.tsx b/apps/sim/app/_shell/desktop-update-gate.tsx index a48cc3b2de8..ed231c04534 100644 --- a/apps/sim/app/_shell/desktop-update-gate.tsx +++ b/apps/sim/app/_shell/desktop-update-gate.tsx @@ -5,6 +5,7 @@ import type { DesktopUpdateState } from '@sim/desktop-bridge' import { Chip, useNativeSurfaceOcclusionReady } from '@sim/emcn' import { getDesktopBridge, getDesktopShellVersion, getDesktopUpdates } from '@/lib/desktop' import { isShellOutdated } from '@/lib/desktop/min-version' +import { useDesktopUpdateState } from '@/hooks/use-desktop-update-state' /** * Resolves this deployment's channel to the newest release's installer, so a @@ -70,21 +71,13 @@ function gateActionFor(state: DesktopUpdateState): GateAction { */ export function DesktopUpdateGate() { const [outdated, setOutdated] = useState(false) - const [updateState, setUpdateState] = useState({ status: 'idle' }) + const updateState = useDesktopUpdateState() useEffect(() => { if (!getDesktopBridge() || !isShellOutdated(getDesktopShellVersion())) { return } setOutdated(true) - const updates = getDesktopUpdates() - if (!updates) return - const unsubscribe = updates.onState(setUpdateState) - void updates - .getState() - .then(setUpdateState) - .catch(() => {}) - return unsubscribe }, []) const nativeSurfaceReady = useNativeSurfaceOcclusionReady(outdated, 'takeover') diff --git a/apps/sim/app/api/desktop/update/download/route.test.ts b/apps/sim/app/api/desktop/update/download/route.test.ts index c85a16cce05..43cc05b7e5d 100644 --- a/apps/sim/app/api/desktop/update/download/route.test.ts +++ b/apps/sim/app/api/desktop/update/download/route.test.ts @@ -108,4 +108,13 @@ describe('desktop update download route', () => { expect(response.status).toBe(502) expect(await response.json()).toMatchObject({ error: 'Release feed unavailable' }) }) + + it('surfaces a GitHub network failure instead of returning an internal error', async () => { + fetchMock.mockRejectedValueOnce(new Error('network unavailable')) + + const response = await getDownload() + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release feed unavailable' }) + }) }) diff --git a/apps/sim/app/api/desktop/update/download/route.ts b/apps/sim/app/api/desktop/update/download/route.ts index 520e4761452..2f9eb070dc3 100644 --- a/apps/sim/app/api/desktop/update/download/route.ts +++ b/apps/sim/app/api/desktop/update/download/route.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { type NextRequest, NextResponse } from 'next/server' import { env } from '@/lib/core/config/env' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' @@ -34,32 +35,42 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise { - const response = await fetch(releasesApiUrl(releaseRepository, page), { - headers: { - accept: 'application/vnd.github+json', - ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), - }, - next: { revalidate: REVALIDATE_SECONDS }, - }) - if (!response.ok) { - logger.error('GitHub releases lookup failed', { - status: response.status, + try { + const response = await fetch(releasesApiUrl(releaseRepository, page), { + headers: { + accept: 'application/vnd.github+json', + ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), + }, + next: { revalidate: REVALIDATE_SECONDS }, + }) + if (!response.ok) { + logger.error('GitHub releases lookup failed', { + status: response.status, + page, + channel, + releaseRepository, + }) + return null + } + return (await response.json()) as DesktopReleaseCandidate[] + } catch (error) { + logger.error('GitHub releases response could not be read', { + message: getErrorMessage(error), page, channel, releaseRepository, }) return null } - return (await response.json()) as DesktopReleaseCandidate[] }) if ('error' in resolved) { return NextResponse.json({ error: 'Release feed unavailable' }, { status: 502 }) } const release = resolved.release - const asset = release ? selectInstallerAsset(release) : null + const asset = release ? selectInstallerAsset(release, releaseRepository) : null if (!release || !asset) { if (release) { logger.error('Release has no installer artifact', { tag: release.tag_name, channel }) diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts index 256551d910a..5daf8b843b9 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts @@ -32,7 +32,7 @@ function release(tag: string) { } function manifest(version: string) { - return [`version: ${version}`, 'files:', ` - url: Sim-${version}-universal-mac.zip`].join('\n') + return [`version: ${version}`, 'files:', ` - url: Sim-${version}-universal.zip`].join('\n') } async function getFeed(hostname: string, headers?: HeadersInit): Promise { @@ -84,7 +84,7 @@ describe('desktop update manifest route', () => { expect(response.headers.get(FEED_STATUS_HEADER)).toBe('release') expect(body).toContain(`version: ${version}`) expect(body).toContain( - `https://github.com/${repository}/releases/download/${tag}/Sim-${version}-universal-mac.zip` + `https://github.com/${repository}/releases/download/${tag}/Sim-${version}-universal.zip` ) } ) @@ -225,4 +225,24 @@ describe('desktop update manifest route', () => { expect(await response.json()).toMatchObject({ error: 'Release manifest unavailable' }) expect(fetchMock).toHaveBeenNthCalledWith(1, PRERELEASE_RELEASES_URL, expect.any(Object)) }) + + it('rejects an oversized updater manifest', async () => { + fetchMock + .mockResolvedValueOnce(Response.json([release('v1.1.0')])) + .mockResolvedValueOnce(new Response(new Uint8Array(256 * 1024 + 1))) + + const response = await getFeed('www.sim.ai') + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release manifest unavailable' }) + }) + + it('surfaces malformed GitHub release data as a feed failure', async () => { + fetchMock.mockResolvedValueOnce(new Response('not json')) + + const response = await getFeed('www.sim.ai') + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release feed unavailable' }) + }) }) diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts index 6c443caa43e..8b4a15ffffa 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts @@ -1,11 +1,14 @@ import { createLogger } from '@sim/logger' +import { getErrorMessage } from '@sim/utils/errors' import { type NextRequest, NextResponse } from 'next/server' import { env } from '@/lib/core/config/env' +import { readResponseTextWithLimit } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { channelForDeploymentEnvironment, type DesktopReleaseCandidate, MANIFEST_ASSET_NAME, + MAX_DESKTOP_UPDATE_MANIFEST_BYTES, releaseRepositoryForChannel, releasesApiUrl, resolveLatestRelease, @@ -41,25 +44,35 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise { - const response = await fetch(releasesApiUrl(releaseRepository, page), { - headers: { - accept: 'application/vnd.github+json', - ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), - }, - next: { revalidate: REVALIDATE_SECONDS }, - }) - if (!response.ok) { - logger.error('GitHub releases lookup failed', { - status: response.status, + try { + const response = await fetch(releasesApiUrl(releaseRepository, page), { + headers: { + accept: 'application/vnd.github+json', + ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), + }, + next: { revalidate: REVALIDATE_SECONDS }, + }) + if (!response.ok) { + logger.error('GitHub releases lookup failed', { + status: response.status, + page, + channel, + releaseRepository, + }) + return null + } + return (await response.json()) as DesktopReleaseCandidate[] + } catch (error) { + logger.error('GitHub releases response could not be read', { + message: getErrorMessage(error), page, channel, releaseRepository, }) return null } - return (await response.json()) as DesktopReleaseCandidate[] }) if ('error' in resolved) { return NextResponse.json({ error: 'Release feed unavailable' }, { status: 502 }) @@ -78,8 +91,6 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise candidate.name === MANIFEST_ASSET_NAME) if (!asset) { - // selectReleaseForChannel already skips assetless releases, so this only - // fires when the API response omitted assets entirely. logger.error('Release is missing its updater manifest', { tag: release.tag_name, channel, @@ -97,7 +108,19 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise(null) const [pendingPreference, setPendingPreference] = useState(null) - const [updateState, setUpdateState] = useState({ status: 'idle' }) - const [shellVersion, setShellVersion] = useState(undefined) + const updateState = useDesktopUpdateState() + const shellVersion = getDesktopShellVersion() useEffect(() => { const bridge = getDesktopBridge() @@ -50,19 +47,7 @@ export function Desktop() { .catch(() => toast.error('Could not load desktop settings')) }, [router, workspaceId]) - useEffect(() => { - setShellVersion(getDesktopShellVersion()) - const updates = getDesktopUpdates() - if (!updates) return - const unsubscribe = updates.onState(setUpdateState) - void updates - .getState() - .then(setUpdateState) - .catch(() => {}) - return unsubscribe - }, []) - - const updatePreference = useCallback(async (key: DesktopPreferenceKey, value: boolean) => { + const updatePreference = async (key: DesktopPreferenceKey, value: boolean) => { const settings = getDesktopBridge()?.settings if (!settings) return setPendingPreference(key) @@ -73,7 +58,7 @@ export function Desktop() { } finally { setPendingPreference(null) } - }, []) + } if (!preferences) { return null @@ -88,7 +73,9 @@ export function Desktop() {
{shellVersion && (
- + {updateState.status === 'ready' && updateState.version ? `${shellVersion} → ${updateState.version} on restart` diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx index 58bc39d2a15..c72aeec6c1b 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.test.tsx @@ -175,7 +175,7 @@ describe('SidebarFooter', () => { expect(helpTrigger()).toHaveClass('h-[30px]', 'px-2') expect(helpTrigger()).not.toHaveClass('bg-[var(--text-primary)]') expect(helpTrigger().querySelector('circle')).not.toBeInTheDocument() - expect(helpTrigger().querySelector('span')).toHaveClass( + expect(helpTrigger().querySelector('div')).toHaveClass( 'size-[17px]', 'rounded-full', 'bg-[var(--text-primary)]' @@ -193,15 +193,25 @@ describe('SidebarFooter', () => { expect(desktopMocks.install).not.toHaveBeenCalled() }) + it('uses a collapsed-sidebar-safe element for the update icon', async () => { + await renderFooter( + { status: 'available', version: '1.4.0' }, + { isCollapsed: true, showCollapsedTooltips: true } + ) + + expect(helpTrigger().querySelector('div')).toHaveClass('size-[17px]') + expect(helpTrigger().querySelector('span')).toBeNull() + }) + it('turns the menu action into restart-and-install when the update is ready', async () => { await renderFooter({ status: 'idle' }) act(() => { desktopMocks.listener?.({ status: 'ready', version: '1.4.0' }) }) - expect(helpTrigger().querySelector('span')).toHaveClass('bg-[var(--text-primary)]') + expect(helpTrigger().querySelector('div')).toHaveClass('bg-[var(--text-primary)]') openHelpMenu() - act(() => menuItem('Update').click()) + act(() => menuItem('Restart to update').click()) expect(desktopMocks.install).toHaveBeenCalledTimes(1) expect(desktopMocks.check).not.toHaveBeenCalled() diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx index fc4c8902929..efc79d2e73e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/sidebar-footer/sidebar-footer.tsx @@ -1,6 +1,6 @@ 'use client' -import { type ComponentType, useEffect, useState } from 'react' +import type { ComponentType } from 'react' import type { DesktopUpdateState } from '@sim/desktop-bridge' import { Chip, @@ -33,6 +33,7 @@ import { } from '@/app/workspace/[workspaceId]/w/components/sidebar/constants' import { SidebarTooltip } from '@/app/workspace/[workspaceId]/w/components/sidebar/sidebar' import { useUserProfile } from '@/hooks/queries/user-profile' +import { useDesktopUpdateState } from '@/hooks/use-desktop-update-state' import { useWorkspaceInvitePolicy } from '@/hooks/use-workspace-invite-policy' /** @@ -65,13 +66,13 @@ function desktopUpdateActionLabel(state: DesktopUpdateState): string { ? 'Downloading update…' : `Downloading update ${state.percent}%` } - return 'Update' + return state.status === 'ready' ? 'Restart to update' : 'Update' } /** Compact primary update circle using the same footprint as the surrounding sidebar icons. */ function DesktopUpdateIcon({ className }: { className?: string }) { return ( - - +
) } @@ -133,25 +134,7 @@ export function SidebarFooter({ const { data: session } = useSession() const hostContext = useWorkspaceHostContext() const { isInvitationsDisabled } = useWorkspaceInvitePolicy(workspaceId) - const [updateState, setUpdateState] = useState({ status: 'idle' }) - - useEffect(() => { - const updates = getDesktopUpdates() - if (!updates) return - - let stateEventReceived = false - const unsubscribe = updates.onState((state) => { - stateEventReceived = true - setUpdateState(state) - }) - void updates - .getState() - .then((state) => { - if (!stateEventReceived) setUpdateState(state) - }) - .catch(() => {}) - return unsubscribe - }, []) + const updateState = useDesktopUpdateState() const name = profile ? profile.name?.trim() || profile.email : '' const updateAvailable = hasAvailableDesktopUpdate(updateState) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx index da5bdaeed8e..46a2e7b6a6f 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx @@ -854,15 +854,12 @@ export const Sidebar = memo(function Sidebar({ files: { hover: filesHover, content: }, } - const handleOpenSettings = useCallback( - (section: SettingsSection) => { - if (!isCollapsedRef.current) { - setSidebarWidth(SIDEBAR_WIDTH.MIN) - } - navigateToSettings({ section }) - }, - [navigateToSettings, setSidebarWidth] - ) + const handleOpenSettings = (section: SettingsSection) => { + if (!isCollapsedRef.current) { + setSidebarWidth(SIDEBAR_WIDTH.MIN) + } + navigateToSettings({ section }) + } const { data: fetchedChats = EMPTY_CHATS, isLoading: chatsLoading } = useMothershipChats( workspaceId, @@ -1239,17 +1236,17 @@ export const Sidebar = memo(function Sidebar({ [isCollapsed, toggleCollapsed] ) - const handleOpenHelpFromMenu = useCallback(() => setIsHelpModalOpen(true), []) + const handleOpenHelpFromMenu = () => setIsHelpModalOpen(true) - const handleOpenDocs = useCallback(() => { + const handleOpenDocs = () => { window.open('https://docs.sim.ai', '_blank', 'noopener,noreferrer') captureEvent(posthog, 'docs_opened', { source: 'help_menu' }) - }, [posthog]) + } - const handleOpenSlackCommunity = useCallback(() => { + const handleOpenSlackCommunity = () => { window.open(SLACK_COMMUNITY_URL, '_blank', 'noopener,noreferrer') captureEvent(posthog, 'slack_community_opened', { source: 'help_menu' }) - }, [posthog]) + } const handleChatRenameBlur = useCallback( () => void chatFlyoutRename.saveRename(), diff --git a/apps/sim/hooks/use-desktop-update-state.test.tsx b/apps/sim/hooks/use-desktop-update-state.test.tsx new file mode 100644 index 00000000000..0d67de0d5b2 --- /dev/null +++ b/apps/sim/hooks/use-desktop-update-state.test.tsx @@ -0,0 +1,90 @@ +/** + * @vitest-environment jsdom + */ + +import { act } from 'react' +import type { DesktopUpdateState } from '@sim/desktop-bridge' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const desktopMocks = vi.hoisted(() => ({ + getState: vi.fn(), + onState: vi.fn(), + unsubscribe: vi.fn(), + listener: null as ((state: DesktopUpdateState) => void) | null, +})) + +vi.mock('@/lib/desktop', () => ({ + getDesktopUpdates: () => ({ + getState: desktopMocks.getState, + onState: desktopMocks.onState, + }), +})) + +import { useDesktopUpdateState } from '@/hooks/use-desktop-update-state' + +let container: HTMLDivElement +let root: Root +let currentState: DesktopUpdateState + +function Harness() { + currentState = useDesktopUpdateState() + return null +} + +describe('useDesktopUpdateState', () => { + beforeEach(() => { + vi.clearAllMocks() + desktopMocks.listener = null + desktopMocks.onState.mockImplementation((listener) => { + desktopMocks.listener = listener + return desktopMocks.unsubscribe + }) + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + }) + + afterEach(() => { + if (container.isConnected) { + act(() => root.unmount()) + container.remove() + } + }) + + it('does not let a stale snapshot replace a newer state event', async () => { + let resolveSnapshot: (state: DesktopUpdateState) => void = () => { + throw new Error('Update-state snapshot did not initialize') + } + desktopMocks.getState.mockReturnValue( + new Promise((resolve) => { + resolveSnapshot = resolve + }) + ) + await act(async () => root.render()) + + act(() => desktopMocks.listener?.({ status: 'ready', version: '2.0.0' })) + await act(async () => resolveSnapshot({ status: 'checking' })) + + expect(currentState).toEqual({ status: 'ready', version: '2.0.0' }) + }) + + it('unsubscribes and ignores a snapshot after unmount', async () => { + let resolveSnapshot: (state: DesktopUpdateState) => void = () => { + throw new Error('Update-state snapshot did not initialize') + } + desktopMocks.getState.mockReturnValue( + new Promise((resolve) => { + resolveSnapshot = resolve + }) + ) + await act(async () => root.render()) + act(() => root.unmount()) + container.remove() + + await act(async () => resolveSnapshot({ status: 'ready', version: '2.0.0' })) + + expect(desktopMocks.unsubscribe).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/sim/hooks/use-desktop-update-state.ts b/apps/sim/hooks/use-desktop-update-state.ts new file mode 100644 index 00000000000..32ccb904f1a --- /dev/null +++ b/apps/sim/hooks/use-desktop-update-state.ts @@ -0,0 +1,37 @@ +'use client' + +import { useEffect, useState } from 'react' +import type { DesktopUpdateState } from '@sim/desktop-bridge' +import { getDesktopUpdates } from '@/lib/desktop' + +const INITIAL_UPDATE_STATE: DesktopUpdateState = { status: 'idle' } + +export function useDesktopUpdateState(): DesktopUpdateState { + const [state, setState] = useState(INITIAL_UPDATE_STATE) + + useEffect(() => { + const updates = getDesktopUpdates() + if (!updates) return + + let active = true + let eventReceived = false + const unsubscribe = updates.onState((next) => { + if (!active) return + eventReceived = true + setState(next) + }) + void updates + .getState() + .then((next) => { + if (active && !eventReceived) setState(next) + }) + .catch(() => {}) + + return () => { + active = false + unsubscribe() + } + }, []) + + return state +} diff --git a/apps/sim/lib/desktop/update-feed.test.ts b/apps/sim/lib/desktop/update-feed.test.ts index 0d899e39a42..aba3385a9fe 100644 --- a/apps/sim/lib/desktop/update-feed.test.ts +++ b/apps/sim/lib/desktop/update-feed.test.ts @@ -121,9 +121,9 @@ describe('selectReleaseForChannel', () => { expect(selectReleaseForChannel(withBrokenNewest, 'dev')?.tag_name).toBe('v0.5.25-dev.412') }) - it('tolerates release listings without asset data', () => { - const bare = { tag_name: 'v0.5.24', draft: false, prerelease: false } - expect(selectReleaseForChannel([bare], 'latest')?.tag_name).toBe('v0.5.24') + it('skips release listings without asset data', () => { + const bare = { tag_name: 'v0.5.25', draft: false, prerelease: false } + expect(selectReleaseForChannel([bare, release('v0.5.24')], 'latest')?.tag_name).toBe('v0.5.24') }) it('skips drafts and unparseable tags', () => { @@ -140,27 +140,32 @@ describe('rewriteManifestUrls', () => { const manifest = [ 'version: 0.5.24', 'files:', - ' - url: Sim-0.5.24-universal-mac.zip', + ' - url: Sim-0.5.24-universal.zip', ' sha512: abc', ' size: 123', - 'path: Sim-0.5.24-universal-mac.zip', + 'path: Sim-0.5.24-universal.zip', 'sha512: abc', "releaseDate: '2026-07-23T00:00:00.000Z'", ].join('\n') const rewritten = rewriteManifestUrls(manifest, 'v0.5.24', repository) expect(rewritten).toContain( - ` - url: https://github.com/${repository}/releases/download/v0.5.24/Sim-0.5.24-universal-mac.zip` + ` - url: https://github.com/${repository}/releases/download/v0.5.24/Sim-0.5.24-universal.zip` ) expect(rewritten).toContain( - `path: https://github.com/${repository}/releases/download/v0.5.24/Sim-0.5.24-universal-mac.zip` + `path: https://github.com/${repository}/releases/download/v0.5.24/Sim-0.5.24-universal.zip` ) expect(rewritten).toContain('sha512: abc') }) - it('leaves already-absolute URLs alone', () => { - const manifest = ' - url: https://cdn.example.com/Sim.zip' + it('canonicalizes an expected absolute asset URL', () => { + const manifest = ' - url: https://cdn.example.com/Sim-0.5.24-universal.zip' expect(rewriteManifestUrls(manifest, 'v0.5.24', DESKTOP_STABLE_RELEASE_REPOSITORY)).toBe( - manifest + ` - url: https://github.com/${DESKTOP_STABLE_RELEASE_REPOSITORY}/releases/download/v0.5.24/Sim-0.5.24-universal.zip` ) }) + + it('rejects unexpected manifest asset names', () => { + const manifest = ' - url: https://cdn.example.com/unreviewed.zip' + expect(rewriteManifestUrls(manifest, 'v0.5.24', DESKTOP_STABLE_RELEASE_REPOSITORY)).toBeNull() + }) }) diff --git a/apps/sim/lib/desktop/update-feed.ts b/apps/sim/lib/desktop/update-feed.ts index ed8d14c16f3..49723afcad3 100644 --- a/apps/sim/lib/desktop/update-feed.ts +++ b/apps/sim/lib/desktop/update-feed.ts @@ -66,6 +66,7 @@ export function channelOfVersion(version: string): DesktopUpdateChannel { * release belongs to is carried entirely by its tag. */ export const MANIFEST_ASSET_NAME = 'latest-mac.yml' +export const MAX_DESKTOP_UPDATE_MANIFEST_BYTES = 256 * 1024 /** The subset of the GitHub releases API the feed needs. */ export interface DesktopReleaseCandidate { @@ -95,7 +96,7 @@ export function selectReleaseForChannel( // Defense in depth: a bare vX.Y.Z tag manually marked "pre-release" on // GitHub must not reach stable clients. if (channel === 'latest' && release.prerelease) continue - if (release.assets && !release.assets.some((asset) => asset.name === MANIFEST_ASSET_NAME)) { + if (!release.assets?.some((asset) => asset.name === MANIFEST_ASSET_NAME)) { continue } if (best === null) { @@ -125,14 +126,32 @@ export function rewriteManifestUrls( manifest: string, tag: string, repository: DesktopReleaseRepository -): string { +): string | null { const base = `https://github.com/${repository}/releases/download/${tag}/` - return manifest.replace(/^(\s*(?:-\s*)?(?:url|path):\s*)(\S+)\s*$/gm, (line, prefix, value) => { - if (value.startsWith('http://') || value.startsWith('https://')) { - return line + const version = tag.replace(/^v/, '') + const expectedNames = new Set([`Sim-${version}-universal.dmg`, `Sim-${version}-universal.zip`]) + let valid = true + const rewritten = manifest.replace( + /^(\s*(?:-\s*)?(?:url|path):\s*)(\S+)\s*$/gm, + (_line, prefix: string, value: string) => { + try { + const pathname = + value.startsWith('http://') || value.startsWith('https://') + ? new URL(value).pathname + : value + const name = decodeURIComponent(pathname.split('/').at(-1) ?? '') + if (!expectedNames.has(name)) { + valid = false + return '' + } + return `${prefix}${base}${encodeURIComponent(name)}` + } catch { + valid = false + return '' + } } - return `${prefix}${base}${encodeURIComponent(value)}` - }) + ) + return valid ? rewritten : null } /** @@ -189,12 +208,19 @@ export async function resolveLatestRelease( * web-app and SDK tags that carry no desktop artifact at all. */ export function selectInstallerAsset( - release: DesktopReleaseCandidate + release: DesktopReleaseCandidate, + repository: DesktopReleaseRepository ): { name: string; browser_download_url: string } | null { const assets = release.assets ?? [] - return ( - assets.find((asset) => asset.name.endsWith('.dmg')) ?? - assets.find((asset) => asset.name.endsWith('.zip')) ?? - null - ) + const version = release.tag_name.replace(/^v/, '') + const dmgName = `Sim-${version}-universal.dmg` + const zipName = `Sim-${version}-universal.zip` + const asset = + assets.find((candidate) => candidate.name === dmgName) ?? + assets.find((candidate) => candidate.name === zipName) + if (!asset) return null + return { + name: asset.name, + browser_download_url: `https://github.com/${repository}/releases/download/${release.tag_name}/${asset.name}`, + } } diff --git a/packages/desktop-bridge/contract-snapshot.ts b/packages/desktop-bridge/contract-snapshot.ts index b4f37265219..e044e056469 100644 --- a/packages/desktop-bridge/contract-snapshot.ts +++ b/packages/desktop-bridge/contract-snapshot.ts @@ -75,6 +75,15 @@ export type BrowserToolName = (typeof BROWSER_TOOL_NAMES)[number] export const BROWSER_WAIT_FOR_DEFAULT_TIMEOUT_MS = 10_000 export const BROWSER_WAIT_FOR_MAX_TIMEOUT_MS = 120_000 export const BROWSER_WAIT_FOR_RENDERER_GRACE_MS = 15_000 +export const BROWSER_TOOL_AUTHORIZATION_TIMEOUT_MS = 8_000 +export const BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS = 60_000 +export const BROWSER_TOOL_QUEUE_WAIT_TIMEOUT_MS = BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS +const BROWSER_RENDERER_TRANSPORT_GRACE_MS = 2_000 +export const BROWSER_NAVIGATION_RENDERER_TIMEOUT_MS = + BROWSER_TOOL_AUTHORIZATION_TIMEOUT_MS + + BROWSER_TOOL_QUEUE_WAIT_TIMEOUT_MS + + BROWSER_NAVIGATION_NATIVE_WATCHDOG_MS + + BROWSER_RENDERER_TRANSPORT_GRACE_MS /** * Normalizes the model-visible `browser_wait_for.timeoutMs` consistently in @@ -1819,9 +1828,9 @@ export interface SimDesktopTerminalThemesApi { } /** - * Where the shell's update pipeline currently is. `available` only occurs - * when automatic downloads are disabled; with them enabled the shell moves - * straight to `downloading`. + * Where the shell's update pipeline currently is. `available` occurs when + * automatic downloads are disabled or the shell requires a manual installer; + * self-updating shells with automatic downloads enabled move to `downloading`. */ export type DesktopUpdateStatus = | 'idle' @@ -1838,11 +1847,9 @@ export interface DesktopUpdateState { /** Whole-number download progress (0-100) while `downloading`. */ percent?: number /** - * True when this shell cannot apply updates in place (a build without a - * Developer ID signature — local installs and pre-signing CI prereleases; - * Squirrel.Mac refuses to swap unsigned bundles). `available` is then the - * pipeline's terminal state and the advance action opens the download in - * the browser instead of downloading in the background. + * True when this shell cannot apply updates in place, such as an unsigned build + * or an app running outside /Applications. `available` is then the terminal state + * and the advance action opens the installer in the browser. */ manual?: boolean } @@ -1851,11 +1858,11 @@ export interface DesktopUpdateState { export interface SimDesktopUpdatesApi { getState(): Promise /** - * Advance the pipeline: checks for an update, or starts the download when - * one is already known to be available (auto-download off). + * Advances the pipeline: checks for an update, downloads an available + * self-update, or opens an available manual installer. */ check(): void - /** Quit and install a `ready` update. No-op in any other state. */ + /** Installs a ready update or opens the installer for an available manual update. */ install(): void /** Subscribe to pipeline state changes. Returns an unsubscribe function. */ onState(callback: (state: DesktopUpdateState) => void): () => void diff --git a/packages/desktop-bridge/src/index.ts b/packages/desktop-bridge/src/index.ts index 77f3f6208d9..cb8dc0926bf 100644 --- a/packages/desktop-bridge/src/index.ts +++ b/packages/desktop-bridge/src/index.ts @@ -956,9 +956,9 @@ export interface SimDesktopTerminalThemesApi { } /** - * Where the shell's update pipeline currently is. `available` only occurs - * when automatic downloads are disabled; with them enabled the shell moves - * straight to `downloading`. + * Where the shell's update pipeline currently is. `available` occurs when + * automatic downloads are disabled or the shell requires a manual installer; + * self-updating shells with automatic downloads enabled move to `downloading`. */ export type DesktopUpdateStatus = | 'idle' @@ -975,11 +975,9 @@ export interface DesktopUpdateState { /** Whole-number download progress (0-100) while `downloading`. */ percent?: number /** - * True when this shell cannot apply updates in place (a build without a - * Developer ID signature — local installs and pre-signing CI prereleases; - * Squirrel.Mac refuses to swap unsigned bundles). `available` is then the - * pipeline's terminal state and the advance action opens the download in - * the browser instead of downloading in the background. + * True when this shell cannot apply updates in place, such as an unsigned build + * or an app running outside /Applications. `available` is then the terminal state + * and the advance action opens the installer in the browser. */ manual?: boolean } @@ -988,11 +986,11 @@ export interface DesktopUpdateState { export interface SimDesktopUpdatesApi { getState(): Promise /** - * Advance the pipeline: checks for an update, or starts the download when - * one is already known to be available (auto-download off). + * Advances the pipeline: checks for an update, downloads an available + * self-update, or opens an available manual installer. */ check(): void - /** Quit and install a `ready` update. No-op in any other state. */ + /** Installs a ready update or opens the installer for an available manual update. */ install(): void /** Subscribe to pipeline state changes. Returns an unsubscribe function. */ onState(callback: (state: DesktopUpdateState) => void): () => void From 47cd071e337124d51cf8cad4feeb4750e610ae9e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 2 Sep 2026 01:20:17 -0700 Subject: [PATCH 2/4] fix(desktop): fall back from invalid releases --- apps/desktop/README.md | 2 +- apps/desktop/src/main/updater.test.ts | 31 +++++ apps/desktop/src/main/updater.ts | 9 +- .../api/desktop/update/download/route.test.ts | 52 ++++++-- .../app/api/desktop/update/download/route.ts | 72 ++++++----- .../update/latest-mac.yml/route.test.ts | 31 +++++ .../desktop/update/latest-mac.yml/route.ts | 120 +++++++----------- apps/sim/lib/desktop/update-feed.test.ts | 32 ++++- apps/sim/lib/desktop/update-feed.ts | 103 ++++++++++----- 9 files changed, 297 insertions(+), 155 deletions(-) diff --git a/apps/desktop/README.md b/apps/desktop/README.md index ecbbf33de59..bd3e9dffc5d 100644 --- a/apps/desktop/README.md +++ b/apps/desktop/README.md @@ -171,7 +171,7 @@ Raw local file bytes are never exposed through the preload bridge and cannot be ## Auto-update, channels, rollout, rollback -- `electron-updater` reads the deployment's `/api/desktop/update` feed; production resolves stable releases from `simstudioai/sim`, while dev/staging resolve prereleases from `simstudioai/sim-desktop-releases`. Artifact downloads go directly to GitHub and deltas use `.zip.blockmap`. Sim validates every candidate before starting its download. Install is prompt-based (Restart and update / Later; Later installs on quit) — never forced mid-session. +- `electron-updater` reads the deployment's `/api/desktop/update` feed; production resolves stable releases from `simstudioai/sim`, while dev/staging resolve prereleases from `simstudioai/sim-desktop-releases`. Artifact downloads go directly to GitHub and deltas use `.zip.blockmap`. Sim validates every candidate before starting its download. Developer ID builds installed under `/Applications` use a prompt (Restart and update / Later; Later installs on quit); other packaged builds offer a validated installer download — never forced mid-session. - Streams: production follows stable `X.Y.Z` releases, dev follows `-dev.N`, and staging follows `-staging.N`. The feed still recognizes legacy `-alpha.N`/`-beta.N` releases during migration. - Staged rollout: after publishing, edit `stagingPercentage: 10` into the release's `latest-mac.yml`, then raise as crash metrics stay clean. - Rollback: a pulled release must be superseded by a **higher** version — users on the broken build will not reinstall an equal one. (A blocked-versions kill-switch was removed as unwired dead code; reintroduce it in `updater.ts` if a remote config source ever exists to feed it.) diff --git a/apps/desktop/src/main/updater.test.ts b/apps/desktop/src/main/updater.test.ts index ee70f5b3ce6..7f49db4fb9a 100644 --- a/apps/desktop/src/main/updater.test.ts +++ b/apps/desktop/src/main/updater.test.ts @@ -353,6 +353,37 @@ describe('initUpdater state machine', () => { expect(autoUpdaterMock.quitAndInstall).toHaveBeenCalledTimes(1) }) + it('does not install when the updater fails during pre-install teardown', async () => { + let finishTeardown: (() => void) | undefined + const setRelaunchPending = vi.fn() + const { handle } = await createUpdater({ + beforeInstall: () => + new Promise((resolve) => { + finishTeardown = resolve + }), + setRelaunchPending, + }) + + handle.check() + await vi.advanceTimersByTimeAsync(0) + emit('update-available', { version: '2.0.0' }) + emit('update-downloaded', { version: '2.0.0' }) + vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ + response: 1, + checkboxChecked: false, + }) + handle.install() + await vi.advanceTimersByTimeAsync(0) + + emit('error', new Error('native staging failed')) + finishTeardown?.() + await vi.advanceTimersByTimeAsync(0) + + expect(handle.getState()).toEqual({ status: 'error', version: '2.0.0' }) + expect(setRelaunchPending).not.toHaveBeenCalledWith(true) + expect(autoUpdaterMock.quitAndInstall).not.toHaveBeenCalled() + }) + it('bypasses renderer unload guards only after teardown succeeds', async () => { const setRelaunchPending = vi.fn() vi.mocked(dialog.showMessageBox).mockResolvedValueOnce({ diff --git a/apps/desktop/src/main/updater.ts b/apps/desktop/src/main/updater.ts index 0de7d007f26..a185f6c1669 100644 --- a/apps/desktop/src/main/updater.ts +++ b/apps/desktop/src/main/updater.ts @@ -394,12 +394,17 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { let installInFlight = false let installConfirmationInFlight = false - const quitAndInstall = () => { + const quitAndInstall = (version: string | undefined) => { if (installInFlight) return installInFlight = true void Promise.resolve() .then(() => deps.beforeInstall?.()) .then(() => { + if (state.status !== 'ready' || state.version !== version) { + autoUpdater.autoInstallOnAppQuit = false + installInFlight = false + return + } deps.setRelaunchPending?.(true) autoUpdater.quitAndInstall() }) @@ -435,7 +440,7 @@ export function initUpdater(deps: UpdaterDeps): UpdaterHandle { void confirmation .then(({ response }) => { if (response === 1 && state.status === 'ready' && state.version === version) { - quitAndInstall() + quitAndInstall(version) } }) .catch((error) => { diff --git a/apps/sim/app/api/desktop/update/download/route.test.ts b/apps/sim/app/api/desktop/update/download/route.test.ts index 43cc05b7e5d..63670a75a81 100644 --- a/apps/sim/app/api/desktop/update/download/route.test.ts +++ b/apps/sim/app/api/desktop/update/download/route.test.ts @@ -36,6 +36,10 @@ function release(tag: string, repository: string) { } } +function manifest(version: string) { + return [`version: ${version}`, 'files:', ` - url: Sim-${version}-universal.zip`].join('\n') +} + async function getDownload(): Promise { return GET(new NextRequest('https://www.sim.ai/api/desktop/update/download'), undefined) } @@ -43,6 +47,21 @@ async function getDownload(): Promise { describe('desktop update download route', () => { const fetchMock = vi.fn() + function mockReleases(releases: ReturnType[]) { + fetchMock.mockImplementation(async (input: string | URL | Request) => { + const url = String(input) + if (url === STABLE_RELEASES_URL || url === PRERELEASE_RELEASES_URL) { + return Response.json(releases) + } + const candidate = releases.find((release) => + release.assets.some((asset) => asset.browser_download_url === url) + ) + return candidate + ? new Response(manifest(candidate.tag_name.replace(/^v/, ''))) + : new Response(null, { status: 404 }) + }) + } + beforeEach(() => { fetchMock.mockReset() vi.stubGlobal('fetch', fetchMock) @@ -54,13 +73,11 @@ describe('desktop update download route', () => { }) it('redirects to the newest stable installer', async () => { - fetchMock.mockResolvedValueOnce( - Response.json([ - release('v1.1.0', DESKTOP_STABLE_RELEASE_REPOSITORY), - release('v1.3.0', DESKTOP_STABLE_RELEASE_REPOSITORY), - release('v1.2.0', DESKTOP_STABLE_RELEASE_REPOSITORY), - ]) - ) + mockReleases([ + release('v1.1.0', DESKTOP_STABLE_RELEASE_REPOSITORY), + release('v1.3.0', DESKTOP_STABLE_RELEASE_REPOSITORY), + release('v1.2.0', DESKTOP_STABLE_RELEASE_REPOSITORY), + ]) const response = await getDownload() @@ -73,12 +90,10 @@ describe('desktop update download route', () => { it('serves its own deployment channel rather than the stable stream', async () => { setEnv({ APPCONFIG_ENVIRONMENT: 'dev' }) - fetchMock.mockResolvedValueOnce( - Response.json([ - release('v1.3.0-dev.4', DESKTOP_PRERELEASE_REPOSITORY), - release('v1.4.0-staging.1', DESKTOP_PRERELEASE_REPOSITORY), - ]) - ) + mockReleases([ + release('v1.3.0-dev.4', DESKTOP_PRERELEASE_REPOSITORY), + release('v1.4.0-staging.1', DESKTOP_PRERELEASE_REPOSITORY), + ]) const response = await getDownload() @@ -87,6 +102,17 @@ describe('desktop update download route', () => { expect(fetchMock).toHaveBeenCalledWith(PRERELEASE_RELEASES_URL, expect.any(Object)) }) + it('falls back when the newest release has no installer artifact', async () => { + const incomplete = release('v1.4.0', DESKTOP_STABLE_RELEASE_REPOSITORY) + incomplete.assets = incomplete.assets.filter((asset) => asset.name === MANIFEST_ASSET_NAME) + mockReleases([incomplete, release('v1.3.0', DESKTOP_STABLE_RELEASE_REPOSITORY)]) + + const response = await getDownload() + + expect(response.status).toBe(302) + expect(response.headers.get('location')).toContain('Sim-1.3.0-universal.dmg') + }) + it('reports no release when the channel has none', async () => { fetchMock.mockResolvedValueOnce( Response.json([release('v1.3.0-dev.4', DESKTOP_PRERELEASE_REPOSITORY)]) diff --git a/apps/sim/app/api/desktop/update/download/route.ts b/apps/sim/app/api/desktop/update/download/route.ts index 2f9eb070dc3..0d6499755af 100644 --- a/apps/sim/app/api/desktop/update/download/route.ts +++ b/apps/sim/app/api/desktop/update/download/route.ts @@ -9,7 +9,7 @@ import { releaseRepositoryForChannel, releasesApiUrl, resolveLatestRelease, - selectInstallerAsset, + resolveReleaseAssets, } from '@/lib/desktop/update-feed' const logger = createLogger('DesktopUpdateDownloadAPI') @@ -36,52 +36,64 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise { - try { - const response = await fetch(releasesApiUrl(releaseRepository, page), { - headers: { - accept: 'application/vnd.github+json', - ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), - }, - next: { revalidate: REVALIDATE_SECONDS }, - }) - if (!response.ok) { - logger.error('GitHub releases lookup failed', { - status: response.status, + const resolved = await resolveLatestRelease( + channel, + async (page) => { + try { + const response = await fetch(releasesApiUrl(releaseRepository, page), { + headers: { + accept: 'application/vnd.github+json', + ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), + }, + next: { revalidate: REVALIDATE_SECONDS }, + }) + if (!response.ok) { + logger.error('GitHub releases lookup failed', { + status: response.status, + page, + channel, + releaseRepository, + }) + return null + } + return (await response.json()) as DesktopReleaseCandidate[] + } catch (error) { + logger.error('GitHub releases response could not be read', { + message: getErrorMessage(error), page, channel, releaseRepository, }) return null } - return (await response.json()) as DesktopReleaseCandidate[] - } catch (error) { - logger.error('GitHub releases response could not be read', { - message: getErrorMessage(error), - page, - channel, - releaseRepository, - }) - return null + }, + async (release) => { + const assets = await resolveReleaseAssets(release, releaseRepository, (url) => + fetch(url, { + next: { revalidate: REVALIDATE_SECONDS }, + }) + ) + if (!assets) { + logger.warn('Skipping incomplete or invalid desktop release', { + tag: release.tag_name, + channel, + }) + } + return assets?.installer ?? null } - }) + ) if ('error' in resolved) { return NextResponse.json({ error: 'Release feed unavailable' }, { status: 502 }) } - const release = resolved.release - const asset = release ? selectInstallerAsset(release, releaseRepository) : null - if (!release || !asset) { - if (release) { - logger.error('Release has no installer artifact', { tag: release.tag_name, channel }) - } + if (!resolved.release) { return NextResponse.json( { error: `No desktop release for channel ${channel}` }, { status: 404 } ) } - return NextResponse.redirect(asset.browser_download_url, { + return NextResponse.redirect(resolved.value.browser_download_url, { status: 302, headers: { 'cache-control': `public, max-age=${REVALIDATE_SECONDS}` }, }) diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts index 5daf8b843b9..9267d6bc33f 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts @@ -18,6 +18,7 @@ const PRERELEASE_RELEASES_URL = releasesApiUrl(DESKTOP_PRERELEASE_REPOSITORY, 1) const FEED_STATUS_HEADER = 'x-sim-desktop-update-feed' function release(tag: string) { + const version = tag.replace(/^v/, '') return { tag_name: tag, draft: false, @@ -27,6 +28,14 @@ function release(tag: string) { name: MANIFEST_ASSET_NAME, browser_download_url: `https://downloads.example/${tag}/${MANIFEST_ASSET_NAME}`, }, + { + name: `Sim-${version}-universal.zip`, + browser_download_url: `https://downloads.example/${tag}/Sim-${version}-universal.zip`, + }, + { + name: `Sim-${version}-universal.dmg`, + browser_download_url: `https://downloads.example/${tag}/Sim-${version}-universal.dmg`, + }, ], } } @@ -226,6 +235,28 @@ describe('desktop update manifest route', () => { expect(fetchMock).toHaveBeenNthCalledWith(1, PRERELEASE_RELEASES_URL, expect.any(Object)) }) + it('falls back when the newest release has an invalid manifest', async () => { + setEnv({ APPCONFIG_ENVIRONMENT: 'dev' }) + fetchMock.mockImplementation(async (input: string | URL | Request) => { + const url = String(input) + if (url === PRERELEASE_RELEASES_URL) { + return Response.json([release('v1.2.0-dev.5'), release('v1.2.0-dev.4')]) + } + if (url === `https://downloads.example/v1.2.0-dev.5/${MANIFEST_ASSET_NAME}`) { + return new Response(manifest('1.2.0-staging.5')) + } + if (url === `https://downloads.example/v1.2.0-dev.4/${MANIFEST_ASSET_NAME}`) { + return new Response(manifest('1.2.0-dev.4')) + } + return new Response(null, { status: 404 }) + }) + + const response = await getFeed('www.dev.sim.ai') + + expect(response.status).toBe(200) + expect(await response.text()).toContain('version: 1.2.0-dev.4') + }) + it('rejects an oversized updater manifest', async () => { fetchMock .mockResolvedValueOnce(Response.json([release('v1.1.0')])) diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts index 8b4a15ffffa..dafe66d433f 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts @@ -2,17 +2,14 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { type NextRequest, NextResponse } from 'next/server' import { env } from '@/lib/core/config/env' -import { readResponseTextWithLimit } from '@/lib/core/utils/stream-limits' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' import { channelForDeploymentEnvironment, type DesktopReleaseCandidate, - MANIFEST_ASSET_NAME, - MAX_DESKTOP_UPDATE_MANIFEST_BYTES, releaseRepositoryForChannel, releasesApiUrl, resolveLatestRelease, - rewriteManifestUrls, + resolveReleaseAssets, } from '@/lib/desktop/update-feed' const logger = createLogger('DesktopUpdateFeedAPI') @@ -45,41 +42,61 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise { - try { - const response = await fetch(releasesApiUrl(releaseRepository, page), { - headers: { - accept: 'application/vnd.github+json', - ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), - }, - next: { revalidate: REVALIDATE_SECONDS }, - }) - if (!response.ok) { - logger.error('GitHub releases lookup failed', { - status: response.status, + const resolved = await resolveLatestRelease( + channel, + async (page) => { + try { + const response = await fetch(releasesApiUrl(releaseRepository, page), { + headers: { + accept: 'application/vnd.github+json', + ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), + }, + next: { revalidate: REVALIDATE_SECONDS }, + }) + if (!response.ok) { + logger.error('GitHub releases lookup failed', { + status: response.status, + page, + channel, + releaseRepository, + }) + return null + } + return (await response.json()) as DesktopReleaseCandidate[] + } catch (error) { + logger.error('GitHub releases response could not be read', { + message: getErrorMessage(error), page, channel, releaseRepository, }) return null } - return (await response.json()) as DesktopReleaseCandidate[] - } catch (error) { - logger.error('GitHub releases response could not be read', { - message: getErrorMessage(error), - page, - channel, - releaseRepository, - }) - return null + }, + async (release) => { + const assets = await resolveReleaseAssets(release, releaseRepository, (url) => + fetch(url, { + next: { revalidate: REVALIDATE_SECONDS }, + }) + ) + if (!assets) { + logger.warn('Skipping incomplete or invalid desktop release', { + tag: release.tag_name, + channel, + }) + } + return assets?.manifest ?? null } - }) + ) if ('error' in resolved) { return NextResponse.json({ error: 'Release feed unavailable' }, { status: 502 }) } const release = resolved.release if (!release) { + if (resolved.rejectedCandidates) { + return NextResponse.json({ error: 'Release manifest unavailable' }, { status: 502 }) + } return NextResponse.json( { error: `No desktop release for channel ${channel}` }, { @@ -89,56 +106,7 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise candidate.name === MANIFEST_ASSET_NAME) - if (!asset) { - logger.error('Release is missing its updater manifest', { - tag: release.tag_name, - channel, - }) - return NextResponse.json({ error: 'Release manifest unavailable' }, { status: 404 }) - } - - const manifestResponse = await fetch(asset.browser_download_url, { - next: { revalidate: REVALIDATE_SECONDS }, - }) - if (!manifestResponse.ok) { - logger.error('Updater manifest download failed', { - status: manifestResponse.status, - tag: release.tag_name, - }) - return NextResponse.json({ error: 'Release manifest unavailable' }, { status: 502 }) - } - let manifestSource: string - try { - manifestSource = await readResponseTextWithLimit(manifestResponse, { - maxBytes: MAX_DESKTOP_UPDATE_MANIFEST_BYTES, - label: 'Desktop update manifest', - }) - } catch (error) { - logger.error('Updater manifest could not be read safely', { - tag: release.tag_name, - message: getErrorMessage(error), - }) - return NextResponse.json({ error: 'Release manifest unavailable' }, { status: 502 }) - } - const manifestVersion = /^version:\s*(\S+)\s*$/m.exec(manifestSource)?.[1] - const releaseVersion = release.tag_name.replace(/^v/, '') - if (manifestVersion !== releaseVersion) { - logger.error('Updater manifest version does not match its release', { - tag: release.tag_name, - manifestVersion, - }) - return NextResponse.json({ error: 'Release manifest unavailable' }, { status: 502 }) - } - const manifest = rewriteManifestUrls(manifestSource, release.tag_name, releaseRepository) - if (!manifest) { - logger.error('Updater manifest referenced an unexpected artifact', { - tag: release.tag_name, - }) - return NextResponse.json({ error: 'Release manifest unavailable' }, { status: 502 }) - } - - return new NextResponse(manifest, { + return new NextResponse(resolved.value, { status: 200, headers: { 'content-type': 'text/yaml; charset=utf-8', diff --git a/apps/sim/lib/desktop/update-feed.test.ts b/apps/sim/lib/desktop/update-feed.test.ts index aba3385a9fe..4fee8fbfd4f 100644 --- a/apps/sim/lib/desktop/update-feed.test.ts +++ b/apps/sim/lib/desktop/update-feed.test.ts @@ -147,7 +147,12 @@ describe('rewriteManifestUrls', () => { 'sha512: abc', "releaseDate: '2026-07-23T00:00:00.000Z'", ].join('\n') - const rewritten = rewriteManifestUrls(manifest, 'v0.5.24', repository) + const rewritten = rewriteManifestUrls( + manifest, + 'v0.5.24', + repository, + new Set(['Sim-0.5.24-universal.zip']) + ) expect(rewritten).toContain( ` - url: https://github.com/${repository}/releases/download/v0.5.24/Sim-0.5.24-universal.zip` ) @@ -159,13 +164,34 @@ describe('rewriteManifestUrls', () => { it('canonicalizes an expected absolute asset URL', () => { const manifest = ' - url: https://cdn.example.com/Sim-0.5.24-universal.zip' - expect(rewriteManifestUrls(manifest, 'v0.5.24', DESKTOP_STABLE_RELEASE_REPOSITORY)).toBe( + expect( + rewriteManifestUrls( + manifest, + 'v0.5.24', + DESKTOP_STABLE_RELEASE_REPOSITORY, + new Set(['Sim-0.5.24-universal.zip']) + ) + ).toBe( ` - url: https://github.com/${DESKTOP_STABLE_RELEASE_REPOSITORY}/releases/download/v0.5.24/Sim-0.5.24-universal.zip` ) }) it('rejects unexpected manifest asset names', () => { const manifest = ' - url: https://cdn.example.com/unreviewed.zip' - expect(rewriteManifestUrls(manifest, 'v0.5.24', DESKTOP_STABLE_RELEASE_REPOSITORY)).toBeNull() + expect( + rewriteManifestUrls( + manifest, + 'v0.5.24', + DESKTOP_STABLE_RELEASE_REPOSITORY, + new Set(['Sim-0.5.24-universal.zip']) + ) + ).toBeNull() + }) + + it('rejects an expected artifact that is absent from the release', () => { + const manifest = ' - url: Sim-0.5.24-universal.zip' + expect( + rewriteManifestUrls(manifest, 'v0.5.24', DESKTOP_STABLE_RELEASE_REPOSITORY, new Set()) + ).toBeNull() }) }) diff --git a/apps/sim/lib/desktop/update-feed.ts b/apps/sim/lib/desktop/update-feed.ts index 49723afcad3..3806ec494ec 100644 --- a/apps/sim/lib/desktop/update-feed.ts +++ b/apps/sim/lib/desktop/update-feed.ts @@ -25,6 +25,8 @@ * Squirrel.Mac cannot apply (bundle-id mismatch) — each channel only ever * moves forward on its own artifacts. */ + +import { readResponseTextWithLimit } from '@/lib/core/utils/stream-limits' import { compareVersions } from '@/lib/desktop/min-version' export const DESKTOP_STABLE_RELEASE_REPOSITORY = 'simstudioai/sim' @@ -77,18 +79,16 @@ export interface DesktopReleaseCandidate { } /** - * Picks the newest release of the channel's own kind. Channels never see + * Lists releases of the channel's own kind, newest first. Channels never see * another channel's artifacts (see module docs). Releases without their * updater manifest asset are skipped — a release created before its build - * finished (or whose build failed) must not take the channel down. Returns - * null when nothing qualifies. + * finished (or whose build failed) must not take the channel down. */ -export function selectReleaseForChannel( +function releasesForChannel( releases: DesktopReleaseCandidate[], channel: DesktopUpdateChannel -): DesktopReleaseCandidate | null { - let best: DesktopReleaseCandidate | null = null - let bestVersion = '' +): DesktopReleaseCandidate[] { + const candidates: Array<{ release: DesktopReleaseCandidate; version: string }> = [] for (const release of releases) { if (release.draft) continue const version = release.tag_name.replace(/^v/, '') @@ -99,20 +99,19 @@ export function selectReleaseForChannel( if (!release.assets?.some((asset) => asset.name === MANIFEST_ASSET_NAME)) { continue } - if (best === null) { - const valid = compareVersions(version, '0.0.0') - if (valid === null) continue - best = release - bestVersion = version - continue - } - const comparison = compareVersions(version, bestVersion) - if (comparison !== null && comparison > 0) { - best = release - bestVersion = version - } + if (compareVersions(version, '0.0.0') === null) continue + candidates.push({ release, version }) } - return best + candidates.sort((left, right) => compareVersions(right.version, left.version) ?? 0) + return candidates.map(({ release }) => release) +} + +/** Picks the newest release that passes the channel and manifest-presence checks. */ +export function selectReleaseForChannel( + releases: DesktopReleaseCandidate[], + channel: DesktopUpdateChannel +): DesktopReleaseCandidate | null { + return releasesForChannel(releases, channel)[0] ?? null } /** @@ -125,7 +124,8 @@ export function selectReleaseForChannel( export function rewriteManifestUrls( manifest: string, tag: string, - repository: DesktopReleaseRepository + repository: DesktopReleaseRepository, + availableAssetNames: ReadonlySet ): string | null { const base = `https://github.com/${repository}/releases/download/${tag}/` const version = tag.replace(/^v/, '') @@ -140,7 +140,7 @@ export function rewriteManifestUrls( ? new URL(value).pathname : value const name = decodeURIComponent(pathname.split('/').at(-1) ?? '') - if (!expectedNames.has(name)) { + if (!expectedNames.has(name) || !availableAssetNames.has(name)) { valid = false return '' } @@ -169,6 +169,39 @@ export const DESKTOP_RELEASES_PAGE_SIZE = 100 */ export const MAX_DESKTOP_RELEASE_PAGES = 5 +export interface DesktopReleaseAssets { + manifest: string + installer: { name: string; browser_download_url: string } +} + +/** Reads and validates the complete artifact set required to offer a release. */ +export async function resolveReleaseAssets( + release: DesktopReleaseCandidate, + repository: DesktopReleaseRepository, + fetchManifest: (url: string) => Promise +): Promise { + const manifestAsset = release.assets?.find((asset) => asset.name === MANIFEST_ASSET_NAME) + const installer = selectInstallerAsset(release, repository) + if (!manifestAsset || !installer) return null + + try { + const response = await fetchManifest(manifestAsset.browser_download_url) + if (!response.ok) return null + const source = await readResponseTextWithLimit(response, { + maxBytes: MAX_DESKTOP_UPDATE_MANIFEST_BYTES, + label: 'Desktop update manifest', + }) + const version = release.tag_name.replace(/^v/, '') + if (/^version:\s*(\S+)\s*$/m.exec(source)?.[1] !== version) return null + + const availableAssetNames = new Set(release.assets?.map((asset) => asset.name)) + const manifest = rewriteManifestUrls(source, release.tag_name, repository, availableAssetNames) + return manifest ? { manifest, installer } : null + } catch { + return null + } +} + /** One page of the GitHub releases API, newest release first. */ export function releasesApiUrl(repository: DesktopReleaseRepository, page: number): string { return `https://api.github.com/repos/${repository}/releases?per_page=${DESKTOP_RELEASES_PAGE_SIZE}&page=${page}` @@ -183,22 +216,32 @@ export function releasesApiUrl(repository: DesktopReleaseRepository, page: numbe * (other tag families, other channels) cannot push a channel's newest build * out of the window and take the whole channel's updates down. * - * `fetchPage` returns null when the page could not be read; the resolver - * surfaces that as a failure rather than silently serving an older release. + * Every candidate is passed to `resolveCandidate`; a rejected candidate falls + * through to the next version. `fetchPage` returning null remains fatal because + * an unreadable page could hide a newer valid release. */ -export async function resolveLatestRelease( +export async function resolveLatestRelease( channel: DesktopUpdateChannel, - fetchPage: (page: number) => Promise -): Promise<{ release: DesktopReleaseCandidate | null } | { error: 'fetch-failed' }> { + fetchPage: (page: number) => Promise, + resolveCandidate: (release: DesktopReleaseCandidate) => T | null | Promise +): Promise< + | { release: DesktopReleaseCandidate; value: T } + | { release: null; rejectedCandidates: boolean } + | { error: 'fetch-failed' } +> { + let rejectedCandidates = false for (let page = 1; page <= MAX_DESKTOP_RELEASE_PAGES; page++) { const releases = await fetchPage(page) if (releases === null) return { error: 'fetch-failed' } - const release = selectReleaseForChannel(releases, channel) - if (release) return { release } + for (const release of releasesForChannel(releases, channel)) { + const value = await resolveCandidate(release) + if (value !== null) return { release, value } + rejectedCandidates = true + } // A short page is the end of the list; nothing older remains to walk. if (releases.length < DESKTOP_RELEASES_PAGE_SIZE) break } - return { release: null } + return { release: null, rejectedCandidates } } /** From eff2a95d6fd629a14eddeb0d52823d5a1ff7176c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 2 Sep 2026 01:32:22 -0700 Subject: [PATCH 3/4] fix(desktop): reject incomplete update feeds --- .../app/api/desktop/update/download/route.test.ts | 11 +++++++++++ apps/sim/app/api/desktop/update/download/route.ts | 3 +++ apps/sim/lib/desktop/update-feed.test.ts | 12 ++++++++++++ apps/sim/lib/desktop/update-feed.ts | 4 +++- 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/apps/sim/app/api/desktop/update/download/route.test.ts b/apps/sim/app/api/desktop/update/download/route.test.ts index 63670a75a81..9610e37e244 100644 --- a/apps/sim/app/api/desktop/update/download/route.test.ts +++ b/apps/sim/app/api/desktop/update/download/route.test.ts @@ -126,6 +126,17 @@ describe('desktop update download route', () => { }) }) + it('reports an invalid feed when every release candidate is rejected', async () => { + const incomplete = release('v1.4.0', DESKTOP_STABLE_RELEASE_REPOSITORY) + incomplete.assets = incomplete.assets.filter((asset) => asset.name === MANIFEST_ASSET_NAME) + mockReleases([incomplete]) + + const response = await getDownload() + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release installer unavailable' }) + }) + it('surfaces an unreadable release list instead of redirecting', async () => { fetchMock.mockResolvedValueOnce(new Response(null, { status: 500 })) diff --git a/apps/sim/app/api/desktop/update/download/route.ts b/apps/sim/app/api/desktop/update/download/route.ts index 0d6499755af..f48124a11cd 100644 --- a/apps/sim/app/api/desktop/update/download/route.ts +++ b/apps/sim/app/api/desktop/update/download/route.ts @@ -87,6 +87,9 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise { rewriteManifestUrls(manifest, 'v0.5.24', DESKTOP_STABLE_RELEASE_REPOSITORY, new Set()) ).toBeNull() }) + + it('rejects a manifest without an updater file entry', () => { + const manifest = ['version: 0.5.24', 'files: []', 'path: Sim-0.5.24-universal.zip'].join('\n') + expect( + rewriteManifestUrls( + manifest, + 'v0.5.24', + DESKTOP_STABLE_RELEASE_REPOSITORY, + new Set(['Sim-0.5.24-universal.zip']) + ) + ).toBeNull() + }) }) diff --git a/apps/sim/lib/desktop/update-feed.ts b/apps/sim/lib/desktop/update-feed.ts index 3806ec494ec..440de5cda29 100644 --- a/apps/sim/lib/desktop/update-feed.ts +++ b/apps/sim/lib/desktop/update-feed.ts @@ -131,6 +131,7 @@ export function rewriteManifestUrls( const version = tag.replace(/^v/, '') const expectedNames = new Set([`Sim-${version}-universal.dmg`, `Sim-${version}-universal.zip`]) let valid = true + let hasUpdaterFile = false const rewritten = manifest.replace( /^(\s*(?:-\s*)?(?:url|path):\s*)(\S+)\s*$/gm, (_line, prefix: string, value: string) => { @@ -144,6 +145,7 @@ export function rewriteManifestUrls( valid = false return '' } + if (/\burl:\s*$/.test(prefix)) hasUpdaterFile = true return `${prefix}${base}${encodeURIComponent(name)}` } catch { valid = false @@ -151,7 +153,7 @@ export function rewriteManifestUrls( } } ) - return valid ? rewritten : null + return valid && hasUpdaterFile ? rewritten : null } /** From 439dc742131aa98e6eccc0d05060fdd190f59c2e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 2 Sep 2026 01:43:44 -0700 Subject: [PATCH 4/4] fix(desktop): classify invalid release feeds --- .../sim/app/api/desktop/update/download/route.test.ts | 11 +++++++++++ .../api/desktop/update/latest-mac.yml/route.test.ts | 11 +++++++++++ apps/sim/lib/desktop/update-feed.test.ts | 10 ++++------ apps/sim/lib/desktop/update-feed.ts | 11 ++++------- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/apps/sim/app/api/desktop/update/download/route.test.ts b/apps/sim/app/api/desktop/update/download/route.test.ts index 9610e37e244..b3b0783c592 100644 --- a/apps/sim/app/api/desktop/update/download/route.test.ts +++ b/apps/sim/app/api/desktop/update/download/route.test.ts @@ -137,6 +137,17 @@ describe('desktop update download route', () => { expect(await response.json()).toMatchObject({ error: 'Release installer unavailable' }) }) + it('reports an invalid feed when the release has no updater manifest', async () => { + const incomplete = release('v1.4.0', DESKTOP_STABLE_RELEASE_REPOSITORY) + incomplete.assets = incomplete.assets.filter((asset) => asset.name !== MANIFEST_ASSET_NAME) + mockReleases([incomplete]) + + const response = await getDownload() + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release installer unavailable' }) + }) + it('surfaces an unreadable release list instead of redirecting', async () => { fetchMock.mockResolvedValueOnce(new Response(null, { status: 500 })) diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts index 9267d6bc33f..67eec2e2d2f 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts @@ -167,6 +167,17 @@ describe('desktop update manifest route', () => { expect(fetchMock).toHaveBeenCalledTimes(1) }) + it('reports an invalid feed when the release has no updater manifest', async () => { + const incomplete = release('v1.1.0') + incomplete.assets = incomplete.assets.filter((asset) => asset.name !== MANIFEST_ASSET_NAME) + fetchMock.mockResolvedValueOnce(Response.json([incomplete])) + + const response = await getFeed('www.sim.ai') + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release manifest unavailable' }) + }) + it('walks past a page of unrelated releases to reach the newest desktop build', async () => { const filler = Array.from({ length: DESKTOP_RELEASES_PAGE_SIZE }, (_, index) => ({ tag_name: `python-sdk-v0.${index}.0`, diff --git a/apps/sim/lib/desktop/update-feed.test.ts b/apps/sim/lib/desktop/update-feed.test.ts index 9cdd8919f7e..13e5d22f5af 100644 --- a/apps/sim/lib/desktop/update-feed.test.ts +++ b/apps/sim/lib/desktop/update-feed.test.ts @@ -111,19 +111,17 @@ describe('selectReleaseForChannel', () => { expect(selectReleaseForChannel(flagged, 'latest')?.tag_name).toBe('v0.5.24') }) - it('skips releases missing the updater manifest asset', () => { - // A release whose build failed (or is mid-upload) must not take the - // channel down; the previous good release keeps serving. + it('keeps releases missing the updater manifest eligible for candidate validation', () => { const withBrokenNewest = [ release('v0.5.25-dev.413', { assets: [{ name: 'Sim-0.5.25-dev.413-universal.dmg' }] }), release('v0.5.25-dev.412'), ] - expect(selectReleaseForChannel(withBrokenNewest, 'dev')?.tag_name).toBe('v0.5.25-dev.412') + expect(selectReleaseForChannel(withBrokenNewest, 'dev')?.tag_name).toBe('v0.5.25-dev.413') }) - it('skips release listings without asset data', () => { + it('keeps release listings without asset data eligible for candidate validation', () => { const bare = { tag_name: 'v0.5.25', draft: false, prerelease: false } - expect(selectReleaseForChannel([bare, release('v0.5.24')], 'latest')?.tag_name).toBe('v0.5.24') + expect(selectReleaseForChannel([bare, release('v0.5.24')], 'latest')?.tag_name).toBe('v0.5.25') }) it('skips drafts and unparseable tags', () => { diff --git a/apps/sim/lib/desktop/update-feed.ts b/apps/sim/lib/desktop/update-feed.ts index 440de5cda29..f6ba20a3788 100644 --- a/apps/sim/lib/desktop/update-feed.ts +++ b/apps/sim/lib/desktop/update-feed.ts @@ -80,9 +80,9 @@ export interface DesktopReleaseCandidate { /** * Lists releases of the channel's own kind, newest first. Channels never see - * another channel's artifacts (see module docs). Releases without their - * updater manifest asset are skipped — a release created before its build - * finished (or whose build failed) must not take the channel down. + * another channel's artifacts (see module docs). Artifact validation happens + * in the candidate resolver so invalid releases remain distinguishable from + * a channel with no releases. */ function releasesForChannel( releases: DesktopReleaseCandidate[], @@ -96,9 +96,6 @@ function releasesForChannel( // Defense in depth: a bare vX.Y.Z tag manually marked "pre-release" on // GitHub must not reach stable clients. if (channel === 'latest' && release.prerelease) continue - if (!release.assets?.some((asset) => asset.name === MANIFEST_ASSET_NAME)) { - continue - } if (compareVersions(version, '0.0.0') === null) continue candidates.push({ release, version }) } @@ -106,7 +103,7 @@ function releasesForChannel( return candidates.map(({ release }) => release) } -/** Picks the newest release that passes the channel and manifest-presence checks. */ +/** Picks the newest release that passes the channel and version checks. */ export function selectReleaseForChannel( releases: DesktopReleaseCandidate[], channel: DesktopUpdateChannel