diff --git a/mobile/src/files/mobile-diff-image-preview.test.ts b/mobile/src/files/mobile-diff-image-preview.test.ts index 7a705112c32..3301e22d183 100644 --- a/mobile/src/files/mobile-diff-image-preview.test.ts +++ b/mobile/src/files/mobile-diff-image-preview.test.ts @@ -26,18 +26,37 @@ describe('mobileDiffImageDataUri', () => { ).toBe('data:image/png;base64,bmV3') }) - it('falls back to the original bytes for a deleted image', () => { + it('falls back to the original bytes for a deleted image (modified side absent)', () => { expect( mobileDiffImageDataUri({ kind: 'binary', originalContent: 'b2xk', + originalIsBinary: true, modifiedContent: '', + modifiedIsBinary: false, isImage: true, mimeType: 'image/jpeg' }) ).toBe('data:image/jpeg;base64,b2xk') }) + // Guards the SSH/relay + >size-cap case: the modified side IS a binary image but + // its bytes arrive empty. Falling back to the original would render the stale + // pre-change image; "unavailable" is the honest result. + it('returns null for a modify whose binary modified side is empty (no stale fallback)', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: 'b2xk', + originalIsBinary: true, + modifiedContent: '', + modifiedIsBinary: true, + isImage: true, + mimeType: 'image/png' + }) + ).toBeNull() + }) + it('returns null for a non-previewable binary (no isImage/mimeType)', () => { expect( mobileDiffImageDataUri({ @@ -59,4 +78,27 @@ describe('mobileDiffImageDataUri', () => { }) ).toBeNull() }) + + it('returns null for application/pdf even when flagged as previewable', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: '', + modifiedContent: 'JVBER', + isImage: true, + mimeType: 'application/pdf' + }) + ).toBeNull() + }) + + it('returns null when flagged as image but mimeType is missing', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: '', + modifiedContent: 'bmV3', + isImage: true + }) + ).toBeNull() + }) }) diff --git a/mobile/src/files/mobile-diff-image-preview.ts b/mobile/src/files/mobile-diff-image-preview.ts index 745cb513a2c..bb5908d7fca 100644 --- a/mobile/src/files/mobile-diff-image-preview.ts +++ b/mobile/src/files/mobile-diff-image-preview.ts @@ -2,21 +2,35 @@ import { buildImageDataUri } from '../../../src/shared/image-data-uri' // A git.diff result for a binary file. The host (git.diff) already sends base64 // image bytes plus isImage/mimeType for previewable formats, so mobile can render -// image diffs instead of falling back to "Binary preview unavailable". +// image diffs instead of falling back to "Binary preview unavailable". The +// originalIsBinary/modifiedIsBinary flags tell a deletion (modified side absent) +// apart from a modify whose bytes the host couldn't inline (relay/size-capped). export type MobileBinaryDiffResult = { kind: 'binary' originalContent?: string modifiedContent?: string + originalIsBinary?: boolean + modifiedIsBinary?: boolean isImage?: boolean mimeType?: string } // Builds a data URI for a binary image diff, preferring the modified (post-change) -// bytes and falling back to the original for deletions. Returns null for -// non-previewable binaries, which stay "Binary preview unavailable". +// bytes. Only falls back to the original for a genuine deletion — when the modified +// side is absent (not binary). Returns null for non-previewable binaries and when +// the modified side IS a binary image but arrives empty (e.g. a relay working-tree +// read or a >size-cap file), since showing the pre-change image there would be +// silently stale rather than the current content. export function mobileDiffImageDataUri(result: MobileBinaryDiffResult): string | null { if (result.isImage !== true) { return null } - return buildImageDataUri(result.mimeType, result.modifiedContent || result.originalContent || '') + const modified = result.modifiedContent || '' + if (modified) { + return buildImageDataUri(result.mimeType, modified) + } + if (result.modifiedIsBinary === true) { + return null + } + return buildImageDataUri(result.mimeType, result.originalContent || '') } diff --git a/mobile/src/files/mobile-file-tab-doc.test.ts b/mobile/src/files/mobile-file-tab-doc.test.ts new file mode 100644 index 00000000000..a9570e6cf44 --- /dev/null +++ b/mobile/src/files/mobile-file-tab-doc.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest' +import type { RpcResponse } from '../transport/types' +import { resolveMobileFileTabDoc } from './mobile-file-tab-doc' + +function ok(result: unknown): RpcResponse { + return { id: 'x', ok: true, result, _meta: { runtimeId: 'r' } } +} + +function fail(code: string, message: string): RpcResponse { + return { id: 'x', ok: false, error: { code, message }, _meta: { runtimeId: 'r' } } +} + +// Fake client that returns a canned response per RPC method. +function clientOf(byMethod: Record): { + sendRequest: (method: string) => Promise + calls: string[] +} { + const calls: string[] = [] + return { + calls, + sendRequest: (method: string) => { + calls.push(method) + const response = byMethod[method] + if (!response) { + throw new Error(`unexpected method ${method}`) + } + return Promise.resolve(response) + } + } +} + +const WT = { worktreeId: 'wt1' } + +describe('resolveMobileFileTabDoc', () => { + it('renders a staged text diff', async () => { + const client = clientOf({ + 'git.diff': ok({ kind: 'text', originalContent: 'a\n', modifiedContent: 'a\nb\n' }) + }) + const doc = await resolveMobileFileTabDoc(client, { + ...WT, + relativePath: 'a.ts', + diffSource: 'staged' + }) + expect(doc.kind).toBe('diff') + expect(client.calls).toEqual(['git.diff']) + }) + + it('renders an unstaged image diff from the modified bytes', async () => { + const client = clientOf({ + 'git.diff': ok({ + kind: 'binary', + originalContent: 'b2xk', + modifiedContent: 'bmV3', + modifiedIsBinary: true, + isImage: true, + mimeType: 'image/png' + }) + }) + const doc = await resolveMobileFileTabDoc(client, { + ...WT, + relativePath: 'm1.png', + diffSource: 'unstaged' + }) + expect(doc).toEqual({ status: 'ready', kind: 'image', dataUri: 'data:image/png;base64,bmV3' }) + }) + + it('throws binary_file for an image modify whose bytes are empty (no stale fallback)', async () => { + const client = clientOf({ + 'git.diff': ok({ + kind: 'binary', + originalContent: 'b2xk', + modifiedContent: '', + modifiedIsBinary: true, + isImage: true, + mimeType: 'image/png' + }) + }) + await expect( + resolveMobileFileTabDoc(client, { ...WT, relativePath: 'm1.png', diffSource: 'unstaged' }) + ).rejects.toThrow('binary_file') + }) + + it('throws binary_file for a non-image binary diff', async () => { + const client = clientOf({ 'git.diff': ok({ kind: 'binary', modifiedContent: 'AAAA' }) }) + await expect( + resolveMobileFileTabDoc(client, { ...WT, relativePath: 'a.bin', diffSource: 'unstaged' }) + ).rejects.toThrow('binary_file') + }) + + it('renders a live image preview via files.readPreview', async () => { + const client = clientOf({ + 'files.readPreview': ok({ content: 'bmV3', isImage: true, mimeType: 'image/png' }) + }) + const doc = await resolveMobileFileTabDoc(client, { ...WT, relativePath: 'logo.png' }) + expect(doc).toEqual({ status: 'ready', kind: 'image', dataUri: 'data:image/png;base64,bmV3' }) + expect(client.calls).toEqual(['files.readPreview']) + }) + + it('throws binary_file when readPreview returns no image bytes', async () => { + const client = clientOf({ + 'files.readPreview': ok({ content: '', isImage: true, mimeType: 'image/png' }) + }) + await expect( + resolveMobileFileTabDoc(client, { ...WT, relativePath: 'logo.png' }) + ).rejects.toThrow('binary_file') + }) + + it('renders html source via files.read', async () => { + const client = clientOf({ + 'files.read': ok({ content: '

hi

', truncated: false, byteLength: 11 }) + }) + const doc = await resolveMobileFileTabDoc(client, { ...WT, relativePath: 'page.html' }) + expect(doc).toEqual({ status: 'ready', kind: 'html', content: '

hi

' }) + }) + + it('renders a plain text file via files.read', async () => { + const client = clientOf({ + 'files.read': ok({ content: 'hello', truncated: true, byteLength: 5 }) + }) + const doc = await resolveMobileFileTabDoc(client, { ...WT, relativePath: 'notes.txt' }) + expect(doc).toEqual({ + status: 'ready', + kind: 'file', + content: 'hello', + truncated: true, + byteLength: 5 + }) + }) + + it('propagates the RPC error message when a read fails', async () => { + const client = clientOf({ 'files.read': fail('EIO', 'file_too_large') }) + await expect( + resolveMobileFileTabDoc(client, { ...WT, relativePath: 'notes.txt' }) + ).rejects.toThrow('file_too_large') + }) +}) diff --git a/src/relay/git-working-file-read.test.ts b/src/relay/git-working-file-read.test.ts index b4f8e3e6f7f..d4184775d19 100644 --- a/src/relay/git-working-file-read.test.ts +++ b/src/relay/git-working-file-read.test.ts @@ -35,4 +35,16 @@ describe('readWorkingDiffFile', () => { isBinary: true }) }) + + it('base64-encodes a previewable image by its extension', async () => { + tmpDir = await mkdtemp(path.join(tmpdir(), 'relay-working-file-')) + const filePath = path.join(tmpDir, 'icon.png') + const pngBytes = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x01]) + await writeFile(filePath, pngBytes) + + await expect(readWorkingDiffFile(filePath)).resolves.toEqual({ + content: pngBytes.toString('base64'), + isBinary: true + }) + }) }) diff --git a/src/relay/git-working-file-read.ts b/src/relay/git-working-file-read.ts index 2c7db8e9778..d2228dd04fe 100644 --- a/src/relay/git-working-file-read.ts +++ b/src/relay/git-working-file-read.ts @@ -16,7 +16,9 @@ export async function readWorkingDiffFile( return { content: '', isBinary: true } } const buffer = await readFile(absPath) - return bufferToBlob(buffer) + // Why: bufferToBlob needs the path's extension to know an image is + // previewable; omitting it made every relay-side binary diff empty. + return bufferToBlob(buffer, absPath) } catch { return { content: '', isBinary: false } }