From dfa0133f457024de9b6bfa9ab93f50a0ed052006 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 27 Jul 2026 16:23:37 -0700 Subject: [PATCH] fix(mobile): retry a failed Android release refresh in minutes, not hours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A cold refresh failure cached `null` for the full 6h success TTL, so an Orca that autostarts before Wi-Fi/VPN associates advertised no Android recommendation for the rest of the session. Mobile could not recover on its own: once the refresh settled, `recommendedMobileAppVersionsPending` was undefined, so it scheduled no follow-up read. The cache is per-main-process, so this recurred on every desktop relaunch. Track failure in the cache state and give failed refreshes a 10-minute TTL while successes keep 6h. The inFlight dedupe guard still collapses concurrent status.get calls, so the retry floor is ~6 calls/hour against GitHub's 60/hr unauthenticated budget. A failure still retains the last-known-good version; only a cold failure yields null. Also drop the dead `appVersion` line from getStatus(): status.get spreads getStatus() and then re-assigns appVersion from the remote-server-updater snapshot, so the spread always lost. That snapshot falls back to '0.0.0-dev' rather than undefined. Its duplicate RuntimeStatus declaration broke `tsc` (TS2300) — appVersion was already declared. --- .../mobile-android-release-feed.test.ts | 58 +++++++++++++++++++ .../runtime/mobile-android-release-feed.ts | 22 +++++-- src/main/runtime/orca-runtime.test.ts | 10 ---- src/main/runtime/orca-runtime.ts | 3 - src/shared/runtime-types.ts | 2 - 5 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/main/runtime/mobile-android-release-feed.test.ts b/src/main/runtime/mobile-android-release-feed.test.ts index 85e9e8684f2..135123efe50 100644 --- a/src/main/runtime/mobile-android-release-feed.test.ts +++ b/src/main/runtime/mobile-android-release-feed.test.ts @@ -103,6 +103,64 @@ describe('extractAndroidVersionCandidates', () => { expect(getRecommendedAndroidVersion(1_000)).toBeNull() expect(fetchMock).toHaveBeenCalledTimes(2) }) + + it('retries a cold failure well before the success TTL', async () => { + let networkUp = false + const fetchMock = vi.fn(async (url: string) => + url.includes('/git/matching-refs/') + ? networkUp + ? jsonResponse([{ ref: 'refs/tags/mobile-android-v0.0.50' }]) + : jsonResponse({ message: 'offline' }, 503) + : jsonResponse({ + tag_name: 'mobile-android-v0.0.50', + draft: false, + assets: [{ name: 'app-release.apk' }] + }) + ) + __setAndroidReleaseFeedFetcherForTests(fetchMock) + + // Cold failure: no prior known version, so the host advertises nothing. + expect(getRecommendedAndroidVersion(1_000)).toBeNull() + await vi.waitFor(() => expect(isAndroidReleaseFeedRefreshPending()).toBe(false)) + expect(fetchMock).toHaveBeenCalledOnce() + + // Still inside the short failure TTL — no retry yet. + expect(getRecommendedAndroidVersion(9 * 60_000)).toBeNull() + expect(fetchMock).toHaveBeenCalledOnce() + + // Past the failure TTL but far inside the 6h success TTL: retry anyway. + networkUp = true + expect(getRecommendedAndroidVersion(11 * 60_000)).toBeNull() + await vi.waitFor(() => expect(isAndroidReleaseFeedRefreshPending()).toBe(false)) + + expect(getRecommendedAndroidVersion(11 * 60_000)).toBe('0.0.50') + expect(fetchMock).toHaveBeenCalledTimes(3) + }) + + it('does not re-fetch a successful refresh before the 6h TTL', async () => { + const fetchMock = vi.fn(async (url: string) => + url.includes('/git/matching-refs/') + ? jsonResponse([{ ref: 'refs/tags/mobile-android-v0.0.51' }]) + : jsonResponse({ + tag_name: 'mobile-android-v0.0.51', + draft: false, + assets: [{ name: 'app-release.apk' }] + }) + ) + __setAndroidReleaseFeedFetcherForTests(fetchMock) + + expect(getRecommendedAndroidVersion(1_000)).toBeNull() + await vi.waitFor(() => expect(isAndroidReleaseFeedRefreshPending()).toBe(false)) + expect(fetchMock).toHaveBeenCalledTimes(2) + + // Well past the failure TTL, still inside the success TTL — no refresh. + expect(getRecommendedAndroidVersion(60 * 60_000)).toBe('0.0.51') + expect(fetchMock).toHaveBeenCalledTimes(2) + + // Past 6h: a success does eventually revalidate. + expect(getRecommendedAndroidVersion(7 * 60 * 60_000)).toBe('0.0.51') + expect(isAndroidReleaseFeedRefreshPending()).toBe(true) + }) }) function jsonResponse(body: unknown, status = 200): Response { diff --git a/src/main/runtime/mobile-android-release-feed.ts b/src/main/runtime/mobile-android-release-feed.ts index 4a52ac2a79f..a72ad8f79ae 100644 --- a/src/main/runtime/mobile-android-release-feed.ts +++ b/src/main/runtime/mobile-android-release-feed.ts @@ -12,6 +12,11 @@ const FETCH_TIMEOUT_MS = 5000 // Why: a mobile build ships ~monthly, so a long TTL keeps this to a few // unauthenticated GitHub calls per day per host while staying fresh enough. const CACHE_TTL_MS = 6 * 60 * 60 * 1000 +// Why: a cold failure (autostart before Wi-Fi/VPN associates) would otherwise black +// out the nudge for a full TTL, and mobile schedules no follow-up read once the +// refresh settles. Retry soon instead; a 10-minute floor is ~6 calls/hour against +// GitHub's 60/hr unauthenticated budget. +const FAILED_CACHE_TTL_MS = 10 * 60 * 1000 // Why: only exact Android release tags count, so suffixed RC builds never nudge stable users. const ANDROID_TAG_REF_RE = /^refs\/tags\/mobile-android-v(\d+\.\d+\.\d+)$/ // Why: malformed/orphaned tags must not turn one refresh into unbounded API fanout. @@ -30,6 +35,7 @@ type AndroidRelease = { type CacheState = { version: string | null fetchedAt: number + failed: boolean } type GitHubResponse = Pick @@ -132,12 +138,15 @@ function refreshInBackground(now: number): void { inFlight = fetchLatestAndroidVersion() .then((version) => { // Why: keep the last-known version on a failed refresh (null result) so a - // transient outage doesn't drop an already-correct nudge; only the - // timestamp advances, deferring the next retry by one TTL. - cache = { version: version ?? cache?.version ?? null, fetchedAt: now } + // transient outage doesn't drop an already-correct nudge. + cache = { + version: version ?? cache?.version ?? null, + fetchedAt: now, + failed: version === null + } }) .catch(() => { - cache = { version: cache?.version ?? null, fetchedAt: now } + cache = { version: cache?.version ?? null, fetchedAt: now, failed: true } }) .finally(() => { inFlight = null @@ -149,7 +158,8 @@ function refreshInBackground(now: number): void { // (stale-while-revalidate). First call returns null (no banner) until the first // fetch lands, which is the correct fail-open default. export function getRecommendedAndroidVersion(nowMs: number): string | null { - if (cache === null || nowMs - cache.fetchedAt > CACHE_TTL_MS) { + const ttl = cache?.failed === true ? FAILED_CACHE_TTL_MS : CACHE_TTL_MS + if (cache === null || nowMs - cache.fetchedAt > ttl) { refreshInBackground(nowMs) } return cache?.version ?? null @@ -169,7 +179,7 @@ export function __resetAndroidReleaseFeedCacheForTests(): void { // Test-only: seed the cache with a fresh value so status.get reads it without // hitting the network. export function __setAndroidReleaseFeedCacheForTests(version: string | null): void { - cache = { version, fetchedAt: Date.now() } + cache = { version, fetchedAt: Date.now(), failed: false } inFlight = null } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index bf2c5281d37..10c4109e77d 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1763,16 +1763,6 @@ describe('OrcaRuntimeService', () => { expect(status.minCompatibleMobileVersion).toBeGreaterThanOrEqual(0) }) - it('reports app version on status', () => { - vi.stubEnv('ORCA_APP_VERSION', '9.9.9-test') - try { - const status = createRuntime().getStatus() - expect(status.appVersion).toBe('9.9.9-test') - } finally { - vi.unstubAllEnvs() - } - }) - it('omits an unknown mobile recommendation, then reports cached Android', () => { __setAndroidReleaseFeedCacheForTests(null) try { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 67309ec610a..6e1c63935e3 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3687,9 +3687,6 @@ export class OrcaRuntimeService { hostPlatform: process.platform, terminalWindowsShell: this.store?.getSettings?.().terminalWindowsShell ?? null, floatingWorkspaceEnabled: this.store?.getSettings?.().floatingTerminalEnabled !== false, - // Why: the main process stamps its version into the env at startup; - // omit rather than fabricate when running outside that bootstrap (tests). - appVersion: process.env.ORCA_APP_VERSION, // Why: derived live from GitHub Releases (cached, fail-open) instead of a // hand-bumped constant, so releases need no recommendation bookkeeping; omitted // entirely when unknown. iOS defers to the App Store (no soft nudge). diff --git a/src/shared/runtime-types.ts b/src/shared/runtime-types.ts index edfb25cf92a..cb4334cc46a 100644 --- a/src/shared/runtime-types.ts +++ b/src/shared/runtime-types.ts @@ -90,8 +90,6 @@ export type RuntimeStatus = { // Why: mobile gates its Floating Workspace entry on this; absent on older // hosts, false when the user disabled the feature in desktop settings. floatingWorkspaceEnabled?: boolean - // Why: this host's own app version (semver); informational only, never gating. - appVersion?: string // Why: per-platform release trains can diverge; absent on older hosts and // mobile fails open (no update nudge). recommendedMobileAppVersions?: RecommendedMobileAppVersions