fix(remote-files): cap search replies, say when a diff is too large, and measure a 100 MB read on WSL (#34882)

Follow-ups from the large-file audit (E7):

- files.search had no byte budget: a result past the remote reply cap failed outright with
  reply_too_large. It now keeps the whole files that fit and marks the result truncated, as
  files.listAll and searchPaths already do. Local callers stay uncapped.
- A diff side past the 10 MiB read cap came back as binary, so the diff said "Binary file
  changed" for a large text file. Blob reads mark it `tooLarge`, the binary diff result carries an
  optional `tooLarge`, and both diff views say "File too large to diff". Older clients ignore the
  field and keep the binary view. buildDiffResult builds its binary sides without a cast.
- The real-WSL lane pages a 100 MB file over the plain distro's shared control socket in the
  editor's 384 KiB chunks and records the time, chunk count and orcad's VmHWM. Record-only: it
  asserts nothing, and a failure is recorded in the receipt.
This commit is contained in:
Neil
2026-10-28 02:01:16 -07:00
committed by GitHub
parent e7f9a914d9
commit d6918c7a1d
16 changed files with 274 additions and 28 deletions
+2 -1
View File
@@ -53,7 +53,8 @@ async function loadBranchDiff(
rightBlob.content,
leftBlob.isBinary,
rightBlob.isBinary,
args.filePath
args.filePath,
leftBlob.tooLarge === true || rightBlob.tooLarge === true
)
} catch {
return {
+4 -3
View File
@@ -4,7 +4,7 @@ import type { GitRuntimeOptions } from '../git-runtime-options'
import { gitRuntimeOptionsKey } from './git-runtime-options-cache-key'
import { gitDiffReadDedupe } from './git-read-cache-invalidation'
import { buildDiffResult } from './diff-result'
import { readGitBlobAtOidPath } from './git-blob-read'
import { readGitBlobAtOidPath, type GitBlobReadResult } from './git-blob-read'
export async function getCommitDiff(
worktreePath: string,
@@ -46,7 +46,7 @@ async function loadCommitDiff(
const [leftBlob, rightBlob] = await Promise.all([
args.parentOid
? readGitBlobAtOidPath(worktreePath, args.parentOid, leftPath, options)
: Promise.resolve({ content: '', isBinary: false }),
: Promise.resolve<GitBlobReadResult>({ content: '', isBinary: false, exists: false }),
readGitBlobAtOidPath(worktreePath, args.commitOid, args.filePath, options)
])
@@ -55,7 +55,8 @@ async function loadCommitDiff(
rightBlob.content,
leftBlob.isBinary,
rightBlob.isBinary,
args.filePath
args.filePath,
leftBlob.tooLarge === true || rightBlob.tooLarge === true
)
} catch {
return {
+10 -5
View File
@@ -8,21 +8,26 @@ export function buildDiffResult(
modifiedContent: string,
originalIsBinary: boolean,
modifiedIsBinary: boolean,
filePath?: string
filePath?: string,
/** A side was over the read cap rather than proven binary. */
tooLarge = false
): GitDiffResult {
if (originalIsBinary || modifiedIsBinary) {
const mimeType = filePath
? PREVIEWABLE_BINARY_MIME_TYPES[path.extname(filePath).toLowerCase()]
: undefined
const sides = originalIsBinary
? { originalIsBinary: true as const, modifiedIsBinary }
: { originalIsBinary: false, modifiedIsBinary: true as const }
return {
kind: 'binary',
originalContent,
modifiedContent,
originalIsBinary,
modifiedIsBinary,
...sides,
// Why: renderer still checks legacy `isImage` before previewing, so set it for PDFs too until the contract is renamed.
...(mimeType ? { isImage: true, mimeType } : {})
} as GitDiffResult
...(mimeType ? { isImage: true, mimeType } : {}),
...(tooLarge ? { tooLarge: true } : {})
}
}
// Why: over the render limit, return metadata instead of huge text so the renderer can show fallback UI.
@@ -0,0 +1,64 @@
import { rm, writeFile } from 'node:fs/promises'
import { execFileSync } from 'node:child_process'
import { mkdtempSync } from 'node:fs'
import { tmpdir } from 'node:os'
import * as path from 'node:path'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { getDiff } from './file-diff'
import { MAX_GIT_SHOW_BYTES } from './git-show-max-bytes'
import { invalidateGitReadCaches } from './git-read-cache-invalidation'
function git(dir: string, args: string[]): void {
execFileSync('git', args, { cwd: dir, stdio: 'pipe' })
}
// Text, not binary: only its size keeps it out of a text diff.
const largeText = (): string => 'a line of text\n'.repeat(Math.ceil(MAX_GIT_SHOW_BYTES / 15) + 1)
describe('a diff side past the read cap', () => {
let repo: string
beforeEach(() => {
repo = mkdtempSync(path.join(tmpdir(), 'orca-diff-too-large-'))
git(repo, ['init'])
git(repo, ['config', 'user.email', 'test@example.com'])
git(repo, ['config', 'user.name', 'Test User'])
invalidateGitReadCaches()
})
afterEach(async () => {
invalidateGitReadCaches()
await rm(repo, { recursive: true, force: true })
})
it('is marked too large, still binary for older readers, from the working tree and from Git', async () => {
await writeFile(path.join(repo, 'big.txt'), 'small\n')
git(repo, ['add', '.'])
git(repo, ['commit', '-m', 'initial'])
await writeFile(path.join(repo, 'big.txt'), largeText())
expect(await getDiff(repo, 'big.txt', false)).toMatchObject({
kind: 'binary',
modifiedIsBinary: true,
tooLarge: true
})
git(repo, ['add', 'big.txt'])
invalidateGitReadCaches()
expect(await getDiff(repo, 'big.txt', true)).toMatchObject({
kind: 'binary',
modifiedIsBinary: true,
tooLarge: true
})
})
it('leaves a real binary unmarked', async () => {
await writeFile(path.join(repo, 'blob.bin'), Buffer.from([1, 0, 2]))
git(repo, ['add', '.'])
git(repo, ['commit', '-m', 'initial'])
await writeFile(path.join(repo, 'blob.bin'), Buffer.from([3, 0, 4]))
const result = await getDiff(repo, 'blob.bin', false)
expect(result).toMatchObject({ kind: 'binary', modifiedIsBinary: true })
expect(result).not.toHaveProperty('tooLarge')
})
})
+5 -1
View File
@@ -141,6 +141,7 @@ async function loadDiff(
let modifiedIsBinary = false
let modifiedDeleted = false
let readFailed = false
let tooLarge = false
try {
if (staged) {
@@ -155,6 +156,7 @@ async function loadDiff(
modifiedIsBinary = rightBlob.isBinary
modifiedDeleted = !rightBlob.exists && !rightBlob.failed
readFailed = leftBlob.failed === true || rightBlob.failed === true
tooLarge = leftBlob.tooLarge === true || rightBlob.tooLarge === true
} else {
// The index-to-HEAD fallback is sequential internally, but independent of the working-tree read.
// WSL Git can use a Linux cwd while Node needs its Win32 spelling.
@@ -173,6 +175,7 @@ async function loadDiff(
modifiedIsBinary = workingTreeBlob.isBinary
modifiedDeleted = !workingTreeBlob.exists
readFailed = leftBlob.failed === true || workingTreeBlob.failed === true
tooLarge = leftBlob.tooLarge === true || workingTreeBlob.tooLarge === true
}
} catch {
readFailed = true
@@ -183,7 +186,8 @@ async function loadDiff(
modifiedContent,
originalIsBinary,
modifiedIsBinary,
filePath
filePath,
tooLarge
)
// Mark proven deletions so previewers can distinguish them from failed empty reads.
if (result.kind === 'binary' && modifiedDeleted) {
+12 -3
View File
@@ -15,6 +15,15 @@ export type GitBlobReadResult = {
exists: boolean
/** Unknown presence: never cache a diff whose empty side came from an incomplete read. */
failed?: boolean
/** Past MAX_GIT_SHOW_BYTES: left unread and reported binary, so older diff readers stay safe. */
tooLarge?: boolean
}
const TOO_LARGE_BLOB: GitBlobReadResult = {
content: '',
isBinary: true,
exists: true,
tooLarge: true
}
export async function readUnstagedLeftBlob(
@@ -46,7 +55,7 @@ export async function readGitBlobAtIndexPath(
return { ...bufferToBlob(stdout, filePath), exists: true }
} catch (error) {
if (isMaxBufferOverflowError(error)) {
return { content: '', isBinary: true, exists: true }
return TOO_LARGE_BLOB
}
return {
content: '',
@@ -77,7 +86,7 @@ export async function readGitBlobAtOidPath(
return { ...bufferToBlob(stdout, filePath), exists: true }
} catch (error) {
if (isMaxBufferOverflowError(error)) {
return { content: '', isBinary: true, exists: true }
return TOO_LARGE_BLOB
}
return {
content: '',
@@ -102,7 +111,7 @@ export async function readWorkingTreeFile(filePath: string): Promise<GitBlobRead
}
if (fileStat.size > MAX_GIT_SHOW_BYTES) {
// Mirror the Git buffer cap so filesystem reads cannot load huge assets.
return { content: '', isBinary: true, exists: true }
return TOO_LARGE_BLOB
}
try {
const buffer = await readFile(filePath)
@@ -54,6 +54,7 @@ export async function buildSubmoduleInnerCommitRangeDiff(
let modifiedContent = ''
let originalIsBinary = false
let modifiedIsBinary = false
let tooLarge = false
try {
const left = await readGitBlobAtOidPath(submoduleWorktreePath, fromOid, innerPath, options)
originalContent = left.content
@@ -61,12 +62,14 @@ export async function buildSubmoduleInnerCommitRangeDiff(
const right = await readGitBlobAtOidPath(submoduleWorktreePath, toOid, innerPath, options)
modifiedContent = right.content
modifiedIsBinary = right.isBinary
tooLarge = left.tooLarge === true || right.tooLarge === true
} catch {}
return buildDiffResult(
originalContent,
modifiedContent,
originalIsBinary,
modifiedIsBinary,
innerPath
innerPath,
tooLarge
)
}
+17 -13
View File
@@ -26,6 +26,7 @@ import {
ResolveTerminalPath,
ServerDirectoryBrowse
} from '../../../../shared/rpc-contract/files-params'
import { limitSearchResultToContentBudget } from '../../../../shared/search-result-transport-budget'
let filesWatchSubscriptionSeq = 0
@@ -174,19 +175,22 @@ export const FILE_METHODS = [
name: 'files.search',
permission: 'workspace',
params: FileSearch,
handler: async (params, { runtime, signal }) =>
runtime.searchRuntimeFiles(
params.worktree,
{
query: params.query,
caseSensitive: params.caseSensitive,
wholeWord: params.wholeWord,
useRegex: params.useRegex,
includePattern: params.includePattern,
excludePattern: params.excludePattern,
maxResults: params.maxResults
},
{ signal }
handler: async (params, { runtime, clientKind, requestId, signal }) =>
limitSearchResultToContentBudget(
await runtime.searchRuntimeFiles(
params.worktree,
{
query: params.query,
caseSensitive: params.caseSensitive,
wholeWord: params.wholeWord,
useRegex: params.useRegex,
includePattern: params.includePattern,
excludePattern: params.excludePattern,
maxResults: params.maxResults
},
{ signal }
),
remoteFileContentBudget(clientKind, requestId)
)
}),
defineMethod({
@@ -486,6 +486,56 @@ async function updatePhase(distro: string): Promise<Receipt> {
return result
}
const LARGE_FILE_BYTES = 100 * 1024 * 1024
// The editor's chunk size (runtime-paged-file-read.ts).
const LARGE_FILE_CHUNK_BYTES = 384 * 1024
/**
* Record-only (E7): a 100 MB file paged over the host the way the editor pages one, on the shared
* control socket the editor uses, with orcad's peak RSS. Failures are recorded, never asserted.
*/
async function measureLargeFileRead(
control: RemoteRuntimeSharedControlConnection,
orcadPid: number | null,
distro: string
): Promise<Receipt> {
if (orcadPid === null) {
return { error: 'no orcad pid' }
}
try {
const repo = (
await guest(
distro,
`set -e; d="${LANE_REPO}"; head -c ${LARGE_FILE_BYTES} /dev/zero | tr '\\0' a > "$d/big.txt"; ` +
'printf %s "$d"'
)
).trim()
// Reset the peak, so VmHWM is this read's.
await asRoot(distro, `echo 5 > /proc/${orcadPid}/clear_refs`)
const started = performance.now()
let chunks = 0
for (let offset = 0; offset < LARGE_FILE_BYTES; offset += LARGE_FILE_CHUNK_BYTES) {
await control.request(
'files.readChunk',
{
worktree: `path:${repo}`,
relativePath: 'big.txt',
offset,
length: LARGE_FILE_CHUNK_BYTES
},
60_000
)
chunks += 1
}
const readMs = Math.round(performance.now() - started)
const peak = (await guest(distro, `grep VmHWM /proc/${orcadPid}/status`)).trim()
await guest(distro, `rm -f "${repo}/big.txt"`)
return { bytes: LARGE_FILE_BYTES, chunks, readMs, orcadVmHWM: peak }
} catch (error) {
return { error: error instanceof Error ? error.message : String(error) }
}
}
async function launchPhase(distro: string, systemd: boolean): Promise<Receipt> {
const result: Receipt = {}
visibleWork = true
@@ -524,6 +574,9 @@ async function launchPhase(distro: string, systemd: boolean): Promise<Receipt> {
clientCapabilities: ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES
})
await control.request('status.get', undefined, 30_000)
if (!systemd) {
result.largeFileRead = await measureLargeFileRead(control, connection.pid, distro)
}
result.watchFiresOnNormalCloses = await countWatchFiresAcrossRpcs(connection, 10)
expect(result.watchFiresOnNormalCloses, 'normal socket closes must not read as orcad exit').toBe(
0
@@ -1,3 +1,4 @@
import { DiffTooLargeNotice } from './DiffTooLargeNotice'
import { useEffect, useLayoutEffect, useRef } from 'react'
import type { editor } from 'monaco-editor'
import { lazyWithRetry as lazy } from '@/lib/lazy-with-retry'
@@ -152,7 +153,9 @@ export function DiffSectionBody({
</Button>
</div>
) : section.diffResult?.kind === 'binary' ? (
section.diffResult.isImage ? (
section.diffResult.tooLarge ? (
<DiffTooLargeNotice />
) : section.diffResult.isImage ? (
<ImageDiffViewer
originalContent={section.diffResult.originalContent}
modifiedContent={section.diffResult.modifiedContent}
@@ -0,0 +1,21 @@
import React from 'react'
import { translate } from '@/i18n/i18n'
/** A diff side past the 10 MB read cap: Orca leaves it unread, so there is nothing to compare. */
export function DiffTooLargeNotice(): React.JSX.Element {
return (
<div className="flex h-full items-center justify-center px-6 text-center">
<div className="space-y-2">
<div className="text-sm font-medium text-foreground">
{translate('editor.diffTooLarge.title', 'File too large to diff')}
</div>
<div className="text-xs text-muted-foreground">
{translate(
'editor.diffTooLarge.detail',
'One version of this file is over 10 MB, so Orca doesn’t load it for a text diff.'
)}
</div>
</div>
</div>
)
}
@@ -1,3 +1,4 @@
import { DiffTooLargeNotice } from './DiffTooLargeNotice'
import { translate } from '@/i18n/i18n'
import type { OpenFile } from '@/store/slices/editor'
import type { GitDiffResult } from '../../../../shared/git-diff-compare-types'
@@ -54,6 +55,9 @@ export function EditorDiffFileSurface({
const isEditable = activeFile.diffSource === 'unstaged'
if (diffContent.kind === 'binary') {
if (diffContent.tooLarge) {
return <DiffTooLargeNotice />
}
if (diffContent.isImage) {
return (
<ImageDiffViewer
+4
View File
@@ -278,6 +278,10 @@
"fileLoad": {
"hostUnresolved": "The host couldn't find this file's workspace. It may have been removed, or the host may not know about it yet. Retry, or close this tab from the tab strip.",
"remoteTooLarge": "This file is too large to open from its host."
},
"diffTooLarge": {
"title": "File too large to diff",
"detail": "One version of this file is over 10 MB, so Orca doesn’t load it for a text diff."
}
},
"githubChecks": {
+2
View File
@@ -63,6 +63,8 @@ export type GitDiffBinaryResult = {
mimeType?: string
/** True only for a proven deletion — distinct from an empty modified side caused by a read failure or size cap. */
modifiedDeleted?: boolean
/** A side passed the 10 MiB read cap: too large to diff, not necessarily binary. Older clients show it as binary. */
tooLarge?: boolean
} & (
| { originalIsBinary: true; modifiedIsBinary: boolean }
| { originalIsBinary: boolean; modifiedIsBinary: true }
@@ -0,0 +1,37 @@
import { describe, expect, it } from 'vitest'
import type { SearchFileResult, SearchResult } from './code-search-types'
import { limitSearchResultToContentBudget } from './search-result-transport-budget'
function file(index: number, lineBytes: number): SearchFileResult {
return {
filePath: `/repo/src/file-${index}.ts`,
relativePath: `src/file-${index}.ts`,
matches: [{ line: 1, column: 1, matchLength: 3, lineContent: 'x'.repeat(lineBytes) }]
}
}
function result(files: SearchFileResult[]): SearchResult {
return { files, totalMatches: files.length, truncated: false }
}
describe('limitSearchResultToContentBudget', () => {
it('leaves a result that fits, or an uncapped local one, as it is', () => {
const small = result([file(1, 10)])
expect(limitSearchResultToContentBudget(small, 64 * 1024)).toBe(small)
const large = result(Array.from({ length: 200 }, (_, index) => file(index, 400)))
expect(limitSearchResultToContentBudget(large, undefined)).toBe(large)
})
it('keeps whole files that fit the reply and marks the rest truncated', () => {
const large = result(Array.from({ length: 200 }, (_, index) => file(index, 400)))
const maxBytes = 16 * 1024
const limited = limitSearchResultToContentBudget(large, maxBytes)
expect(limited.truncated).toBe(true)
expect(limited.files.length).toBeGreaterThan(0)
expect(limited.files.length).toBeLessThan(200)
expect(limited.files).toEqual(large.files.slice(0, limited.files.length))
expect(limited.totalMatches).toBe(limited.files.length)
expect(Buffer.byteLength(JSON.stringify(limited), 'utf8')).toBeLessThanOrEqual(maxBytes)
})
})
@@ -0,0 +1,31 @@
import type { SearchFileResult, SearchResult } from './code-search-types'
import { remoteRpcResultExceedsContentBudget } from './remote-rpc-content-budget'
/**
* `result` cut to whole files that fit one remote reply, marked truncated: a search past the reply
* cap otherwise fails outright. `maxBytes === undefined` means uncapped (local callers).
*/
export function limitSearchResultToContentBudget(
result: SearchResult,
maxBytes: number | undefined
): SearchResult {
if (maxBytes === undefined || !remoteRpcResultExceedsContentBudget(result, maxBytes)) {
return result
}
const files: SearchFileResult[] = []
// `{"files":[],"totalMatches":N,"truncated":true}` with room for any count.
let bytes = 64
for (const file of result.files) {
const fileBytes = Buffer.byteLength(JSON.stringify(file), 'utf8') + 1
if (bytes + fileBytes > maxBytes) {
break
}
files.push(file)
bytes += fileBytes
}
const totalMatches = files.reduce(
(sum, file) => sum + (file.matchCount ?? file.matches.length),
0
)
return { files, totalMatches, truncated: true }
}