diff --git a/src/main/updater-prerelease-feed.test.ts b/src/main/updater-prerelease-feed.test.ts index d4209da6daf..f644b4e588b 100644 --- a/src/main/updater-prerelease-feed.test.ts +++ b/src/main/updater-prerelease-feed.test.ts @@ -43,6 +43,18 @@ describe('fetchNewerReleaseTag', () => { expect(await fetchNewerReleaseTag('1.3.19-rc.4')).toBe('v1.3.19-rc.6') }) + it('can exclude prerelease tags for stable-channel checks', async () => { + respondWithAtom(['v1.4.1-rc.0', 'v1.4.0', 'v1.3.52-rc.3', 'v1.3.51']) + const { fetchNewerReleaseTag } = await import('./updater-prerelease-feed') + expect(await fetchNewerReleaseTag('1.3.51', { includePrerelease: false })).toBe('v1.4.0') + }) + + it('returns null for stable-channel checks when only prereleases are newer', async () => { + respondWithAtom(['v1.4.1-rc.0', 'v1.3.52-rc.3', 'v1.3.51']) + const { fetchNewerReleaseTag } = await import('./updater-prerelease-feed') + expect(await fetchNewerReleaseTag('1.3.51', { includePrerelease: false })).toBe(null) + }) + it('returns null when nothing in the feed is newer than the current version', async () => { respondWithAtom(['v1.3.18', 'v1.3.17']) const { fetchNewerReleaseTag } = await import('./updater-prerelease-feed') diff --git a/src/main/updater-prerelease-feed.ts b/src/main/updater-prerelease-feed.ts index 39f3d5810bc..0e4feb70d2a 100644 --- a/src/main/updater-prerelease-feed.ts +++ b/src/main/updater-prerelease-feed.ts @@ -1,5 +1,5 @@ import { net } from 'electron' -import { compareVersions, isValidVersion } from './updater-fallback' +import { compareVersions, isPrereleaseVersion, isValidVersion } from './updater-fallback' const ATOM_FEED_URL = 'https://github.com/stablyai/orca/releases.atom' const RELEASES_DOWNLOAD_BASE = 'https://github.com/stablyai/orca/releases/download' @@ -54,37 +54,48 @@ async function fetchReleaseFeedTags(): Promise { /** * Walks the GitHub releases atom feed and returns the tag of the newest - * release strictly greater than `currentVersion`, regardless of channel. + * release strictly greater than `currentVersion`. * - * Why: electron-updater's GitHubProvider filters the feed by channel — when - * the running build is an RC, it only considers other RC/alpha/beta entries, - * so an RC user never gets offered the next *stable* release. By resolving - * the newest tag ourselves (any channel) and then pinning the generic - * provider at `/releases/download/`, we sidestep that channel filter - * entirely. Generic provider just reads the manifest at the URL we give it. + * Why: electron-updater's GitHubProvider filters the feed by channel, and + * GitHub's /latest/download redirect can move between check and download. + * By resolving the newest tag ourselves and pinning the generic provider at + * `/releases/download/`, the manifest and downloaded asset stay tied to + * the same release. * * Returns null if the fetch fails, the feed has no parseable tags, or * nothing in the feed is newer than `currentVersion`. */ -export async function fetchNewerReleaseTag(currentVersion: string): Promise { - return (await fetchNewerReleaseTags(currentVersion, 1))[0] ?? null +type FetchNewerReleaseTagOptions = { + includePrerelease?: boolean +} + +export async function fetchNewerReleaseTag( + currentVersion: string, + options: FetchNewerReleaseTagOptions = {} +): Promise { + return (await fetchNewerReleaseTags(currentVersion, 1, options))[0] ?? null } export async function fetchNewerReleaseTags( currentVersion: string, - maxTags: number + maxTags: number, + options: FetchNewerReleaseTagOptions = {} ): Promise { + const includePrerelease = options.includePrerelease ?? true const tags = await fetchReleaseFeedTags() if (!tags || maxTags <= 0) { return [] } - const newestNewerIndex = tags.findIndex( + const candidates = includePrerelease + ? tags + : tags.filter(({ version }) => !isPrereleaseVersion(version)) + const newestNewerIndex = candidates.findIndex( ({ version }) => compareVersions(version, currentVersion) > 0 ) if (newestNewerIndex === -1) { return [] } - return tags.slice(newestNewerIndex, newestNewerIndex + maxTags).map(({ tag }) => tag) + return candidates.slice(newestNewerIndex, newestNewerIndex + maxTags).map(({ tag }) => tag) } diff --git a/src/main/updater.test.ts b/src/main/updater.test.ts index bd262797cd4..98f7a0060c7 100644 --- a/src/main/updater.test.ts +++ b/src/main/updater.test.ts @@ -309,7 +309,9 @@ describe('updater', () => { setLastUpdateCheckAt }) - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) expect(setLastUpdateCheckAt).not.toHaveBeenCalled() }) @@ -349,11 +351,13 @@ describe('updater', () => { expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() - vi.advanceTimersByTime(59 * 60 * 1000) + await vi.advanceTimersByTimeAsync(59 * 60 * 1000) expect(autoUpdaterMock.checkForUpdates).not.toHaveBeenCalled() - vi.advanceTimersByTime(60 * 1000) - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(60 * 1000) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) expect(setLastUpdateCheckAt).not.toHaveBeenCalled() }) @@ -373,7 +377,9 @@ describe('updater', () => { appMock.emit('browser-window-focus') appMock.emit('browser-window-focus') - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) }) it('does not persist lastUpdateCheckAt when a focus-triggered check fails benignly', async () => { @@ -432,14 +438,17 @@ describe('updater', () => { setLastUpdateCheckAt: vi.fn() }) - await vi.runAllTicks() + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) + + await vi.advanceTimersByTimeAsync(59 * 60 * 1000) expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) - vi.advanceTimersByTime(59 * 60 * 1000) - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) - - vi.advanceTimersByTime(60 * 1000) - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(60 * 1000) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(2) + }) }) it('reschedules the next automatic check 24 hours after finding an available update', async () => { @@ -465,10 +474,11 @@ describe('updater', () => { setLastUpdateCheckAt }) - await vi.runAllTicks() + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) + }) await vi.advanceTimersByTimeAsync(0) - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) expect(setLastUpdateCheckAt).toHaveBeenCalledTimes(1) expect(sendMock).toHaveBeenCalledWith('updater:status', { state: 'available', @@ -476,11 +486,13 @@ describe('updater', () => { changelog: null }) - vi.advanceTimersByTime(23 * 60 * 60 * 1000 + 59 * 60 * 1000) + await vi.advanceTimersByTimeAsync(23 * 60 * 60 * 1000 + 59 * 60 * 1000) expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) - vi.advanceTimersByTime(60 * 1000) - expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(60 * 1000) + await vi.waitFor(() => { + expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(2) + }) }) it('does not leak a nudge marker into a later ordinary update cycle', async () => { @@ -516,11 +528,13 @@ describe('updater', () => { sendMock.mockClear() checkForUpdatesFromMenu() - const statusCalls = sendMock.mock.calls - .filter(([channel]) => channel === 'updater:status') - .map(([, status]) => status) + await vi.waitFor(() => { + const statusCalls = sendMock.mock.calls + .filter(([channel]) => channel === 'updater:status') + .map(([, status]) => status) - expect(statusCalls).toContainEqual({ state: 'checking', userInitiated: true }) + expect(statusCalls).toContainEqual({ state: 'checking', userInitiated: true }) + }) autoUpdaterMock.emit('update-available', { version: '1.0.62' }) await new Promise((resolve) => setTimeout(resolve, 0)) @@ -795,7 +809,9 @@ describe('updater', () => { checkForUpdatesFromMenu() await vi.waitFor(() => { - expect(fetchNewerReleaseTagsMock).toHaveBeenCalledWith('1.3.17-rc.1', 2) + expect(fetchNewerReleaseTagsMock).toHaveBeenCalledWith('1.3.17-rc.1', 2, { + includePrerelease: true + }) expect(autoUpdaterMock.setFeedURL).toHaveBeenLastCalledWith({ provider: 'generic', url: 'https://github.com/stablyai/orca/releases/download/v1.3.17-rc.2' @@ -1493,8 +1509,12 @@ describe('updater', () => { }) }) - it('does not invoke the atom-feed resolver for a stable user', async () => { + // Why: /releases/latest/download is a moving redirect. If a new stable + // release publishes between check and manual download, a relative ZIP URL + // from the old manifest can resolve against the new release and 404. + it('pins the generic feed to a concrete stable tag for a stable user', async () => { appMock.getVersion.mockReturnValue('1.3.17') + fetchNewerReleaseTagsMock.mockResolvedValue(['v1.3.18']) autoUpdaterMock.checkForUpdates.mockResolvedValue(undefined) const { setupAutoUpdater, checkForUpdatesFromMenu } = await import('./updater') @@ -1505,12 +1525,14 @@ describe('updater', () => { checkForUpdatesFromMenu() await vi.waitFor(() => { + expect(fetchNewerReleaseTagsMock).toHaveBeenCalledWith('1.3.17', 1, { + includePrerelease: false + }) expect(autoUpdaterMock.checkForUpdates).toHaveBeenCalledTimes(1) }) - expect(fetchNewerReleaseTagsMock).not.toHaveBeenCalled() - expect(autoUpdaterMock.setFeedURL).toHaveBeenCalledWith({ + expect(autoUpdaterMock.setFeedURL).toHaveBeenLastCalledWith({ provider: 'generic', - url: 'https://github.com/stablyai/orca/releases/latest/download' + url: 'https://github.com/stablyai/orca/releases/download/v1.3.18' }) }) diff --git a/src/main/updater.ts b/src/main/updater.ts index 22c68c9a15d..c885efe7ef1 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -422,33 +422,28 @@ function markMissingManifestPrereleaseFallbackPromiseHandled(message: string): v ) } -function shouldResolvePrereleaseFeed(): boolean { +function shouldPinDefaultReleaseFeed(): boolean { // Why: if the user Shift-clicked the menu to opt into RC this process, we've - // already switched to the native github provider — leave that alone. The - // atom-feed resolver only applies to users *running* a prerelease build on - // the default generic feed. - return !includePrereleaseActive && isPrereleaseVersion(app.getVersion()) + // already switched to the native github provider — leave that alone. + return !includePrereleaseActive } -async function pinPrereleaseFeed(): Promise { - // Why: for prerelease users we mine the atom feed ourselves and pin the - // generic feed at /releases/download// so the follow-up manifest fetch - // resolves against exactly that release. This handles BOTH RC→newer-RC and - // RC→stable, which is what a prerelease user wants. We avoid the native - // github provider because GitHubProvider.getLatestVersion() filters the feed - // by channel — when currentChannel is "rc", stable releases get skipped and - // the user never sees the GA (trapping them on the RC channel). +async function pinDefaultReleaseFeed(): Promise { + // Why: the /releases/latest/download/ redirect can move between the update + // check and the later manual download click. Pinning to the concrete tag + // keeps the manifest and ZIP asset on the same release. // - // If the resolver returns null (no newer release, or fetch failed), we fall - // back to the default /releases/latest/download/ URL. In the "no newer" - // case that feed will report the latest stable and compareVersions in the - // 'update-available' handler will correctly mark it as not-available. + // Prerelease users still need any-channel resolution so they can move to a + // newer RC or the next stable. Stable users should only resolve stable tags. const currentVersion = app.getVersion() - const releaseTags = await fetchNewerReleaseTags(currentVersion, 2) + const includePrerelease = isPrereleaseVersion(currentVersion) + const releaseTags = await fetchNewerReleaseTags(currentVersion, includePrerelease ? 2 : 1, { + includePrerelease + }) const newerTag = releaseTags[0] ?? null - const fallbackTag = releaseTags[1] ?? null + const fallbackTag = includePrerelease ? (releaseTags[1] ?? null) : null pendingPrereleaseFallback = - newerTag && fallbackTag + includePrerelease && newerTag && fallbackTag ? { primaryTag: newerTag, fallbackTag, @@ -464,16 +459,20 @@ async function pinPrereleaseFeed(): Promise { : null // Why: console.info goes to stdout and is captured by Console.app on macOS // and by --enable-logging elsewhere. This is the only window we have into - // the updater on a user's machine when something goes wrong (issue: RC user - // not offered newer stable). Cheap to keep, invaluable when triaging. + // the updater on a user's machine when something goes wrong. Cheap to keep, + // invaluable when triaging. if (newerTag) { const url = getReleaseDownloadUrl(newerTag) - console.info(`[updater] prerelease feed pinned: current=${currentVersion} → ${url}`) + console.info( + `[updater] release feed pinned: current=${currentVersion} includePrerelease=${includePrerelease} → ${url}` + ) autoUpdater.setFeedURL({ provider: 'generic', url }) } else { clearPrereleaseFallbackContext() const url = 'https://github.com/stablyai/orca/releases/latest/download' - console.info(`[updater] prerelease feed fallback: current=${currentVersion} → ${url}`) + console.info( + `[updater] release feed fallback: current=${currentVersion} includePrerelease=${includePrerelease} → ${url}` + ) autoUpdater.setFeedURL({ provider: 'generic', url }) } } @@ -554,8 +553,8 @@ function runBackgroundUpdateCheck( // Don't send 'checking' here — the 'checking-for-update' event handler does it, // and sending it from both places causes duplicate notifications (issue #35). const launch = (): Promise => autoUpdater.checkForUpdates() - const run = shouldResolvePrereleaseFeed() - ? pinPrereleaseFeed().then(launch) + const run = shouldPinDefaultReleaseFeed() + ? pinDefaultReleaseFeed().then(launch) : launchWithoutPrereleaseFallback(launch) void Promise.resolve(run).catch((err) => { backgroundCheckLaunchPending = false @@ -607,8 +606,8 @@ export function checkForUpdatesFromMenu(options?: { includePrerelease?: boolean // and sending it from both places causes duplicate notifications (issue #35). const launch = (): Promise => autoUpdater.checkForUpdates() - const run = shouldResolvePrereleaseFeed() - ? pinPrereleaseFeed().then(launch) + const run = shouldPinDefaultReleaseFeed() + ? pinDefaultReleaseFeed().then(launch) : launchWithoutPrereleaseFallback(launch) void Promise.resolve(run).catch((err) => { userInitiatedCheck = false @@ -776,19 +775,15 @@ export function setupAutoUpdater( ;(autoUpdater as NsisUpdater).verifyUpdateCodeSignature = () => Promise.resolve(null) } - // Use the generic provider with GitHub's /releases/latest/download/ URL so - // electron-updater always fetches the manifest (latest-mac.yml, latest.yml, - // latest-linux.yml) from the latest non-prerelease release. This sidesteps - // the broken /releases/latest API endpoint (returns 406) and automatically - // excludes RC/prerelease versions without client-side filtering. + // Use the generic provider with GitHub's /releases/latest/download/ URL as + // the startup fallback so electron-updater can fetch the manifest + // (latest-mac.yml, latest.yml, latest-linux.yml) from the latest + // non-prerelease release. // - // Why: for users already running a prerelease (e.g. 1.3.19-rc.6) we repin - // this URL to a specific /releases/download// before each check — see - // ensurePrereleaseFeedReady. That handles both RC→newer-RC AND RC→stable. - // We keep the generic provider (rather than switching to electron-updater's - // native github provider + allowPrerelease) because GitHubProvider filters - // the atom feed by channel and would silently skip stable releases when the - // running build is an RC — trapping the user on the RC channel. + // Why: before each default-channel check we repin this URL to a concrete + // /releases/download// URL. Keeping the generic provider avoids the + // native GitHub provider's RC channel filtering, and pinning avoids the + // moving /latest redirect changing between check and download. autoUpdater.setFeedURL({ provider: 'generic', url: 'https://github.com/stablyai/orca/releases/latest/download'