diff --git a/__tests__/unit/services/parallelMmproj.test.ts b/__tests__/unit/services/parallelMmproj.test.ts index 22ebd386f..13067c9e9 100644 --- a/__tests__/unit/services/parallelMmproj.test.ts +++ b/__tests__/unit/services/parallelMmproj.test.ts @@ -711,6 +711,121 @@ describe('Parallel mmproj download', () => { }); }); + describe('watchBackgroundDownload — mmproj move-failure validation', () => { + // When the mmproj move fails, the target may already hold a valid mmproj + // (a re-download, or a file another model references). Validate the existing + // file READ-ONLY (size + GGUF magic): reuse if valid, downgrade to text-only + // if not — but never delete it, so this recovery path can't destroy a valid + // or shared sidecar. + const MMPROJ_PATH = `${MODELS_DIR}/vision-mmproj.gguf`; + + async function setupWithMoveFailure() { + stubStartDownload(['42', '43']); + const completeCbs = captureCompleteCallbacks(); + await performBackgroundDownload({ + modelId: 'test/model', + file: visionFile(), + modelsDir: MODELS_DIR, + backgroundDownloadContext: bgContext, + backgroundDownloadMetadataCallback: metadataCallback, + }); + // main (42) move succeeds; mmproj (43) move rejects (target already exists). + mockService.moveCompletedDownload.mockImplementation((id: string) => + id === '43' + ? Promise.reject(new Error('target exists')) + : Promise.resolve(`${MODELS_DIR}/vision.gguf`), + ); + mockedRNFS.exists.mockResolvedValue(true); + mockedRNFS.stat.mockResolvedValue({ size: 500_000_000 } as any); + const onComplete = jest.fn(); + watchBackgroundDownload({ + downloadId: '42', + modelsDir: MODELS_DIR, + backgroundDownloadContext: bgContext, + backgroundDownloadMetadataCallback: metadataCallback, + onComplete, + }); + const ctx = bgContext.get('42') as any; + return { completeCbs, onComplete, ctx }; + } + + it('keeps vision when the existing target is a valid GGUF', async () => { + const { completeCbs, onComplete, ctx } = await setupWithMoveFailure(); + (mockedRNFS.read as jest.Mock).mockResolvedValue('GGUF'); + + await completeCbs['43']?.({ downloadId: '43', fileName: 'mmproj.gguf' }); + await completeCbs['42']?.({ downloadId: '42', fileName: 'vision.gguf' }); + + expect(ctx.mmProjLocalPath).toBe(MMPROJ_PATH); + expect(mockedRNFS.unlink).not.toHaveBeenCalledWith(MMPROJ_PATH); + expect(onComplete).toHaveBeenCalledTimes(1); + expect(onComplete.mock.calls[0][0].mmProjPath).toBe(MMPROJ_PATH); + }); + + it('downgrades to text-only WITHOUT deleting when the target has bad magic bytes', async () => { + const { completeCbs, onComplete, ctx } = await setupWithMoveFailure(); + (mockedRNFS.read as jest.Mock).mockResolvedValue('XXXX'); + + await completeCbs['43']?.({ downloadId: '43', fileName: 'mmproj.gguf' }); + await completeCbs['42']?.({ downloadId: '42', fileName: 'vision.gguf' }); + + expect(ctx.mmProjLocalPath).toBeNull(); + // Recovery path must never delete the existing target (may be shared/valid-but-misread). + expect(mockedRNFS.unlink).not.toHaveBeenCalledWith(MMPROJ_PATH); + expect(onComplete).toHaveBeenCalledTimes(1); + expect(onComplete.mock.calls[0][0].mmProjPath).toBeUndefined(); + }); + + it('downgrades WITHOUT deleting when the target is smaller than expected', async () => { + const { completeCbs, onComplete, ctx } = await setupWithMoveFailure(); + // Good magic but short file (< expected 500MB) → invalid, but not ours to delete. + mockedRNFS.stat.mockResolvedValue({ size: 100 } as any); + (mockedRNFS.read as jest.Mock).mockResolvedValue('GGUF'); + + await completeCbs['43']?.({ downloadId: '43', fileName: 'mmproj.gguf' }); + await completeCbs['42']?.({ downloadId: '42', fileName: 'vision.gguf' }); + + expect(ctx.mmProjLocalPath).toBeNull(); + expect(mockedRNFS.unlink).not.toHaveBeenCalledWith(MMPROJ_PATH); + expect(onComplete.mock.calls[0][0].mmProjPath).toBeUndefined(); + }); + + it('keeps vision when the magic read throws (iOS RNFS.read bug)', async () => { + const { completeCbs, onComplete, ctx } = await setupWithMoveFailure(); + // RNFS.read rejects → hasGgufMagic returns null → accept, llama.rn validates on load. + (mockedRNFS.read as jest.Mock).mockRejectedValue(new Error('NSInteger bridging error')); + + await completeCbs['43']?.({ downloadId: '43', fileName: 'mmproj.gguf' }); + await completeCbs['42']?.({ downloadId: '42', fileName: 'vision.gguf' }); + + expect(ctx.mmProjLocalPath).toBe(MMPROJ_PATH); + expect(onComplete.mock.calls[0][0].mmProjPath).toBe(MMPROJ_PATH); + }); + + it('keeps vision when the magic read returns a short/empty string (iOS RNFS.read bug)', async () => { + const { completeCbs, onComplete, ctx } = await setupWithMoveFailure(); + // A truncated read is inconclusive, not a confirmed-bad file — must not downgrade. + (mockedRNFS.read as jest.Mock).mockResolvedValue(''); + + await completeCbs['43']?.({ downloadId: '43', fileName: 'mmproj.gguf' }); + await completeCbs['42']?.({ downloadId: '42', fileName: 'vision.gguf' }); + + expect(ctx.mmProjLocalPath).toBe(MMPROJ_PATH); + expect(onComplete.mock.calls[0][0].mmProjPath).toBe(MMPROJ_PATH); + }); + + it('downgrades to text-only when the target no longer exists', async () => { + const { completeCbs, onComplete, ctx } = await setupWithMoveFailure(); + mockedRNFS.exists.mockResolvedValue(false); + + await completeCbs['43']?.({ downloadId: '43', fileName: 'mmproj.gguf' }); + await completeCbs['42']?.({ downloadId: '42', fileName: 'vision.gguf' }); + + expect(ctx.mmProjLocalPath).toBeNull(); + expect(onComplete.mock.calls[0][0].mmProjPath).toBeUndefined(); + }); + }); + describe('watchBackgroundDownload — already-downloaded recovery', () => { it('persists already-downloaded models before firing onComplete', async () => { mockedRNFS.exists.mockResolvedValue(true); diff --git a/src/services/modelManager/download.ts b/src/services/modelManager/download.ts index 024b8a3df..c900746a1 100644 --- a/src/services/modelManager/download.ts +++ b/src/services/modelManager/download.ts @@ -59,18 +59,62 @@ export async function performBackgroundDownload(opts: PerformBackgroundDownloadO }); } +const GGUF_MAGIC = 'GGUF'; + +async function hasGgufMagic(path: string): Promise { + // Returns true if GGUF magic confirmed, false if confirmed invalid, null if + // the read is inconclusive. iOS RNFS.read has a known NSInteger bridging bug + // that can either throw or return a short/empty string; both are treated as + // inconclusive rather than invalid so we never destroy a valid file on a misread. + try { + const header = await RNFS.read(path, 4, 0, 'ascii'); + if (header.length < GGUF_MAGIC.length) return null; // short/empty read — inconclusive + return header.startsWith(GGUF_MAGIC); + } catch { + return null; + } +} + +// Read-only validity check: present, large enough, and not confirmed-corrupt. +// A null (inconclusive) magic read counts as valid — llama.rn validates on load. +// Never mutates the file, so it is safe to run against a possibly-shared target. +async function mmProjFileValid(path: string, expectedSize?: number): Promise { + try { + if (!(await RNFS.exists(path))) return false; + if (expectedSize) { + const stat = await RNFS.stat(path); + const actualSize = typeof stat.size === 'string' ? Number.parseInt(stat.size, 10) : stat.size; + if (actualSize < expectedSize) return false; + } + return (await hasGgufMagic(path)) !== false; + } catch { + return false; + } +} + +// Pre-download gate: a present-but-invalid file is deleted so it re-downloads clean. +// (This path owns the file exclusively — unlike the move-failure recovery path, +// which uses the read-only mmProjFileValid and never deletes.) async function checkMmProjExists(path: string | null, expectedSize?: number): Promise { if (!path) return true; const exists = await RNFS.exists(path); - if (!exists || !expectedSize) return exists; + if (!exists) return false; try { - const stat = await RNFS.stat(path); - const actualSize = typeof stat.size === 'string' ? Number.parseInt(stat.size, 10) : stat.size; - if (actualSize < expectedSize) { - logger.warn(`[ModelManager] mmproj partial (${actualSize}/${expectedSize}), re-downloading`); + if (expectedSize) { + const stat = await RNFS.stat(path); + const actualSize = typeof stat.size === 'string' ? Number.parseInt(stat.size, 10) : stat.size; + if (actualSize < expectedSize) { + logger.warn(`[ModelManager] mmproj partial (${actualSize}/${expectedSize}), re-downloading`); + await RNFS.unlink(path).catch(() => {}); + return false; + } + } + if ((await hasGgufMagic(path)) === false) { + logger.warn(`[ModelManager] mmproj failed GGUF magic check, re-downloading: ${path}`); await RNFS.unlink(path).catch(() => {}); return false; } + // magic ok or inconclusive: accept — llama.rn validates on load. return true; } catch { await RNFS.unlink(path).catch(() => {}); @@ -533,9 +577,19 @@ export function watchBackgroundDownload(opts: WatchDownloadOpts): void { try { await backgroundDownloadService.moveCompletedDownload(event.downloadId, ctx.mmProjLocalPath!); } catch (moveErr) { - const targetExists = ctx.mmProjLocalPath ? await RNFS.exists(ctx.mmProjLocalPath) : false; - if (!targetExists) { - logger.warn('[ModelManager] mmproj move failed and target not found, continuing without vision:', moveErr); + // Move can fail legitimately when a valid mmproj already sits at this path + // (a re-download, or another model sharing the path). Validate the existing + // file READ-ONLY: reuse it if valid, downgrade to text-only if not — but + // never delete it, so a misread or a file another model references can't be + // destroyed here (llama.rn re-validates on load). + const expectedSize = ctx.file.mmProjFile?.size; + const valid = ctx.mmProjLocalPath + ? await mmProjFileValid(ctx.mmProjLocalPath, expectedSize) + : false; + if (valid) { + logger.log('[ModelManager] mmproj move failed but existing target is valid, reusing:', moveErr); + } else { + logger.warn('[ModelManager] mmproj move failed and target invalid, continuing without vision:', moveErr); ctx.mmProjLocalPath = null; } }