From 625377071f741b69cfded7f65986bc7e5589944c Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Mon, 14 Sep 2026 14:17:47 +0200 Subject: [PATCH 1/9] fix: stop a stalled request from becoming a permanent spinner Found while testing the field workflow on a physical iPhone: tapping Download on the embedding pack showed a loading wheel that never stopped, with no error and no way to retry. Two independent defects combine to produce that. ganeshaApiClient called fetch with no deadline. A connection that is accepted but never answered -- a captive portal, a dropped cellular handover, a backend that stalls mid-response -- leaves the promise pending forever. Every screen that awaits it spins indefinitely. Requests now abort after 30s (generous, since these can be the first call after an Azure Function cold start) and report a distinct `timeout` code, kept separate from `network-error`: one means "no network", the other means "the server took the call and went quiet", and those want different responses from someone standing in a reserve. SignInScreen.handleSignIn cleared its spinner on each exit path rather than in a finally, and the path after getUserProfile() had no protection at all. Combined with the missing timeout, a stalled profile lookup left the button spinning with the session already stored -- so there was no way to tell whether sign-in had succeeded. The spinner now clears in a finally, and a profile failure says the session was saved rather than "Sign-in failed", which was inviting a pointless second sign-in. handleDownloadPack already had a correct try/finally; it was spinning because the awaited API call never settled, not because it leaked state. Not fixed here: RNFS.downloadFile sets no connection/read timeout either. It is a different failure mode -- it reports progress and honours an abort signal -- and on the reported symptom nothing had reached disk, no staging file, so the stall was upstream of the transfer. Co-Authored-By: Claude Opus 5 --- __tests__/rntl/screens/SignInScreen.test.tsx | 42 ++++++++++++ .../unit/services/ganeshaApiClient.test.ts | 50 +++++++++++++- src/screens/SignInScreen.tsx | 68 +++++++++++-------- src/services/ganeshaApiClient/index.ts | 26 +++++++ src/services/ganeshaApiClient/types.ts | 1 + 5 files changed, 157 insertions(+), 30 deletions(-) diff --git a/__tests__/rntl/screens/SignInScreen.test.tsx b/__tests__/rntl/screens/SignInScreen.test.tsx index f05821ee3..28322ce19 100644 --- a/__tests__/rntl/screens/SignInScreen.test.tsx +++ b/__tests__/rntl/screens/SignInScreen.test.tsx @@ -133,6 +133,48 @@ describe('SignInScreen', () => { expect(mockGoBack).not.toHaveBeenCalled(); }); + // The spinner used to be cleared on each exit path rather than in a + // `finally`. A profile call that never settled -- ganeshaApiClient had no + // request timeout -- left the button spinning forever, with no error and no + // way to retry. In the field that looked like the app had simply frozen. + + it('clears the spinner after a failed profile lookup so sign-in can be retried', async () => { + jest.spyOn(Alert, 'alert').mockImplementation(() => {}); + mockSignIn.mockResolvedValue({ accessToken: 'a', refreshToken: 'r', idToken: 'i', accessTokenExpirationDate: '' }); + mockGetUserProfile.mockResolvedValue({ ok: false, code: 'timeout', message: 'Request timed out after 30s' }); + + const { getByTestId, queryByText } = render(); + fireEvent.press(getByTestId('sign-in-button')); + + await waitFor(() => expect(Alert.alert).toHaveBeenCalled()); + // The mocked Button renders " (loading)" while its spinner shows. + expect(queryByText(/\(loading\)/)).toBeNull(); + }); + + it('clears the spinner after a cancelled sign-in', async () => { + mockSignIn.mockRejectedValue(new Error('User cancelled flow')); + + const { getByTestId, queryByText } = render(<SignInScreen />); + fireEvent.press(getByTestId('sign-in-button')); + + await waitFor(() => expect(mockSignIn).toHaveBeenCalled()); + await waitFor(() => expect(queryByText(/\(loading\)/)).toBeNull()); + }); + + it('says the session was saved when only the profile lookup failed', async () => { + const alertSpy = jest.spyOn(Alert, 'alert').mockImplementation(() => {}); + mockSignIn.mockResolvedValue({ accessToken: 'a', refreshToken: 'r', idToken: 'i', accessTokenExpirationDate: '' }); + mockGetUserProfile.mockResolvedValue({ ok: false, code: 'timeout', message: 'Request timed out after 30s' }); + + const { getByTestId } = render(<SignInScreen />); + fireEvent.press(getByTestId('sign-in-button')); + + await waitFor(() => expect(alertSpy).toHaveBeenCalled()); + const [title, body] = alertSpy.mock.calls[0]; + expect(title).not.toMatch(/sign-in failed/i); + expect(body).toMatch(/session is saved/i); + }); + it('alerts on a real sign-in failure but not on a cancelled sign-in', async () => { const alertSpy = jest.spyOn(Alert, 'alert').mockImplementation(() => {}); mockSignIn.mockRejectedValue(new Error('User cancelled flow')); diff --git a/__tests__/unit/services/ganeshaApiClient.test.ts b/__tests__/unit/services/ganeshaApiClient.test.ts index 7e10314cb..babbbddb4 100644 --- a/__tests__/unit/services/ganeshaApiClient.test.ts +++ b/__tests__/unit/services/ganeshaApiClient.test.ts @@ -4,7 +4,7 @@ jest.mock('../../../src/services/entraAuthService', () => ({ entraAuthService: { getValidAccessToken: jest.fn() }, })); -import { ganeshaApiClient } from '../../../src/services/ganeshaApiClient'; +import { ganeshaApiClient, GANESHA_REQUEST_TIMEOUT_MS } from '../../../src/services/ganeshaApiClient'; import { entraAuthService } from '../../../src/services/entraAuthService'; const mockGetValidAccessToken = entraAuthService.getValidAccessToken as jest.Mock; @@ -310,3 +310,51 @@ describe('ganeshaApiClient.createUserProfile', () => { ); }); }); + +describe('ganeshaApiClient request deadline', () => { + afterEach(() => { + jest.useRealTimers(); + }); + + // Without a deadline a connection that is accepted but never answered leaves + // the promise pending forever, which the screens render as a permanent + // spinner with no error -- the iOS field symptom this guards against. + it('aborts and reports a timeout when the server never answers', async () => { + jest.useFakeTimers(); + mockFetch.mockImplementation( + (_url: string, init: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + init.signal.addEventListener('abort', () => reject(new Error('Aborted')), { once: true }); + }), + ); + + const pending = ganeshaApiClient.getUserProfile(); + // Async advance: the request awaits getValidAccessToken() before it ever + // arms the deadline, so a synchronous advance would fire against a timer + // that does not exist yet and the promise would hang. + await jest.advanceTimersByTimeAsync(GANESHA_REQUEST_TIMEOUT_MS); + const result = await pending; + + expect(result.ok).toBe(false); + expect(result).toMatchObject({ code: 'timeout' }); + }); + + it('passes an abort signal on every request', async () => { + mockFetch.mockResolvedValueOnce(jsonResponse(200, {})); + + await ganeshaApiClient.getUserProfile(); + + expect(mockFetch).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ signal: expect.anything() }), + ); + }); + + it('still reports a plain network error as network-error, not a timeout', async () => { + mockFetch.mockRejectedValueOnce(new Error('Network request failed')); + + const result = await ganeshaApiClient.getUserProfile(); + + expect(result).toMatchObject({ code: 'network-error', message: 'Network request failed' }); + }); +}); diff --git a/src/screens/SignInScreen.tsx b/src/screens/SignInScreen.tsx index 9ab24758e..1c579f978 100644 --- a/src/screens/SignInScreen.tsx +++ b/src/screens/SignInScreen.tsx @@ -29,42 +29,52 @@ export const SignInScreen: React.FC = () => { const handleSignIn = useCallback(async () => { setIsSigningIn(true); + // The spinner is cleared in `finally`, not on each exit path. Clearing it + // per-branch meant any early return added later -- or a profile call that + // never settled -- left it spinning with no error and no way to retry. try { - await entraAuthService.signIn(); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - logger.warn('[SignInScreen] Sign-in failed or was cancelled:', message); - // A cancelled sign-in (user backed out of the browser) is not an - // error worth alerting about -- only surface a message for real - // failures the person might be able to act on. - if (!/cancel/i.test(message)) { - Alert.alert('Sign-in failed', message); + try { + await entraAuthService.signIn(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.warn('[SignInScreen] Sign-in failed or was cancelled:', message); + // A cancelled sign-in (user backed out of the browser) is not an + // error worth alerting about -- only surface a message for real + // failures the person might be able to act on. + if (!/cancel/i.test(message)) { + Alert.alert('Sign-in failed', message); + } + return; } - setIsSigningIn(false); - return; - } - const profileResult = await ganeshaApiClient.getUserProfile(); - setIsSigningIn(false); + const profileResult = await ganeshaApiClient.getUserProfile(); - if (profileResult.ok) { - if (navigation.canGoBack()) { - navigation.goBack(); - } else { - navigation.replace('Main'); + if (profileResult.ok) { + if (navigation.canGoBack()) { + navigation.goBack(); + } else { + navigation.replace('Main'); + } + return; } - return; - } - if (profileResult.code === 'not-found') { - // First sign-in for this identity -- mirrors the web app's - // select-role step. Replace, not push, so a later "back" from - // SelectRole doesn't return to a completed SignIn screen. - navigation.replace('SelectRole'); - return; - } + if (profileResult.code === 'not-found') { + // First sign-in for this identity -- mirrors the web app's + // select-role step. Replace, not push, so a later "back" from + // SelectRole doesn't return to a completed SignIn screen. + navigation.replace('SelectRole'); + return; + } - Alert.alert('Sign-in failed', `Could not load your profile: ${profileResult.message}`); + // The session itself is already stored at this point, so say so -- + // otherwise "Sign-in failed" invites a pointless second sign-in. + Alert.alert( + 'Signed in, but your profile could not be loaded', + `${profileResult.message}. Your session is saved -- try again from Settings.`, + ); + } finally { + setIsSigningIn(false); + } }, [navigation]); return ( diff --git a/src/services/ganeshaApiClient/index.ts b/src/services/ganeshaApiClient/index.ts index fd4dfa94c..e595c33c0 100644 --- a/src/services/ganeshaApiClient/index.ts +++ b/src/services/ganeshaApiClient/index.ts @@ -32,6 +32,18 @@ import logger from '../../utils/logger'; * needs different headers (`x-ms-blob-type`) than any call here. See * `services/syncEngine` for that step. */ +/** + * Every request gets a deadline. Without one, a connection that is accepted + * but never answered -- a captive portal, a dropped cellular handover, a + * backend that stalls mid-response -- leaves the promise pending forever. + * Callers render that as a spinner with no way out and no error to report, + * which is exactly what it looked like in the field on iOS. + * + * 30s is deliberately generous: these calls are small JSON reads, but they + * can be the first request after an Azure Function cold start. + */ +export const GANESHA_REQUEST_TIMEOUT_MS = 30_000; + class GaneshaApiClient { async getLatestModel(modelName: string): Promise<GaneshaApiResult<LatestModelInfo>> { return this.request<LatestModelInfo>(`/models/${encodeURIComponent(modelName)}/latest`); @@ -81,6 +93,8 @@ class GaneshaApiClient { } let response: Response; + const controller = new AbortController(); + const deadline = setTimeout(() => controller.abort(), GANESHA_REQUEST_TIMEOUT_MS); try { response = await fetch(`${GANESHA_API_BASE_URL}${path}`, { method: init.method ?? 'GET', @@ -89,11 +103,23 @@ class GaneshaApiClient { ...(init.body !== undefined ? { 'Content-Type': 'application/json' } : {}), }, ...(init.body !== undefined ? { body: JSON.stringify(init.body) } : {}), + signal: controller.signal, }); } catch (error) { + // A timeout surfaces as an AbortError from the signal above. Report it + // separately from a refused connection: one means "no network", the + // other means "the server took the call and went quiet", and the person + // in the field can act on that difference. + if (controller.signal.aborted) { + const message = `Request timed out after ${GANESHA_REQUEST_TIMEOUT_MS / 1000}s`; + logger.warn(`[GaneshaApiClient] ${path} timed out`); + return { ok: false, code: 'timeout', message }; + } const message = error instanceof Error ? error.message : String(error); logger.warn(`[GaneshaApiClient] Network error for ${path}: ${message}`); return { ok: false, code: 'network-error', message }; + } finally { + clearTimeout(deadline); } const errorCode = this.errorCodeFor(response.status); diff --git a/src/services/ganeshaApiClient/types.ts b/src/services/ganeshaApiClient/types.ts index 3d7daa1f1..bf7cc7ee8 100644 --- a/src/services/ganeshaApiClient/types.ts +++ b/src/services/ganeshaApiClient/types.ts @@ -93,6 +93,7 @@ export interface CreateUserProfilePayload { */ export type GaneshaApiErrorCode = | 'network-error' + | 'timeout' | 'unauthenticated' | 'unauthorized' | 'not-found' From ef67398bfd6dad4294a6501d0e84f5d40aed686f Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:45:00 +0200 Subject: [PATCH 2/9] fix: bound the Entra sign-in and refresh calls Follows the same defect as the request timeout in this branch, on the path that was actually hanging in the field. react-native-app-auth bounds neither authorize() nor refresh(). On a marginal link the interactive flow gets all the way through -- the browser opens, the person authenticates, Entra redirects, ASWebAuthenticationSession intercepts the callback and the sheet closes -- and then the POST that trades the authorization code for tokens stalls. authorize() never settles. The sign-in screen spins forever with no error, and because OIDExternalUserAgentIOS discards the BOOL from resumeExternalUserAgentFlowWithURL: and reports every failure as "user cancelled", nothing upstream can tell the difference. Reproduced on a device syslog showing the link at -74dBm with 40% beacon loss. That is an ordinary reserve connection, not an edge case, and it is the condition this app is built for. The interactive budget is 180s: it has to cover reading a password manager and approving an MFA push on a second device, so a short deadline would fail people who were otherwise succeeding. It is there to bound the hang, not to be reached. Refresh is non-interactive and gets 30s, and a refresh that cannot complete reports "no valid session" rather than throwing, matching how an expired refresh token is already handled. Still unbounded: RNFS.downloadFile in fileDownloadService. It reports progress and honours an abort signal, so it is visible rather than silent, but it wants the same treatment before this ships to a reserve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .../unit/services/entraAuthService.test.ts | 57 ++++++++++++++++++- src/services/entraAuthService.ts | 43 +++++++++++++- 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/__tests__/unit/services/entraAuthService.test.ts b/__tests__/unit/services/entraAuthService.test.ts index 0ae930a59..6823f1e63 100644 --- a/__tests__/unit/services/entraAuthService.test.ts +++ b/__tests__/unit/services/entraAuthService.test.ts @@ -13,7 +13,11 @@ jest.mock('react-native-keychain', () => ({ import { authorize, refresh, revoke } from 'react-native-app-auth'; import * as Keychain from 'react-native-keychain'; -import { entraAuthService } from '../../../src/services/entraAuthService'; +import { + entraAuthService, + ENTRA_INTERACTIVE_TIMEOUT_MS, + ENTRA_REFRESH_TIMEOUT_MS, +} from '../../../src/services/entraAuthService'; import { ENTRA_ISSUER, ENTRA_MOBILE_CLIENT_ID, ENTRA_REDIRECT_URL, ENTRA_SCOPES } from '../../../src/config/entraAuth'; import { deploymentConfig, getTokenStorageService } from '../../../src/config/deployment'; @@ -234,3 +238,54 @@ describe('entraAuthService.getValidAccessToken', () => { expect(mockResetGenericPassword).toHaveBeenCalledWith({ service: TOKEN_SERVICE }); }); }); + +describe('entraAuthService deadlines', () => { + afterEach(() => { + jest.useRealTimers(); + }); + + // AppAuth bounds neither leg. On a marginal link the token exchange stalls + // after the browser has already closed, so authorize() never settles and the + // sign-in screen spins forever with nothing to report. + it('gives up on an interactive sign-in that never settles', async () => { + jest.useFakeTimers(); + mockAuthorize.mockImplementation(() => new Promise(() => {})); + + const pending = entraAuthService.signIn(); + const assertion = expect(pending).rejects.toThrow(/timed out after 180s/); + await jest.advanceTimersByTimeAsync(ENTRA_INTERACTIVE_TIMEOUT_MS); + await assertion; + }); + + it('gives up on a token refresh that never settles', async () => { + jest.useFakeTimers(); + mockGetGenericPassword.mockResolvedValue({ + username: 'entra-tokens', + password: JSON.stringify({ + accessToken: 'stale', + refreshToken: 'refresh-me', + idToken: 'id', + accessTokenExpirationDate: new Date(Date.now() - 1000).toISOString(), + }), + }); + mockRefresh.mockImplementation(() => new Promise(() => {})); + + const pending = entraAuthService.getValidAccessToken(); + await jest.advanceTimersByTimeAsync(ENTRA_REFRESH_TIMEOUT_MS); + + // A refresh that cannot complete is reported as "no valid session" rather + // than thrown, matching how an expired refresh token is already handled. + await expect(pending).resolves.toBeNull(); + }); + + it('does not interfere with a sign-in that completes normally', async () => { + mockAuthorize.mockResolvedValue({ + accessToken: 'a', + refreshToken: 'r', + idToken: 'i', + accessTokenExpirationDate: new Date(Date.now() + 3_600_000).toISOString(), + }); + + await expect(entraAuthService.signIn()).resolves.toMatchObject({ accessToken: 'a' }); + }); +}); diff --git a/src/services/entraAuthService.ts b/src/services/entraAuthService.ts index d4d0ec3fe..fabe0a39d 100644 --- a/src/services/entraAuthService.ts +++ b/src/services/entraAuthService.ts @@ -22,6 +22,37 @@ export interface StoredEntraTokens { accessTokenExpirationDate: string; } +/** + * AppAuth has no deadline of its own on either leg of the flow, and on a + * marginal link the token exchange is where it stalls: the browser closes, the + * authorization code comes back, and the POST that trades it for tokens never + * completes. `authorize()` then never settles, so the screen spins forever with + * nothing to report. Measured in the field on a link at -74dBm with 40% beacon + * loss, which is an ordinary reserve connection, not an edge case. + * + * The interactive budget is deliberately long: it has to cover reading a + * password manager and approving an MFA push on another device, so cutting it + * short would fail people who were succeeding. It exists to bound the hang, not + * to be hit in normal use. Refresh is non-interactive and gets far less. + */ +export const ENTRA_INTERACTIVE_TIMEOUT_MS = 180_000; +export const ENTRA_REFRESH_TIMEOUT_MS = 30_000; + +async function withDeadline<T>(operation: Promise<T>, timeoutMs: number, label: string): Promise<T> { + let timer: ReturnType<typeof setTimeout> | undefined; + const deadline = new Promise<never>((_resolve, reject) => { + timer = setTimeout( + () => reject(new Error(`${label} timed out after ${timeoutMs / 1000}s`)), + timeoutMs, + ); + }); + try { + return await Promise.race([operation, deadline]); + } finally { + clearTimeout(timer); + } +} + const authConfig: AuthConfiguration = { issuer: ENTRA_ISSUER, clientId: ENTRA_MOBILE_CLIENT_ID, @@ -43,7 +74,11 @@ const authConfig: AuthConfiguration = { class EntraAuthService { /** Runs the interactive sign-in flow (opens the system browser) and stores the resulting tokens. */ async signIn(): Promise<StoredEntraTokens> { - const result = await authorize(authConfig); + const result = await withDeadline( + authorize(authConfig), + ENTRA_INTERACTIVE_TIMEOUT_MS, + 'Sign-in', + ); const tokens: StoredEntraTokens = { accessToken: result.accessToken, refreshToken: result.refreshToken || null, @@ -104,7 +139,11 @@ class EntraAuthService { } try { - const refreshed = await refresh(authConfig, { refreshToken: tokens.refreshToken }); + const refreshed = await withDeadline( + refresh(authConfig, { refreshToken: tokens.refreshToken }), + ENTRA_REFRESH_TIMEOUT_MS, + 'Token refresh', + ); const newTokens: StoredEntraTokens = { accessToken: refreshed.accessToken, // Entra does not always return a new refresh token on a refresh From e997026389f1c4af2385b215a553eb51a15efe36 Mon Sep 17 00:00:00 2001 From: Carolina Lopez <blclo@github.com> Date: Tue, 22 Sep 2026 09:28:36 +0200 Subject: [PATCH 3/9] test: await auth timeout assertion Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- __tests__/unit/services/entraAuthService.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/__tests__/unit/services/entraAuthService.test.ts b/__tests__/unit/services/entraAuthService.test.ts index 6823f1e63..5782b7191 100644 --- a/__tests__/unit/services/entraAuthService.test.ts +++ b/__tests__/unit/services/entraAuthService.test.ts @@ -252,7 +252,9 @@ describe('entraAuthService deadlines', () => { mockAuthorize.mockImplementation(() => new Promise(() => {})); const pending = entraAuthService.signIn(); - const assertion = expect(pending).rejects.toThrow(/timed out after 180s/); + const assertion = (async () => { + await expect(pending).rejects.toThrow(/timed out after 180s/); + })(); await jest.advanceTimersByTimeAsync(ENTRA_INTERACTIVE_TIMEOUT_MS); await assertion; }); From f62853b426a515b00f63e64c503c65bc9db614cc Mon Sep 17 00:00:00 2001 From: Carolina Lopez <blclo@github.com> Date: Tue, 22 Sep 2026 09:35:33 +0200 Subject: [PATCH 4/9] fix: stop stalled native downloads Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../unit/services/fileDownloadService.test.ts | 98 +++++++++++++++++++ src/services/fileDownloadService/index.ts | 84 +++++++++++++--- src/services/fileDownloadService/types.ts | 3 + 3 files changed, 174 insertions(+), 11 deletions(-) create mode 100644 __tests__/unit/services/fileDownloadService.test.ts diff --git a/__tests__/unit/services/fileDownloadService.test.ts b/__tests__/unit/services/fileDownloadService.test.ts new file mode 100644 index 000000000..5ef68a988 --- /dev/null +++ b/__tests__/unit/services/fileDownloadService.test.ts @@ -0,0 +1,98 @@ +jest.mock('react-native-fs', () => ({ + mkdir: jest.fn(() => Promise.resolve()), + unlink: jest.fn(() => Promise.resolve()), + stat: jest.fn(() => Promise.resolve({ size: 1000 })), + hash: jest.fn(() => Promise.resolve('a'.repeat(64))), + moveFile: jest.fn(() => Promise.resolve()), + downloadFile: jest.fn(), + stopDownload: jest.fn(), +})); + +import RNFS from 'react-native-fs'; +import { downloadFileWithIntegrityCheck } from '../../../src/services/fileDownloadService'; + +const mockDownloadFile = RNFS.downloadFile as jest.Mock; +const mockStopDownload = RNFS.stopDownload as jest.Mock; + +const target = { + source: { + url: 'https://example.org/model.onnx', + expectedSha256: 'a'.repeat(64), + expectedSizeBytes: 1000, + }, + stagingPath: '/mock/staging/model.onnx.part', + finalPath: '/mock/models/model.onnx', +}; + +describe('fileDownloadService inactivity timeout', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('stops and reports a download that receives no data before the deadline', async () => { + mockDownloadFile.mockReturnValue({ + jobId: 42, + promise: new Promise(() => {}), + }); + + const pending = downloadFileWithIntegrityCheck(target, { + maxAttempts: 1, + inactivityTimeoutMs: 1000, + }); + await jest.advanceTimersByTimeAsync(1000); + + await expect(pending).resolves.toMatchObject({ + ok: false, + code: 'timeout', + message: 'download received no data for 1s', + }); + expect(mockStopDownload).toHaveBeenCalledWith(42); + expect(mockDownloadFile).toHaveBeenCalledWith( + expect.objectContaining({ + progressInterval: 1000, + readTimeout: 1000, + }), + ); + }); + + it('resets the deadline when download progress arrives', async () => { + let reportProgress: (() => void) | undefined; + let resolveDownload: + | ((result: { statusCode: number; bytesWritten: number }) => void) + | undefined; + mockDownloadFile.mockImplementation( + (options: { + progress?: (result: { + bytesWritten: number; + contentLength: number; + }) => void; + }) => { + reportProgress = () => + options.progress?.({ bytesWritten: 500, contentLength: 1000 }); + return { + jobId: 42, + promise: new Promise(resolve => { + resolveDownload = resolve; + }), + }; + }, + ); + + const pending = downloadFileWithIntegrityCheck(target, { + maxAttempts: 1, + inactivityTimeoutMs: 1000, + }); + await jest.advanceTimersByTimeAsync(750); + reportProgress?.(); + await jest.advanceTimersByTimeAsync(750); + expect(mockStopDownload).not.toHaveBeenCalled(); + + resolveDownload?.({ statusCode: 200, bytesWritten: 1000 }); + await expect(pending).resolves.toMatchObject({ ok: true }); + }); +}); diff --git a/src/services/fileDownloadService/index.ts b/src/services/fileDownloadService/index.ts index 6165b6505..d45491dc5 100644 --- a/src/services/fileDownloadService/index.ts +++ b/src/services/fileDownloadService/index.ts @@ -21,9 +21,11 @@ import logger from '../../utils/logger'; const DEFAULT_MAX_ATTEMPTS = 3; const DEFAULT_BASE_BACKOFF_MS = 1000; +export const DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS = 60_000; const RETRYABLE_CODES: ReadonlySet<DownloadErrorCode> = new Set([ 'network-error', + 'timeout', 'length-mismatch', 'checksum-mismatch', ]); @@ -69,6 +71,53 @@ async function cleanupStaging(stagingPath: string): Promise<void> { } } +interface DownloadInactivityWatchdog { + promise: Promise<never>; + reset: () => void; + didTimeout: () => boolean; + clear: () => void; +} + +function createDownloadInactivityWatchdog( + timeoutMs: number, + onTimeout: () => void, +): DownloadInactivityWatchdog { + let timer: ReturnType<typeof setTimeout> | undefined; + let timedOut = false; + let rejectTimeout: (error: Error) => void = () => {}; + const promise = new Promise<never>((_resolve, reject) => { + rejectTimeout = reject; + }); + const reset = () => { + clearTimeout(timer); + timer = setTimeout(() => { + timedOut = true; + onTimeout(); + rejectTimeout( + new Error(`download received no data for ${timeoutMs / 1000}s`), + ); + }, timeoutMs); + }; + return { + promise, + reset, + didTimeout: () => timedOut, + clear: () => clearTimeout(timer), + }; +} + +function downloadFailure( + error: unknown, + signal: AbortSignal | undefined, + timedOut: boolean, +): DownloadOutcome { + if (signal?.aborted) { + return failure('cancelled', 'download cancelled'); + } + const message = error instanceof Error ? error.message : String(error); + return failure(timedOut ? 'timeout' : 'network-error', message); +} + /** Where a download comes from and where it lands, staged and final. */ export interface DownloadTarget { source: DownloadSource; @@ -86,20 +135,38 @@ async function attemptDownload( await cleanupStaging(stagingPath); let contentLengthFromServer = 0; - const { jobId, promise } = RNFS.downloadFile({ + const inactivityTimeoutMs = + opts.inactivityTimeoutMs ?? DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS; + let jobId = -1; + const stopDownload = () => { + if (jobId >= 0) { + RNFS.stopDownload(jobId); + } + }; + const watchdog = createDownloadInactivityWatchdog( + inactivityTimeoutMs, + stopDownload, + ); + const download = RNFS.downloadFile({ fromUrl: source.url, toFile: stagingPath, headers: source.headers, - progressDivider: 5, + progressInterval: 1000, begin: (res: { contentLength: number }) => { contentLengthFromServer = res.contentLength; + watchdog.reset(); }, progress: (res: { bytesWritten: number; contentLength: number }) => { + watchdog.reset(); opts.onProgress?.(res.bytesWritten, res.contentLength); }, + readTimeout: inactivityTimeoutMs, }); + jobId = download.jobId; + const { promise } = download; + watchdog.reset(); - const onAbort = () => RNFS.stopDownload(jobId); + const onAbort = stopDownload; if (opts.signal?.aborted) { // The signal aborted before we could listen — an aborted signal never // fires 'abort' again, so stop the job directly. @@ -110,17 +177,12 @@ async function attemptDownload( let statusCode: number; try { - const result = await promise; + const result = await Promise.race([promise, watchdog.promise]); statusCode = result.statusCode; } catch (error) { - if (opts.signal?.aborted) { - return failure('cancelled', 'download cancelled'); - } - return failure( - 'network-error', - error instanceof Error ? error.message : String(error), - ); + return downloadFailure(error, opts.signal, watchdog.didTimeout()); } finally { + watchdog.clear(); opts.signal?.removeEventListener('abort', onAbort); } diff --git a/src/services/fileDownloadService/types.ts b/src/services/fileDownloadService/types.ts index 2a5a77a37..af3f8d181 100644 --- a/src/services/fileDownloadService/types.ts +++ b/src/services/fileDownloadService/types.ts @@ -1,6 +1,7 @@ export type DownloadErrorCode = | 'http-error' | 'network-error' + | 'timeout' | 'length-mismatch' | 'checksum-mismatch' | 'cancelled' @@ -18,6 +19,8 @@ export type DownloadOutcome = export interface DownloadOptions { onProgress?: (bytesWritten: number, contentLength: number) => void; signal?: AbortSignal; + /** Maximum time without download activity before stopping the native job. */ + inactivityTimeoutMs?: number; /** Total attempts including the first (default 3). */ maxAttempts?: number; /** Base for exponential backoff with full jitter (default 1000 ms). */ From ab0dd846ab067a2439e3da9f02cf3dd438b1f07d Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:17:41 +0200 Subject: [PATCH 5/9] fix(ios): store the OAuth redirect exactly as Entra returns it iOS sign-in has never worked. It fails silently, and the trailing slash in the redirect URI is why. Entra normalises a custom-scheme redirect that carries no path, so a request sent as `org.ganesha.elebook://oauthredirect` comes back as `org.ganesha.elebook://oauthredirect/`. AppAuth-iOS compares the callback against the configured redirect component by component, path included, so '' != '/' and shouldHandleURL: rejects it. OIDExternalUserAgentIOS then discards the BOOL from resumeExternalUserAgentFlowWithURL:, so the rejection is never reported and the authorization session waits forever. The person sees a spinner that never stops, with no error and nothing to retry. Android never noticed. Its intent filter matches on the scheme alone (appAuthRedirectScheme in android/app/build.gradle), so the extra slash is irrelevant there. Same config, same Entra registration, opposite outcomes. Captured on an iPhone 15 Pro by routing the flow through the external browser so the callback surfaced in application(_:open:): scheme: org.ganesha.elebook host: oauthredirect path: '/' resumed: REJECTED (url mismatch) full: org.ganesha.elebook://oauthredirect/?code=1.AQkA3Rhcn71... Storing the redirect the way Entra returns it makes the comparison succeed on iOS and changes nothing on Android, which never inspected the path. The pre-fix spelling is accepted and normalised so existing configs self-heal rather than failing validation. One migration consequence: getTokenStorageService digests redirectUrl, so the Keychain/Keystore service name moves and anyone already signed in is signed out once and has to sign in again. Observations, packs and the local database are untouched. iOS loses nothing, never having been able to sign in. The field contributes no isolation in any case -- it is validated to a single constant, so every deployment shares it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- __tests__/unit/configureApp.test.ts | 25 ++++++++++++++++++++++- __tests__/unit/deploymentConfig.test.ts | 10 +++++++-- deployment.example.json | 4 ++-- docs/setup.md | 4 ++-- scripts/configure-app.js | 27 ++++++++++++++++++++++++- 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/__tests__/unit/configureApp.test.ts b/__tests__/unit/configureApp.test.ts index 811245b00..83a6f4e4a 100644 --- a/__tests__/unit/configureApp.test.ts +++ b/__tests__/unit/configureApp.test.ts @@ -122,4 +122,27 @@ describe('deployment configuration', () => { configureApp(options); expect(fs.statSync(generated).mtimeMs).toBe(0); }); -}); \ No newline at end of file +}); + +describe('configureApp redirect normalisation', () => { + // Entra returns a custom-scheme redirect with a trailing slash appended. + // AppAuth-iOS compares the callback path against the configured redirect, so + // storing it without the slash makes iOS reject its own callback and hang. + // Android matches on scheme alone and is unaffected either way. + it('stores the redirect exactly as Entra returns it, with the trailing slash', () => { + const config = validateConfig({ ...example, redirectUrl: 'org.ganesha.elebook://oauthredirect/' }); + + expect(config.redirectUrl).toBe('org.ganesha.elebook://oauthredirect/'); + }); + + it('normalises the pre-fix spelling so existing configs self-heal', () => { + const config = validateConfig({ ...example, redirectUrl: 'org.ganesha.elebook://oauthredirect' }); + + expect(config.redirectUrl).toBe('org.ganesha.elebook://oauthredirect/'); + }); + + it('still rejects a redirect belonging to a different app', () => { + expect(() => validateConfig({ ...example, redirectUrl: 'different.app://oauthredirect/' })) + .toThrow(/redirectUrl/); + }); +}); diff --git a/__tests__/unit/deploymentConfig.test.ts b/__tests__/unit/deploymentConfig.test.ts index 215fc887c..5aea88b9d 100644 --- a/__tests__/unit/deploymentConfig.test.ts +++ b/__tests__/unit/deploymentConfig.test.ts @@ -22,7 +22,7 @@ const LONG_DEPLOYMENT = { tenantId: '77777777-7777-4777-8777-777777777777', mobileClientId: '88888888-8888-4888-8888-888888888888', apiClientId: '99999999-9999-4999-8999-999999999999', - redirectUrl: 'org.ganesha.elebook://oauthredirect', + redirectUrl: 'org.ganesha.elebook://oauthredirect/', }; describe('deployment token storage', () => { @@ -40,8 +40,14 @@ describe('deployment token storage', () => { it('derives the documented service name for the example deployment', () => { // Pinned on purpose: changing the derivation orphans tokens stored by earlier builds. + // + // This value moved once, when redirectUrl gained the trailing slash that + // iOS needs to accept its own OAuth callback. Anyone already signed in is + // signed out by that change and has to sign in again; observations, packs + // and the local database are untouched. iOS loses nothing, never having + // been able to sign in at all. expect(getTokenStorageService(example)).toBe( - 'org.ganesha.elebook.entra.92eda79296364b345b9cb882a37983eb15de4e4ac4d7c6aa511b6d2d5ed543ed', + 'org.ganesha.elebook.entra.df2facb18aac60dd0a72437d48a060371a0843065ae7fa0383d7a47f1d1a355b', ); }); diff --git a/deployment.example.json b/deployment.example.json index c1055ecd1..21abc816b 100644 --- a/deployment.example.json +++ b/deployment.example.json @@ -4,5 +4,5 @@ "tenantId": "11111111-1111-4111-8111-111111111111", "mobileClientId": "22222222-2222-4222-8222-222222222222", "apiClientId": "33333333-3333-4333-8333-333333333333", - "redirectUrl": "org.ganesha.elebook://oauthredirect" -} \ No newline at end of file + "redirectUrl": "org.ganesha.elebook://oauthredirect/" +} diff --git a/docs/setup.md b/docs/setup.md index 98bde0925..8a1dab347 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -13,7 +13,7 @@ The app needs exactly six public strings. Start with [deployment.example.json](. "tenantId": "11111111-1111-4111-8111-111111111111", "mobileClientId": "22222222-2222-4222-8222-222222222222", "apiClientId": "33333333-3333-4333-8333-333333333333", - "redirectUrl": "org.ganesha.elebook://oauthredirect" + "redirectUrl": "org.ganesha.elebook://oauthredirect/" } ``` @@ -24,7 +24,7 @@ The app needs exactly six public strings. Start with [deployment.example.json](. | `tenantId` | Tenant UUID. | | `mobileClientId` | Native public-client registration UUID. | | `apiClientId` | API registration UUID. | -| `redirectUrl` | Exactly `org.ganesha.elebook://oauthredirect`, matching the native registrations. | +| `redirectUrl` | Exactly `org.ganesha.elebook://oauthredirect/`, matching the native registrations. The trailing slash is required: Entra appends it to a custom-scheme redirect that has no path, and AppAuth-iOS compares the callback path against this value, so omitting it makes iOS reject its own callback and hang. The pre-fix spelling without the slash is accepted and normalised. | Pack acquisition has a narrower limit: project IDs and pack version strings must fit within 40 UTF-16 code units. For live setup, keep `projectId` within that limit even though configuration accepts up to 128 characters; see [pack candidate paths](../src/services/packDownloadService/candidate.ts). Identities are digested into short directory names, so their length does not affect on-device paths. diff --git a/scripts/configure-app.js b/scripts/configure-app.js index 1066cfc9f..7183a6e22 100644 --- a/scripts/configure-app.js +++ b/scripts/configure-app.js @@ -10,7 +10,29 @@ const CONFIG_FIELDS = [ 'redirectUrl', ]; const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; -const NATIVE_REDIRECT = 'org.ganesha.elebook://oauthredirect'; +/** + * The trailing slash is load-bearing on iOS, and is not cosmetic. + * + * Entra normalises a custom-scheme redirect that has no path, so a request + * sent as `org.ganesha.elebook://oauthredirect` comes back as + * `org.ganesha.elebook://oauthredirect/`. AppAuth-iOS compares the callback + * against the configured redirect component by component, including path, so + * '' != '/' and it rejects the callback. It then discards the rejection -- + * OIDExternalUserAgentIOS ignores the BOOL from + * resumeExternalUserAgentFlowWithURL: -- and the authorization session waits + * forever. Sign-in spins with no error and no way to retry. + * + * Android never noticed: its intent filter matches on the scheme alone + * (appAuthRedirectScheme in android/app/build.gradle), so the extra slash is + * irrelevant there. Storing the redirect exactly as Entra returns it makes the + * comparison succeed on iOS and changes nothing on Android. + * + * Confirmed on an iPhone 15 Pro: the callback arrives as + * `org.ganesha.elebook://oauthredirect/?code=...` with path '/'. + */ +const NATIVE_REDIRECT = 'org.ganesha.elebook://oauthredirect/'; +/** Pre-fix spelling. Accepted and normalised so existing configs self-heal. */ +const NATIVE_REDIRECT_LEGACY = 'org.ganesha.elebook://oauthredirect'; function validateApiUrl(value) { let url; @@ -49,6 +71,9 @@ function validateConfig(value) { if (!/^[a-z0-9][a-z0-9_-]{0,127}$/i.test(config.projectId)) { throw new Error('projectId must contain only letters, digits, underscores, and hyphens.'); } + if (config.redirectUrl === NATIVE_REDIRECT_LEGACY) { + config.redirectUrl = NATIVE_REDIRECT; + } if (config.redirectUrl !== NATIVE_REDIRECT) { throw new Error('redirectUrl must match the native redirect registered by this app.'); } From ac68b585e309202fcd6423a5d057f5e53cc59f76 Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:57:27 +0200 Subject: [PATCH 6/9] fix: keep downloads alive in the background and say why they failed Two more ways the same silent failure reached the field, found while testing the model download on a physical iPhone. RNFS.downloadFile never asked for a background session, so the transfer ran on a foreground URLSession and stopped the moment iOS suspended the app -- the screen locking part-way through an 80MB model was enough. The watchdog then correctly reported "no data for 60s", which was true and useless: the data stopped because iOS stopped it. Nine attempts moved roughly 245MB for an 82MB file and installed nothing, failing at scattered offsets that tracked when the person looked away rather than anything about the link. This is a regression, not an oversight. Upstream removed the foreground path deliberately ("use background downloads exclusively", 02b637e) and AppDelegate still services handleEventsForBackgroundURLSession -- plumbing for a background session nothing was requesting. The rewrite of this service lost the flag. Restoring it needs the watchdog to cope with suspension too. JS timers do not run while the app is suspended and fire late on wake, so a deadline armed before suspension would trip instantly against a transfer that had been progressing in the background the whole time. The watchdog now re-arms when the app becomes active and judges inactivity from then. Separately, prepareMiewidModel collapsed every non-checksum failure into status 'missing' and dropped outcome.code and outcome.message on the floor. A stalled transfer, an unreachable host, a 404 and a cancellation all rendered as "status: missing", with the real reason going to logger.warn, which release builds compile out. The record now carries the reason and the screen shows it: "timeout: download received no data for 60s" instead of one word. That one change turned an unexplained failure into a diagnosis on the first retry. The field is optional because records persisted by earlier builds do not have it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .../unit/services/fileDownloadService.test.ts | 40 +++++++++++++ .../unit/services/miewidModelManager.test.ts | 59 +++++++++++++++++++ src/screens/PacksScreen.tsx | 4 +- src/services/fileDownloadService/index.ts | 21 +++++++ src/services/miewidModelManager/index.ts | 14 ++++- src/types/wildlife.ts | 9 +++ 6 files changed, 143 insertions(+), 4 deletions(-) diff --git a/__tests__/unit/services/fileDownloadService.test.ts b/__tests__/unit/services/fileDownloadService.test.ts index 5ef68a988..5de5d6fcd 100644 --- a/__tests__/unit/services/fileDownloadService.test.ts +++ b/__tests__/unit/services/fileDownloadService.test.ts @@ -96,3 +96,43 @@ describe('fileDownloadService inactivity timeout', () => { await expect(pending).resolves.toMatchObject({ ok: true }); }); }); + +describe('background transfer', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + // A foreground URLSession stops when iOS suspends the app, so locking the + // screen part-way through an 80MB model killed the transfer. Upstream had + // already removed the foreground path ("use background downloads + // exclusively"); this service was rewritten without the flag while + // AppDelegate kept servicing handleEventsForBackgroundURLSession. + it('asks for a background session so a locked screen does not kill the transfer', async () => { + mockDownloadFile.mockReturnValue({ + jobId: 1, + promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), + }); + + await downloadFileWithIntegrityCheck(target); + + expect(mockDownloadFile).toHaveBeenCalledWith( + expect.objectContaining({ background: true }), + ); + }); + + it('re-arms the inactivity deadline when the app returns to the foreground', async () => { + const { AppState } = require('react-native'); + const addEventListener = jest.spyOn(AppState, 'addEventListener'); + mockDownloadFile.mockReturnValue({ + jobId: 1, + promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), + }); + + await downloadFileWithIntegrityCheck(target); + + // Suspended JS timers fire late on wake; without this the watchdog would + // trip against a transfer that progressed fine while backgrounded. + expect(addEventListener).toHaveBeenCalledWith('change', expect.any(Function)); + addEventListener.mockRestore(); + }); +}); diff --git a/__tests__/unit/services/miewidModelManager.test.ts b/__tests__/unit/services/miewidModelManager.test.ts index 9f01ef98d..af90b414d 100644 --- a/__tests__/unit/services/miewidModelManager.test.ts +++ b/__tests__/unit/services/miewidModelManager.test.ts @@ -313,3 +313,62 @@ describe('checkEmbeddingModelCompatibility', () => { ); }); }); + +describe('prepareMiewidModel failure reporting', () => { + const SOURCE = { + name: 'miewid', + version: '4.2.0', + url: 'https://example.org/miewid-4.2.onnx', + expectedSha256: 'abc', + expectedSizeBytes: 2000, + format: 'onnx' as const, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + // 'missing' covers a stalled transfer, an unreachable host, a 404 and a + // cancellation alike. Collapsing them lost the only detail the person in the + // field could act on -- the screen said "status: missing" for an 80MB model + // download that had repeatedly stalled on a weak link. + it('keeps the underlying reason when a download fails', async () => { + mockDownloadModel.mockResolvedValue({ + ok: false, + code: 'timeout', + message: 'download received no data for 60s', + }); + + const candidate = await prepareMiewidModel(SOURCE); + + expect(candidate.status).toBe('missing'); + expect(candidate.failureReason).toBe('timeout: download received no data for 60s'); + }); + + it('keeps the verification detail when a download is corrupt', async () => { + mockDownloadModel.mockResolvedValue({ + ok: false, + code: 'checksum-mismatch', + message: 'expected abc, got def', + }); + + const candidate = await prepareMiewidModel(SOURCE); + + expect(candidate.status).toBe('corrupt'); + expect(candidate.failureReason).toBe('expected abc, got def'); + }); + + it('leaves the reason null on a model that prepared cleanly', async () => { + mockDownloadModel.mockResolvedValue({ + ok: true, + path: '/models/miewid.onnx', + sha256: 'abc', + sizeBytes: 1024, + }); + + const candidate = await prepareMiewidModel(SOURCE); + + expect(candidate.status).toBe('ready'); + expect(candidate.failureReason).toBeNull(); + }); +}); diff --git a/src/screens/PacksScreen.tsx b/src/screens/PacksScreen.tsx index 78c8561c4..264ab4af5 100644 --- a/src/screens/PacksScreen.tsx +++ b/src/screens/PacksScreen.tsx @@ -158,7 +158,9 @@ export const PacksScreen: React.FC = () => { if (modelForPack.status !== 'ready') { Alert.alert( 'Download failed', - `The MiewID model could not be prepared (status: ${modelForPack.status}).`, + modelForPack.failureReason + ? `The MiewID model could not be prepared: ${modelForPack.failureReason}` + : `The MiewID model could not be prepared (status: ${modelForPack.status}).`, ); return; } diff --git a/src/services/fileDownloadService/index.ts b/src/services/fileDownloadService/index.ts index d45491dc5..4836aec26 100644 --- a/src/services/fileDownloadService/index.ts +++ b/src/services/fileDownloadService/index.ts @@ -1,3 +1,4 @@ +import { AppState } from 'react-native'; import RNFS from 'react-native-fs'; import type { DownloadErrorCode, @@ -152,6 +153,15 @@ async function attemptDownload( toFile: stagingPath, headers: source.headers, progressInterval: 1000, + // A foreground session stops the moment iOS suspends the app, so the screen + // locking part-way through an 80MB model was enough to kill the transfer -- + // no data would arrive, and the watchdog below would correctly but uselessly + // report a stall. Upstream removed the foreground path for this reason + // ("use background downloads exclusively"); the rewrite of this service lost + // the flag while AppDelegate kept handling + // handleEventsForBackgroundURLSession for a session nothing was asking for. + // Ignored on Android, which has its own long-running download path. + background: true, begin: (res: { contentLength: number }) => { contentLengthFromServer = res.contentLength; watchdog.reset(); @@ -166,6 +176,16 @@ async function attemptDownload( const { promise } = download; watchdog.reset(); + // JS timers do not run while iOS has the app suspended, so a watchdog armed + // before suspension fires the instant the app wakes -- against a background + // transfer that may have been progressing the whole time. Re-arm on wake and + // judge inactivity from then, not from whenever the app went away. + const appStateSubscription = AppState.addEventListener('change', nextState => { + if (nextState === 'active') { + watchdog.reset(); + } + }); + const onAbort = stopDownload; if (opts.signal?.aborted) { // The signal aborted before we could listen — an aborted signal never @@ -183,6 +203,7 @@ async function attemptDownload( return downloadFailure(error, opts.signal, watchdog.didTimeout()); } finally { watchdog.clear(); + appStateSubscription.remove(); opts.signal?.removeEventListener('abort', onAbort); } diff --git a/src/services/miewidModelManager/index.ts b/src/services/miewidModelManager/index.ts index fd8259204..f46c9afd5 100644 --- a/src/services/miewidModelManager/index.ts +++ b/src/services/miewidModelManager/index.ts @@ -51,6 +51,7 @@ const candidateRecord = ( status: 'downloading', verifiedAt: null, format: source.format, + failureReason: null, ...overrides, }); @@ -63,8 +64,9 @@ export async function prepareMiewidModel( try { outcome = await modelDownloadService.downloadModel(source, opts); } catch (error) { + const message = error instanceof Error ? error.message : String(error); logger.error('[MiewIDModelManager] Model preparation threw:', error); - return candidateRecord(source, { status: 'missing' }); + return candidateRecord(source, { status: 'missing', failureReason: message }); } if (outcome.ok) { @@ -80,12 +82,18 @@ export async function prepareMiewidModel( logger.error( `[MiewIDModelManager] Downloaded model failed verification: ${outcome.message}`, ); - return candidateRecord(source, { status: 'corrupt' }); + return candidateRecord(source, { + status: 'corrupt', + failureReason: outcome.message, + }); } logger.warn( `[MiewIDModelManager] Model preparation failed (${outcome.code}): ${outcome.message}`, ); - return candidateRecord(source, { status: 'missing' }); + return candidateRecord(source, { + status: 'missing', + failureReason: `${outcome.code}: ${outcome.message}`, + }); } /** diff --git a/src/types/wildlife.ts b/src/types/wildlife.ts index ac2cce840..966ed8749 100644 --- a/src/types/wildlife.ts +++ b/src/types/wildlife.ts @@ -30,6 +30,15 @@ export interface MiewIDModelRecord { status: MiewIDModelStatus; verifiedAt: string | null; format: ModelFormat; + /** + * Why a non-ready record ended up that way, for display. 'missing' covers + * every failure that is not an integrity mismatch -- a stalled transfer, an + * unreachable host, a 404, a cancellation -- and collapsing those into one + * word leaves the person in the field with nothing to act on. Null when the + * model is ready or still downloading, and absent on records written by + * builds from before it existed. + */ + failureReason?: string | null; } // === Embedding Pack Types === From 3cf49c663be1335713fa22219e2bdea1b7ba34b8 Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Tue, 22 Sep 2026 22:58:56 +0200 Subject: [PATCH 7/9] ci: pin the Android command-line tools away from the preview channel Every job that installs the Android SDK started dying in setup: Error: The process '.../cmdline-tools/16.0/bin/sdkmanager' failed with exit code 1 "To get started with the Android SDK Preview, you must agree to..." android-actions/setup-android@v3 follows a floating cmdline-tools default that moved to 16.0, whose sdkmanager demands agreement to the Android SDK *Preview* licence -- which accept-android-sdk-licenses does not cover. lint, test and android-build all failed within 30 seconds on every open PR, while typecheck stayed green because it is the only job that never installs the SDK. Nothing in the tree caused it and nothing in the tree could fix it. Pinning the tools version takes the preview channel out of the path. The value is a guess that CI has to confirm; if 13.0 is not a tag this action publishes, the setup step will say so plainly and the pin can move. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2cd6415dd..be132011b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,14 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 + with: + # Pinned away from the floating default. The action moved to + # cmdline-tools 16.0, whose sdkmanager demands agreement to the + # Android SDK *Preview* licence that accept-android-sdk-licenses does + # not cover, so every job touching the SDK died in setup with + # "sdkmanager failed with exit code 1". typecheck was the only job + # left standing because it is the only one that never installs it. + cmdline-tools-version: '13.0' - name: Install dependencies run: npm ci @@ -92,6 +100,14 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 + with: + # Pinned away from the floating default. The action moved to + # cmdline-tools 16.0, whose sdkmanager demands agreement to the + # Android SDK *Preview* licence that accept-android-sdk-licenses does + # not cover, so every job touching the SDK died in setup with + # "sdkmanager failed with exit code 1". typecheck was the only job + # left standing because it is the only one that never installs it. + cmdline-tools-version: '13.0' - name: Setup Ruby dependencies uses: ruby/setup-ruby@v1 @@ -170,6 +186,14 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 + with: + # Pinned away from the floating default. The action moved to + # cmdline-tools 16.0, whose sdkmanager demands agreement to the + # Android SDK *Preview* licence that accept-android-sdk-licenses does + # not cover, so every job touching the SDK died in setup with + # "sdkmanager failed with exit code 1". typecheck was the only job + # left standing because it is the only one that never installs it. + cmdline-tools-version: '13.0' - name: Install dependencies run: npm ci From ed0423070f48110c19c8be4f400db9679a62da5d Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Tue, 22 Sep 2026 23:01:00 +0200 Subject: [PATCH 8/9] ci: pin cmdline-tools by build number, not version string The previous pin used '13.0', which this action cannot resolve to a download -- setup failed with a bare HTTP 404 on all three SDK jobs. cmdline-tools-version takes the published build number; 11076708 is command-line tools 11.0, comfortably off the preview channel that was demanding an unaccepted licence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .github/workflows/ci.yml | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be132011b..1b3fd4f49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,7 +45,11 @@ jobs: # not cover, so every job touching the SDK died in setup with # "sdkmanager failed with exit code 1". typecheck was the only job # left standing because it is the only one that never installs it. - cmdline-tools-version: '13.0' + # + # This input takes the published build number, not a version string: + # 11076708 is command-line tools 11.0. A version like '13.0' is not a + # download the action can resolve and fails with a bare HTTP 404. + cmdline-tools-version: 11076708 - name: Install dependencies run: npm ci @@ -107,7 +111,11 @@ jobs: # not cover, so every job touching the SDK died in setup with # "sdkmanager failed with exit code 1". typecheck was the only job # left standing because it is the only one that never installs it. - cmdline-tools-version: '13.0' + # + # This input takes the published build number, not a version string: + # 11076708 is command-line tools 11.0. A version like '13.0' is not a + # download the action can resolve and fails with a bare HTTP 404. + cmdline-tools-version: 11076708 - name: Setup Ruby dependencies uses: ruby/setup-ruby@v1 @@ -193,7 +201,11 @@ jobs: # not cover, so every job touching the SDK died in setup with # "sdkmanager failed with exit code 1". typecheck was the only job # left standing because it is the only one that never installs it. - cmdline-tools-version: '13.0' + # + # This input takes the published build number, not a version string: + # 11076708 is command-line tools 11.0. A version like '13.0' is not a + # download the action can resolve and fails with a bare HTTP 404. + cmdline-tools-version: 11076708 - name: Install dependencies run: npm ci From 71288a992b66ab7bf006ef46684303f8b92688e8 Mon Sep 17 00:00:00 2001 From: Carolina Lopez <99345307+blclo@users.noreply.github.com> Date: Tue, 22 Sep 2026 23:02:34 +0200 Subject: [PATCH 9/9] Revert "ci: pin the Android command-line tools" Two attempts, neither worked, and I cannot test this locally. Pinning cmdline-tools by version string ('13.0') 404s -- the input takes a published build number. Pinning by build number (11076708) resolves and installs, and sdkmanager still exits 1 while walking the licence list. So the tools version was never the problem: licence acceptance itself is failing on the runner, and android-actions/setup-android@v3 is not getting past it. Backing the pin out rather than leaving a speculative change that does not help and misleads the next person. CI remains broken for every job that installs the Android SDK -- lint, test and android-build, on every open PR, independent of this branch. typecheck stays green because it is the only job that never touches the SDK. Fixing it properly means pinning setup-android to a known-good release or raising it upstream, which wants someone who can iterate against CI directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .github/workflows/ci.yml | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1b3fd4f49..2cd6415dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,18 +38,6 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 - with: - # Pinned away from the floating default. The action moved to - # cmdline-tools 16.0, whose sdkmanager demands agreement to the - # Android SDK *Preview* licence that accept-android-sdk-licenses does - # not cover, so every job touching the SDK died in setup with - # "sdkmanager failed with exit code 1". typecheck was the only job - # left standing because it is the only one that never installs it. - # - # This input takes the published build number, not a version string: - # 11076708 is command-line tools 11.0. A version like '13.0' is not a - # download the action can resolve and fails with a bare HTTP 404. - cmdline-tools-version: 11076708 - name: Install dependencies run: npm ci @@ -104,18 +92,6 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 - with: - # Pinned away from the floating default. The action moved to - # cmdline-tools 16.0, whose sdkmanager demands agreement to the - # Android SDK *Preview* licence that accept-android-sdk-licenses does - # not cover, so every job touching the SDK died in setup with - # "sdkmanager failed with exit code 1". typecheck was the only job - # left standing because it is the only one that never installs it. - # - # This input takes the published build number, not a version string: - # 11076708 is command-line tools 11.0. A version like '13.0' is not a - # download the action can resolve and fails with a bare HTTP 404. - cmdline-tools-version: 11076708 - name: Setup Ruby dependencies uses: ruby/setup-ruby@v1 @@ -194,18 +170,6 @@ jobs: - name: Setup Android SDK uses: android-actions/setup-android@v3 - with: - # Pinned away from the floating default. The action moved to - # cmdline-tools 16.0, whose sdkmanager demands agreement to the - # Android SDK *Preview* licence that accept-android-sdk-licenses does - # not cover, so every job touching the SDK died in setup with - # "sdkmanager failed with exit code 1". typecheck was the only job - # left standing because it is the only one that never installs it. - # - # This input takes the published build number, not a version string: - # 11076708 is command-line tools 11.0. A version like '13.0' is not a - # download the action can resolve and fails with a bare HTTP 404. - cmdline-tools-version: 11076708 - name: Install dependencies run: npm ci