Fix stale binary image fallback for empty modified diffs and relay reads

- mobileDiffImageDataUri now distinguishes a true deletion (modified
  side absent) from a modify whose binary bytes arrived empty
  (relay/size-cap cases), returning null instead of the stale
  pre-change image
- readWorkingDiffFile passes the file path to bufferToBlob so relay
  working-tree reads can detect previewable image extensions instead
  of always reporting empty binary content
- add mobile-file-tab-doc.test.ts covering diff/image/binary/text
  resolution paths
This commit is contained in:
Jinjing
2026-07-16 16:46:59 -07:00
parent 6fffc55102
commit 115b87efe4
5 changed files with 212 additions and 6 deletions
@@ -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()
})
})
+18 -4
View File
@@ -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 || '')
}
@@ -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<string, RpcResponse>): {
sendRequest: (method: string) => Promise<RpcResponse>
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: '<h1>hi</h1>', truncated: false, byteLength: 11 })
})
const doc = await resolveMobileFileTabDoc(client, { ...WT, relativePath: 'page.html' })
expect(doc).toEqual({ status: 'ready', kind: 'html', content: '<h1>hi</h1>' })
})
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')
})
})
+12
View File
@@ -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
})
})
})
+3 -1
View File
@@ -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 }
}