fix(updater): pin stable feed before download (#1797)

Co-authored-by: Orca <help@stably.ai>

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')

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<ReleaseFeedTag[] | null> {
 
 /**
  * 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/<tag>`, 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/<tag>`, 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<string | null> {
-  return (await fetchNewerReleaseTags(currentVersion, 1))[0] ?? null
+type FetchNewerReleaseTagOptions = {
+  includePrerelease?: boolean
+}
+
+export async function fetchNewerReleaseTag(
+  currentVersion: string,
+  options: FetchNewerReleaseTagOptions = {}
+): Promise<string | null> {
+  return (await fetchNewerReleaseTags(currentVersion, 1, options))[0] ?? null
 }
 
 export async function fetchNewerReleaseTags(
   currentVersion: string,
-  maxTags: number
+  maxTags: number,
+  options: FetchNewerReleaseTagOptions = {}
 ): Promise<string[]> {
+  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)
 }

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'
     })
   })
 

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<void> {
-  // Why: for prerelease users we mine the atom feed ourselves and pin the
-  // generic feed at /releases/download/<tag>/ 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<void> {
+  // 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<void> {
       : 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<unknown> => 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<unknown> => 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/<tag>/ 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/<tag>/ 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'