From a03a3dd51b61ba94d8351c468529f8017795b0c1 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:21:08 -0700 Subject: [PATCH] Render png on mobile (#9087) * Add mobile image-diff previews via shared data-URI builder - Extracts a `buildImageDataUri` helper (src/shared/image-data-uri.ts) shared by the desktop ImageViewer and mobile, so both trim whitespace-wrapped base64 and skip non-previewable mimes (e.g. application/pdf) the same way. - Adds mobile-diff-image-preview.ts to render binary git.diff results (add/modify/ delete) as images instead of falling back to "Binary preview unavailable". - Extracts resolveMobileFileTabDoc to consolidate the session file-tab loading logic (diff/image/html/text) out of the route file for testability. * 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 * Regenerate skill bundle manifest for 1.4.144-rc.2 Co-authored-by: Orca * fix(review): trim comments to AGENTS.md's one/two-line why-only rule Comments in mobile-diff-image-preview.ts and mobile-file-tab-doc.ts ran 3-6 lines and narrated mechanism instead of stating only the non-obvious reason, per AGENTS.md's "Code Comments: Document the Why, Briefly" rule. Co-authored-by: Orca * Distinguish read failures from true deletions in binary diff results - Working-tree stat/readFile errors and relay reads previously collapsed onto the same empty-content signal as a genuine deletion, letting previewers fall back to stale original bytes on a failed read. - Add modifiedDeleted/missing flags through status.ts, git-handler-ops, and git-working-file-read so only proven deletions trigger the original-bytes fallback; failed reads now return null. - Tighten buildImageDataUri to accept only image/* mimes instead of special-casing application/pdf. * fix(relay): expect missing:false on index blob maxBuffer overflow readBlobAtIndex now returns a missing flag so staged deletions are distinct from size-capped binary reads; update the overflow test. * Allow opening deleted files to show pre-delete text or image diffs Deleted files can now be opened to view their pre-delete content via git.diff (including images via modifiedDeleted). Only unresolved conflicts remain unopenable. Centralizes the canOpen rule in canOpenMobileGitStatusEntry() to keep opener guards consistent across the mobile source control UI. --------- Co-authored-by: Orca --- .../app/h/[hostId]/session/[worktreeId].tsx | 94 +----------- .../files/mobile-diff-image-preview.test.ts | 123 ++++++++++++++++ mobile/src/files/mobile-diff-image-preview.ts | 32 +++++ mobile/src/files/mobile-file-tab-doc.test.ts | 136 ++++++++++++++++++ mobile/src/files/mobile-file-tab-doc.ts | 94 ++++++++++++ .../source-control/mobile-git-status.test.ts | 26 ++++ .../src/source-control/mobile-git-status.ts | 6 + .../mobile-source-control-screen-state.ts | 6 +- .../use-mobile-source-control-openers.ts | 10 +- src/main/git/status.test.ts | 31 ++++ src/main/git/status.ts | 42 ++++-- src/main/providers/windows-shell-args.ts | 3 +- src/relay/git-handler-blob-readers.test.ts | 3 +- src/relay/git-handler-ops.ts | 21 ++- src/relay/git-working-file-read.test.ts | 36 ++++- src/relay/git-working-file-read.ts | 34 +++-- .../src/components/editor/ImageViewer.tsx | 5 +- .../native-chat/native-chat-runtime-send.ts | 1 - src/shared/image-data-uri.test.ts | 30 ++++ src/shared/image-data-uri.ts | 20 +++ src/shared/types.ts | 7 + 21 files changed, 631 insertions(+), 129 deletions(-) create mode 100644 mobile/src/files/mobile-diff-image-preview.test.ts create mode 100644 mobile/src/files/mobile-diff-image-preview.ts create mode 100644 mobile/src/files/mobile-file-tab-doc.test.ts create mode 100644 mobile/src/files/mobile-file-tab-doc.ts create mode 100644 src/shared/image-data-uri.test.ts create mode 100644 src/shared/image-data-uri.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 0251da70490..d6bf1187811 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -141,7 +141,6 @@ import { saveCustomKeys, type CustomKey } from '../../../../src/components/CustomKeyModal' -import { buildMobileDiffLines } from '../../../../src/session/mobile-diff-lines' import { addMobileDiffComment, formatDiffComments, @@ -177,7 +176,7 @@ import { useMobileTerminalPaste } from '../../../../src/session/use-mobile-termi import { useTerminalLiveInputModePreference } from '../../../../src/session/use-terminal-live-input-mode-preference' import { MobileTerminalLiveInputStatus } from '../../../../src/session/MobileTerminalLiveInputStatus' import { MobileTerminalInputActions } from '../../../../src/session/MobileTerminalInputActions' -import { classifyMobileArtifact } from '../../../../src/session/mobile-artifact-kind' +import { resolveMobileFileTabDoc } from '../../../../src/files/mobile-file-tab-doc' import { openMobileTerminalFileTap } from '../../../../src/session/mobile-terminal-file-tap-open' import { useLiveWorktreeName } from '../../../../src/session/use-live-worktree-name' import { @@ -2043,93 +2042,12 @@ export default function SessionScreen() { } setFileDocs((prev) => new Map(prev).set(tab.id, { status: 'loading' })) try { - if (tab.diffSource === 'staged' || tab.diffSource === 'unstaged') { - const response = await client.sendRequest('git.diff', { - worktree: `id:${worktreeId}`, - filePath: tab.relativePath, - staged: tab.diffSource === 'staged' - }) - if (!response.ok) { - throw new Error((response as RpcFailure).error.message) - } - const result = (response as RpcSuccess).result as - | { - kind: 'text' - originalContent: string - modifiedContent: string - } - | { kind: 'binary' } - if (result.kind !== 'text') { - throw new Error('binary_file') - } - const diff = buildMobileDiffLines(result.originalContent, result.modifiedContent) - setFileDocs((prev) => - new Map(prev).set(tab.id, { - status: 'ready', - kind: 'diff', - lines: diff.lines, - truncated: diff.truncated - }) - ) - return - } - const artifactKind = classifyMobileArtifact(tab.relativePath) - if (artifactKind === 'image') { - const preview = await client.sendRequest('files.readPreview', { - worktree: `id:${worktreeId}`, - relativePath: tab.relativePath - }) - if (!preview.ok) { - throw new Error((preview as RpcFailure).error.message) - } - const result = (preview as RpcSuccess).result as { - content: string - isImage?: boolean - mimeType?: string - } - if (!result.isImage || !result.mimeType || result.content.length === 0) { - throw new Error('binary_file') - } - setFileDocs((prev) => - new Map(prev).set(tab.id, { - status: 'ready', - kind: 'image', - dataUri: `data:${result.mimeType};base64,${result.content}` - }) - ) - return - } - const response = await client.sendRequest('files.read', { - worktree: `id:${worktreeId}`, - relativePath: tab.relativePath + const doc = await resolveMobileFileTabDoc(client, { + worktreeId, + relativePath: tab.relativePath, + diffSource: tab.diffSource }) - if (!response.ok) { - throw new Error((response as RpcFailure).error.message) - } - const result = (response as RpcSuccess).result as { - content: string - truncated: boolean - byteLength: number - } - if (artifactKind === 'html') { - setFileDocs((prev) => - new Map(prev).set(tab.id, { - status: 'ready', - kind: 'html', - content: result.content - }) - ) - return - } - setFileDocs((prev) => - new Map(prev).set(tab.id, { - status: 'ready', - kind: 'file', - content: result.content, - truncated: result.truncated, - byteLength: result.byteLength - }) - ) + setFileDocs((prev) => new Map(prev).set(tab.id, doc)) } catch (err) { const message = err instanceof Error ? err.message : '' const previewMessage = diff --git a/mobile/src/files/mobile-diff-image-preview.test.ts b/mobile/src/files/mobile-diff-image-preview.test.ts new file mode 100644 index 00000000000..e63f532b28d --- /dev/null +++ b/mobile/src/files/mobile-diff-image-preview.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it } from 'vitest' +import { mobileDiffImageDataUri } from './mobile-diff-image-preview' + +describe('mobileDiffImageDataUri', () => { + it('renders a modified image diff from the post-change bytes', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: 'b2xk', + modifiedContent: 'bmV3', + isImage: true, + mimeType: 'image/png' + }) + ).toBe('data:image/png;base64,bmV3') + }) + + it('renders an added image diff (no original) from the modified bytes', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: '', + modifiedContent: 'bmV3', + isImage: true, + mimeType: 'image/png' + }) + ).toBe('data:image/png;base64,bmV3') + }) + + it('falls back to the original bytes for a proven deletion (modifiedDeleted)', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: 'b2xk', + originalIsBinary: true, + modifiedContent: '', + modifiedIsBinary: false, + modifiedDeleted: true, + isImage: true, + mimeType: 'image/jpeg' + }) + ).toBe('data:image/jpeg;base64,b2xk') + }) + + // The reviewer's read-failure case: a relay/SSH read returns an empty modified + // side with modifiedIsBinary false and no modifiedDeleted flag. Without a proven + // deletion, falling back to the original would show a stale pre-change image, so + // "unavailable" (null) is the honest result. + it('returns null on a read failure (empty modified, no modifiedDeleted flag)', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: 'b2xk', + originalIsBinary: true, + modifiedContent: '', + modifiedIsBinary: false, + isImage: true, + mimeType: 'image/png' + }) + ).toBeNull() + }) + + // Guards the >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({ + kind: 'binary', + originalContent: '', + modifiedContent: 'AAAA' + }) + ).toBeNull() + }) + + it('returns null when flagged as image but carrying no bytes', () => { + expect( + mobileDiffImageDataUri({ + kind: 'binary', + originalContent: '', + modifiedContent: '', + isImage: true, + mimeType: 'image/png' + }) + ).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 new file mode 100644 index 00000000000..09d7cd2ef36 --- /dev/null +++ b/mobile/src/files/mobile-diff-image-preview.ts @@ -0,0 +1,32 @@ +import { buildImageDataUri } from '../../../src/shared/image-data-uri' + +// modifiedDeleted marks a proven deletion (modified side genuinely absent); an +// empty modifiedContent alone can't, since a relay/SSH read failure also arrives +// empty with modifiedIsBinary false. +export type MobileBinaryDiffResult = { + kind: 'binary' + originalContent?: string + modifiedContent?: string + originalIsBinary?: boolean + modifiedIsBinary?: boolean + modifiedDeleted?: boolean + isImage?: boolean + mimeType?: string +} + +// Falls back to the original bytes only for a proven deletion; a read failure or +// size-capped modify — also empty on the modified side — returns null instead of +// the stale pre-change image. +export function mobileDiffImageDataUri(result: MobileBinaryDiffResult): string | null { + if (result.isImage !== true) { + return null + } + const modified = result.modifiedContent || '' + if (modified) { + return buildImageDataUri(result.mimeType, modified) + } + if (result.modifiedDeleted === true) { + return buildImageDataUri(result.mimeType, result.originalContent || '') + } + return null +} 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/mobile/src/files/mobile-file-tab-doc.ts b/mobile/src/files/mobile-file-tab-doc.ts new file mode 100644 index 00000000000..78977b5fd30 --- /dev/null +++ b/mobile/src/files/mobile-file-tab-doc.ts @@ -0,0 +1,94 @@ +import { buildImageDataUri } from '../../../src/shared/image-data-uri' +import { classifyMobileArtifact } from '../session/mobile-artifact-kind' +import { buildMobileDiffLines, type MobileDiffLine } from '../session/mobile-diff-lines' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcFailure, RpcSuccess } from '../transport/types' +import { mobileDiffImageDataUri, type MobileBinaryDiffResult } from './mobile-diff-image-preview' + +type FileTabDocClient = Pick + +// The ready doc a session file tab renders. Mirrors the ready arm of the route's +// FileDocState; kept in src so the loader stays testable without the route. +export type MobileFileTabDoc = + | { status: 'ready'; kind: 'file'; content: string; truncated: boolean; byteLength: number } + | { status: 'ready'; kind: 'diff'; lines: MobileDiffLine[]; truncated: boolean } + | { status: 'ready'; kind: 'image'; dataUri: string } + | { status: 'ready'; kind: 'html'; content: string } + +export type MobileFileTabDocRequest = { + worktreeId: string + relativePath: string + diffSource?: 'staged' | 'unstaged' | 'branch' | 'commit' +} + +// Throws 'binary_file'/'file_too_large'/the RPC error message; callers map those +// to error docs. +export async function resolveMobileFileTabDoc( + client: FileTabDocClient, + request: MobileFileTabDocRequest +): Promise { + const worktree = `id:${request.worktreeId}` + const { relativePath } = request + if (request.diffSource === 'staged' || request.diffSource === 'unstaged') { + const response = await client.sendRequest('git.diff', { + worktree, + filePath: relativePath, + staged: request.diffSource === 'staged' + }) + if (!response.ok) { + throw new Error((response as RpcFailure).error.message) + } + const result = (response as RpcSuccess).result as + | { kind: 'text'; originalContent: string; modifiedContent: string } + | MobileBinaryDiffResult + if (result.kind !== 'text') { + // Render image diffs (add/modify/delete) from the base64 the host already + // sends; only non-previewable binaries stay unavailable. + const dataUri = mobileDiffImageDataUri(result) + if (!dataUri) { + throw new Error('binary_file') + } + return { status: 'ready', kind: 'image', dataUri } + } + const diff = buildMobileDiffLines(result.originalContent, result.modifiedContent) + return { status: 'ready', kind: 'diff', lines: diff.lines, truncated: diff.truncated } + } + + const artifactKind = classifyMobileArtifact(relativePath) + if (artifactKind === 'image') { + const preview = await client.sendRequest('files.readPreview', { worktree, relativePath }) + if (!preview.ok) { + throw new Error((preview as RpcFailure).error.message) + } + const result = (preview as RpcSuccess).result as { + content: string + isImage?: boolean + mimeType?: string + } + const dataUri = result.isImage ? buildImageDataUri(result.mimeType, result.content) : null + if (!dataUri) { + throw new Error('binary_file') + } + return { status: 'ready', kind: 'image', dataUri } + } + + const response = await client.sendRequest('files.read', { worktree, relativePath }) + if (!response.ok) { + throw new Error((response as RpcFailure).error.message) + } + const result = (response as RpcSuccess).result as { + content: string + truncated: boolean + byteLength: number + } + if (artifactKind === 'html') { + return { status: 'ready', kind: 'html', content: result.content } + } + return { + status: 'ready', + kind: 'file', + content: result.content, + truncated: result.truncated, + byteLength: result.byteLength + } +} diff --git a/mobile/src/source-control/mobile-git-status.test.ts b/mobile/src/source-control/mobile-git-status.test.ts index 9c3575a75b2..a9e2b554f91 100644 --- a/mobile/src/source-control/mobile-git-status.test.ts +++ b/mobile/src/source-control/mobile-git-status.test.ts @@ -2,6 +2,7 @@ import { describe, expect, expectTypeOf, it } from 'vitest' import type { GitStatusResult } from '../../../src/shared/git-status-types' import { buildMobileSourceControlSections, + canOpenMobileGitStatusEntry, countStagedEntries, countUnstagedEntries, getStageablePaths, @@ -65,6 +66,31 @@ describe('mobile source control status helpers', () => { expect(isMobileGitDiscardableEntry(conflictedEntries[2])).toBe(false) }) + it('allows opening deleted files so pre-delete text/image diffs can load', () => { + expect( + canOpenMobileGitStatusEntry({ path: 'deleted.png', status: 'deleted', area: 'unstaged' }) + ).toBe(true) + expect( + canOpenMobileGitStatusEntry({ path: 'logo.png', status: 'modified', area: 'unstaged' }) + ).toBe(true) + expect( + canOpenMobileGitStatusEntry({ + path: 'conflict.ts', + status: 'modified', + area: 'unstaged', + conflictStatus: 'unresolved' + }) + ).toBe(false) + expect( + canOpenMobileGitStatusEntry({ + path: 'resolved.ts', + status: 'modified', + area: 'unstaged', + conflictStatus: 'resolved_locally' + }) + ).toBe(true) + }) + it('sorts entries by desktop-compatible conflict rank, then path', () => { const sections = buildMobileSourceControlSections([ { path: 'zeta.ts', status: 'modified', area: 'unstaged' }, diff --git a/mobile/src/source-control/mobile-git-status.ts b/mobile/src/source-control/mobile-git-status.ts index c6bfde9b8c2..65fcf61e211 100644 --- a/mobile/src/source-control/mobile-git-status.ts +++ b/mobile/src/source-control/mobile-git-status.ts @@ -90,6 +90,12 @@ export function isMobileGitDiscardableEntry(entry: MobileGitStatusEntry): boolea return entry.conflictStatus !== 'unresolved' && entry.conflictStatus !== 'resolved_locally' } +// Why: unresolved conflicts are not a stable file to open. Deletions are — +// git.diff still returns the pre-delete side (text or image via modifiedDeleted). +export function canOpenMobileGitStatusEntry(entry: MobileGitStatusEntry): boolean { + return entry.conflictStatus !== 'unresolved' +} + export function isMobileGitUnavailable(code: string | undefined, message: string | undefined) { return ( code === 'forbidden' || diff --git a/mobile/src/source-control/mobile-source-control-screen-state.ts b/mobile/src/source-control/mobile-source-control-screen-state.ts index eb20cdaa3ac..78a588d19ff 100644 --- a/mobile/src/source-control/mobile-source-control-screen-state.ts +++ b/mobile/src/source-control/mobile-source-control-screen-state.ts @@ -20,6 +20,7 @@ import type { MobileGitBranchCompareSummary } from './mobile-branch-compare' import { + canOpenMobileGitStatusEntry, isMobileGitDiscardableEntry, isMobileGitStageableEntry, type MobileGitFileStatus, @@ -58,15 +59,14 @@ export type MobileGitStatusEntryView = MobileGitStatusEntry & { } // Decorate raw status entries with the row-level capability/action-id fields the -// file list needs. Deleted/unresolved entries are not openable (matches the -// opener guards). +// file list needs. Opener guards must use the same canOpen rule. export function buildMobileGitStatusEntryViews( entries: readonly MobileGitStatusEntry[] ): MobileGitStatusEntryView[] { return entries.map((entry) => ({ ...entry, canDiscard: isMobileGitDiscardableEntry(entry), - canOpen: entry.status !== 'deleted' && entry.conflictStatus !== 'unresolved', + canOpen: canOpenMobileGitStatusEntry(entry), canStage: isMobileGitStageableEntry(entry), discardActionId: `discard:${entry.path}`, stageActionId: `stage:${entry.path}`, diff --git a/mobile/src/source-control/use-mobile-source-control-openers.ts b/mobile/src/source-control/use-mobile-source-control-openers.ts index be3febb4a80..bf0eff63cf6 100644 --- a/mobile/src/source-control/use-mobile-source-control-openers.ts +++ b/mobile/src/source-control/use-mobile-source-control-openers.ts @@ -12,7 +12,11 @@ import { canOpenMobileBranchCompareDiff, type MobileGitBranchChangeEntry } from './mobile-branch-compare' -import { isMobileGitUnavailable, type MobileGitStatusEntry } from './mobile-git-status' +import { + canOpenMobileGitStatusEntry, + isMobileGitUnavailable, + type MobileGitStatusEntry +} from './mobile-git-status' import { buildMobileReviewFileRoute } from './mobile-review-route' import type { GitDiffTextResult, @@ -70,7 +74,9 @@ export function useMobileSourceControlOpeners(params: Params) { const openFile = useCallback( async (entry: MobileGitStatusEntry) => { - if (entry.status === 'deleted' || entry.conflictStatus === 'unresolved') { + // Deletions are openable (pre-delete text/image via git.diff); only block + // unresolved conflicts, matching canOpenMobileGitStatusEntry / row UI. + if (!canOpenMobileGitStatusEntry(entry)) { return } if (openingPathRef.current || busyActionRef.current) { diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 8af536a31a6..fd5e16f91c5 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -476,6 +476,37 @@ describe('getDiff', () => { }) }) + it('flags a deleted image so previewers can fall back to the original bytes', async () => { + const pngBuffer = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00]) + gitExecFileAsyncBufferMock.mockResolvedValueOnce({ stdout: pngBuffer }) + statMock.mockRejectedValueOnce(Object.assign(new Error('missing'), { code: 'ENOENT' })) + + const result = await getDiff('/repo', 'assets/deleted.png', false) + + expect(result.kind).toBe('binary') + if (result.kind !== 'binary') { + throw new Error('expected binary diff result') + } + expect(result.modifiedDeleted).toBe(true) + expect(result.originalContent).toBe(pngBuffer.toString('base64')) + expect(result.modifiedContent).toBe('') + }) + + it('does not treat an unreadable working-tree image as a deletion', async () => { + const pngBuffer = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00]) + gitExecFileAsyncBufferMock.mockResolvedValueOnce({ stdout: pngBuffer }) + statMock.mockResolvedValueOnce({ isFile: () => true, size: 5 }) + readFileMock.mockRejectedValueOnce(new Error('EIO')) + + const result = await getDiff('/repo', 'assets/unreadable.png', false) + + expect(result.kind).toBe('binary') + if (result.kind !== 'binary') { + throw new Error('expected binary diff result') + } + expect(result.modifiedDeleted).toBeUndefined() + }) + it('coalesces concurrent identical staged diff reads while in flight', async () => { const leftBlob = deferredBuffer('head-content\n') const rightBlob = deferredBuffer('index-content\n') diff --git a/src/main/git/status.ts b/src/main/git/status.ts index a0c103b744c..ae818b52567 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -1329,6 +1329,7 @@ async function loadDiff( let modifiedContent = '' let originalIsBinary = false let modifiedIsBinary = false + let modifiedDeleted = false try { const leftBlob = staged @@ -1343,22 +1344,30 @@ async function loadDiff( const rightBlob = await readGitBlobAtIndexPath(worktreePath, filePath, options) modifiedContent = rightBlob.content modifiedIsBinary = rightBlob.isBinary + modifiedDeleted = !rightBlob.exists } else { const workingTreeBlob = await readWorkingTreeFile(path.join(worktreePath, filePath)) modifiedContent = workingTreeBlob.content modifiedIsBinary = workingTreeBlob.isBinary + modifiedDeleted = !workingTreeBlob.exists } } catch { // Fallback } - return buildDiffResult( + const result = buildDiffResult( originalContent, modifiedContent, originalIsBinary, modifiedIsBinary, filePath ) + // Why: mark a proven deletion so previewers can fall back to the original bytes + // without mistaking a read failure's empty modified side for a deletion. + if (result.kind === 'binary' && modifiedDeleted) { + return { ...result, modifiedDeleted: true } + } + return result } export async function getBranchCompare( @@ -1859,20 +1868,33 @@ async function readGitBlobAtOidPath( } async function readWorkingTreeFile(filePath: string): Promise { + let fileStat try { - const fileStat = await stat(filePath) - if (!fileStat.isFile()) { - return { content: '', isBinary: false, exists: false } - } - if (fileStat.size > MAX_GIT_SHOW_BYTES) { - // Why: git blob reads are capped through maxBuffer; mirror that bound for - // unstaged working-tree content before readFile can pull in huge assets. - return { content: '', isBinary: true, exists: true } + fileStat = await stat(filePath) + } catch (error) { + // Why: ENOENT means the working-tree file is genuinely gone (a deletion); + // any other stat error is a read failure, which must not be reported as an + // absence since callers fall back to the original bytes only for deletions. + return { + content: '', + isBinary: false, + exists: (error as NodeJS.ErrnoException)?.code !== 'ENOENT' } + } + if (!fileStat.isFile()) { + return { content: '', isBinary: false, exists: false } + } + if (fileStat.size > MAX_GIT_SHOW_BYTES) { + // Why: git blob reads are capped through maxBuffer; mirror that bound for + // unstaged working-tree content before readFile can pull in huge assets. + return { content: '', isBinary: true, exists: true } + } + try { const buffer = await readFile(filePath) return bufferToBlob(buffer, filePath) } catch { - return { content: '', isBinary: false, exists: false } + // Why: the file exists but could not be read — a read failure, not a deletion. + return { content: '', isBinary: false, exists: true } } } diff --git a/src/main/providers/windows-shell-args.ts b/src/main/providers/windows-shell-args.ts index c860fa1a11d..523aee72a6a 100644 --- a/src/main/providers/windows-shell-args.ts +++ b/src/main/providers/windows-shell-args.ts @@ -22,8 +22,7 @@ const CMD_UTF8_SETUP_COMMAND = 'chcp 65001 > nul' // (`❯` -> `Γ¥»`). Switch the console to UTF-8, then exec the normal interactive // login shell; cmd.exe and PowerShell already do the equivalent. The `;` (not // `&&`) keeps startup working even if chcp.com is missing. -const GIT_BASH_UTF8_LOGIN_COMMAND = - 'chcp.com 65001 >/dev/null 2>&1; exec "$BASH" --login -i' +const GIT_BASH_UTF8_LOGIN_COMMAND = 'chcp.com 65001 >/dev/null 2>&1; exec "$BASH" --login -i' /** Result of resolving a Windows shell to its launch args + effective cwd. * diff --git a/src/relay/git-handler-blob-readers.test.ts b/src/relay/git-handler-blob-readers.test.ts index 3a32eccaed1..b2b9e87ecbb 100644 --- a/src/relay/git-handler-blob-readers.test.ts +++ b/src/relay/git-handler-blob-readers.test.ts @@ -44,6 +44,7 @@ describe('git blob readers', () => { const result = await readBlobAtIndex(gitBuffer, '/repo', 'large.log') - expect(result).toEqual({ content: '', isBinary: true }) + // Why: overflow is size-capped content, not a staged deletion (missing: false). + expect(result).toEqual({ content: '', isBinary: true, missing: false }) }) }) diff --git a/src/relay/git-handler-ops.ts b/src/relay/git-handler-ops.ts index 395e31badb3..afe75a2b582 100644 --- a/src/relay/git-handler-ops.ts +++ b/src/relay/git-handler-ops.ts @@ -52,17 +52,19 @@ export async function readBlobAtIndex( gitBuffer: GitBufferExec, cwd: string, filePath: string -): Promise<{ content: string; isBinary: boolean }> { +): Promise<{ content: string; isBinary: boolean; missing: boolean }> { // Why: Git's `:` syntax expects forward slashes even on Windows. const gitPath = filePath.replace(/\\/g, '/') try { const buf = await gitBuffer(['show', '--end-of-options', `:${gitPath}`], cwd) - return bufferToBlob(buf, filePath) + return { ...bufferToBlob(buf, filePath), missing: false } } catch (error) { if (isGitBufferOverflowError(error)) { - return { content: '', isBinary: true } + return { content: '', isBinary: true, missing: false } } - return { content: '', isBinary: false } + // Why: a non-overflow failure means the path is absent from the index (a + // staged deletion), distinct from the size-capped case handled above. + return { content: '', isBinary: false, missing: true } } } @@ -91,6 +93,7 @@ export async function computeDiff( let modifiedContent = '' let originalIsBinary = false let modifiedIsBinary = false + let modifiedDeleted = false try { if (staged) { @@ -101,6 +104,7 @@ export async function computeDiff( const right = await readBlobAtIndex(git, worktreePath, filePath) modifiedContent = right.content modifiedIsBinary = right.isBinary + modifiedDeleted = right.missing } else { const left = compareAgainstHead ? await readBlobAtOid(git, worktreePath, 'HEAD', filePath) @@ -111,18 +115,25 @@ export async function computeDiff( const right = await readWorkingDiffFile(path.join(worktreePath, filePath)) modifiedContent = right.content modifiedIsBinary = right.isBinary + modifiedDeleted = right.missing } } catch { // Fallback to empty } - return buildDiffResult( + const result = buildDiffResult( originalContent, modifiedContent, originalIsBinary, modifiedIsBinary, filePath ) + // Why: mark a proven deletion so previewers can fall back to the original bytes + // without mistaking a read failure's empty modified side for a deletion. + if (result.kind === 'binary' && modifiedDeleted) { + return { ...result, modifiedDeleted: true } + } + return result } // ─── Branch compare ────────────────────────────────────────────────── diff --git a/src/relay/git-working-file-read.test.ts b/src/relay/git-working-file-read.test.ts index 4f11437568b..7f9824ead94 100644 --- a/src/relay/git-working-file-read.test.ts +++ b/src/relay/git-working-file-read.test.ts @@ -21,7 +21,8 @@ describe('readWorkingDiffFile', () => { await expect(readWorkingDiffFile(filePath)).resolves.toEqual({ content: 'hello', - isBinary: false + isBinary: false, + missing: false }) }) @@ -32,7 +33,32 @@ describe('readWorkingDiffFile', () => { await expect(readWorkingDiffFile(filePath)).resolves.toEqual({ content: '', - isBinary: true + isBinary: true, + missing: false + }) + }) + + 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, + missing: false + }) + }) + + it('reports an absent working-tree file as a deletion', async () => { + tmpDir = await mkdtemp(path.join(tmpdir(), 'relay-working-file-')) + const filePath = path.join(tmpDir, 'deleted.png') + + await expect(readWorkingDiffFile(filePath)).resolves.toEqual({ + content: '', + isBinary: false, + missing: true }) }) @@ -48,7 +74,8 @@ describe('readWorkingDiffFile', () => { await expect(readWorkingDiffFile(filePath)).resolves.toEqual({ content: pngBuffer.toString('base64'), - isBinary: true + isBinary: true, + missing: false }) }) @@ -59,7 +86,8 @@ describe('readWorkingDiffFile', () => { await expect(readWorkingDiffFile(filePath)).resolves.toEqual({ content: '', - isBinary: true + isBinary: true, + missing: false }) }) }) diff --git a/src/relay/git-working-file-read.ts b/src/relay/git-working-file-read.ts index 30c762c3a24..05cf1ebcdce 100644 --- a/src/relay/git-working-file-read.ts +++ b/src/relay/git-working-file-read.ts @@ -5,19 +5,31 @@ const MAX_RELAY_DIFF_WORKING_FILE_BYTES = 10 * 1024 * 1024 export async function readWorkingDiffFile( absPath: string -): Promise<{ content: string; isBinary: boolean }> { +): Promise<{ content: string; isBinary: boolean; missing: boolean }> { + let fileStat + try { + fileStat = await stat(absPath) + } catch (error) { + // Why: ENOENT means the working-tree file is genuinely gone (a deletion); + // any other stat error is a read failure we must not mistake for one, since + // callers fall back to the original bytes only for a proven deletion. + const missing = (error as NodeJS.ErrnoException)?.code === 'ENOENT' + return { content: '', isBinary: false, missing } + } + if (!fileStat.isFile()) { + return { content: '', isBinary: false, missing: true } + } + if (fileStat.size > MAX_RELAY_DIFF_WORKING_FILE_BYTES) { + // Why: mirror local git diff reads, which cap blob transfer at 10MB. + return { content: '', isBinary: true, missing: false } + } try { - const fileStat = await stat(absPath) - if (!fileStat.isFile()) { - return { content: '', isBinary: false } - } - if (fileStat.size > MAX_RELAY_DIFF_WORKING_FILE_BYTES) { - // Why: mirror local git diff reads, which cap blob transfer at 10MB. - return { content: '', isBinary: true } - } const buffer = await readFile(absPath) - return bufferToBlob(buffer, absPath) + // 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), missing: false } } catch { - return { content: '', isBinary: false } + // Why: the file exists but could not be read — a read failure, not a deletion. + return { content: '', isBinary: false, missing: false } } } diff --git a/src/renderer/src/components/editor/ImageViewer.tsx b/src/renderer/src/components/editor/ImageViewer.tsx index 408afd795b3..88b57b092b6 100644 --- a/src/renderer/src/components/editor/ImageViewer.tsx +++ b/src/renderer/src/components/editor/ImageViewer.tsx @@ -20,6 +20,7 @@ import { getZoomedImageLayoutSize } from './image-viewer-zoom' import { translate } from '@/i18n/i18n' +import { buildImageDataUri } from '../../../../shared/image-data-uri' const FALLBACK_IMAGE_MIME_TYPE = 'image/png' @@ -59,8 +60,8 @@ export default function ImageViewer({ const isPdf = mimeType === 'application/pdf' const isIntrinsicLayout = layout === 'intrinsic' const previewSrc = useMemo( - () => (cleanedContent && !isPdf ? `data:${mimeType};base64,${cleanedContent}` : null), - [cleanedContent, isPdf, mimeType] + () => buildImageDataUri(mimeType, cleanedContent), + [cleanedContent, mimeType] ) const imageError = previewSrc !== null && failedPreviewSrc === previewSrc const estimatedSize = useMemo(() => { diff --git a/src/renderer/src/components/native-chat/native-chat-runtime-send.ts b/src/renderer/src/components/native-chat/native-chat-runtime-send.ts index f1f97d0b530..a1dca86b65f 100644 --- a/src/renderer/src/components/native-chat/native-chat-runtime-send.ts +++ b/src/renderer/src/components/native-chat/native-chat-runtime-send.ts @@ -23,7 +23,6 @@ export { NATIVE_CHAT_ADVANCE_BUFFER_MS, NATIVE_CHAT_QUESTION_STEP_MS, NATIVE_CHA export const NATIVE_CHAT_IMAGE_ATTACHMENT_SETTLE_MS = 300 - /** Cancels an in-flight send's pending pty writes (the delayed Enter, and any * later question bodies/Enters). Safe to call after the send completes. */ export type NativeChatSendHandle = { diff --git a/src/shared/image-data-uri.test.ts b/src/shared/image-data-uri.test.ts new file mode 100644 index 00000000000..8244d2d652b --- /dev/null +++ b/src/shared/image-data-uri.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest' +import { buildImageDataUri } from './image-data-uri' + +describe('buildImageDataUri', () => { + it('builds a data URI from base64 image bytes', () => { + expect(buildImageDataUri('image/png', 'bmV3')).toBe('data:image/png;base64,bmV3') + }) + + it('strips whitespace from line-wrapped base64 payloads', () => { + expect(buildImageDataUri('image/png', 'bm\nV3\t bmV3\r\n')).toBe( + 'data:image/png;base64,bmV3bmV3' + ) + }) + + it('returns null for an empty payload', () => { + expect(buildImageDataUri('image/png', ' \n')).toBeNull() + }) + + it('returns null for a missing mime type', () => { + expect(buildImageDataUri(undefined, 'bmV3')).toBeNull() + }) + + it('returns null for application/pdf (not an source)', () => { + expect(buildImageDataUri('application/pdf', 'JVBER')).toBeNull() + }) + + it('returns null for a non-image mime such as application/octet-stream', () => { + expect(buildImageDataUri('application/octet-stream', 'AAAA')).toBeNull() + }) +}) diff --git a/src/shared/image-data-uri.ts b/src/shared/image-data-uri.ts new file mode 100644 index 00000000000..1501473560f --- /dev/null +++ b/src/shared/image-data-uri.ts @@ -0,0 +1,20 @@ +// Builds an inline `data:` URI for base64 image bytes, shared by the desktop +// editor ImageViewer and the mobile file preview so both decode images the same +// way. Strips whitespace from the payload (base64 from git diffs and SSH streams +// can arrive line-wrapped) and returns null when there is nothing an /RN +// can show — empty content or a non-image mime (PDF, octet-stream, …). +export function buildImageDataUri( + mimeType: string | undefined, + base64Content: string +): string | null { + // Only image/* renders in an /RN ; reject every other mime + // (application/pdf, application/octet-stream, …), not just PDF. + if (!mimeType?.startsWith('image/')) { + return null + } + const cleaned = base64Content.replace(/\s/g, '') + if (!cleaned) { + return null + } + return `data:${mimeType};base64,${cleaned}` +} diff --git a/src/shared/types.ts b/src/shared/types.ts index c805b9a0131..4d4301a0b6f 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -3742,6 +3742,13 @@ export type GitDiffBinaryResult = { isImage?: boolean /** MIME type for binary preview rendering, e.g. "image/png" or "application/pdf" */ mimeType?: string + /** + * True only when the modified side is a proven deletion (working-tree file gone + * or absent from the index) — distinct from an empty modified side caused by a + * read failure or size cap. Lets previewers fall back to the original bytes for + * a deletion without showing a stale image on a failed read. + */ + modifiedDeleted?: boolean } & ( | { originalIsBinary: true; modifiedIsBinary: boolean } | { originalIsBinary: boolean; modifiedIsBinary: true }