From 9e5accee3c688e2a766f56c76d24b6b03a07b72d Mon Sep 17 00:00:00 2001 From: nikuscs Date: Wed, 29 Jul 2026 17:48:05 +0100 Subject: [PATCH] fix: reject oversized checkout diffs (#2488) * fix: reject oversized checkout diffs * fix(diff): keep large previews from disconnecting clients The physical socket limit is an OOM backstop, not a frame-size contract. Bound structured diff construction incrementally and report oversized previews without closing the client's socket. * fix(app): preserve oversized diff state * fix(diff): stay within relay frame limit --------- Co-authored-by: Mohamed Boudra --- packages/app/src/data/push-router.test.ts | 13 +- packages/app/src/data/push-router.ts | 6 + packages/app/src/git/diff-pane.tsx | 8 + packages/app/src/git/diff-too-large-state.tsx | 49 ++++ packages/app/src/git/use-diff-query.ts | 1 + packages/app/src/git/use-working-diff.ts | 2 + packages/app/src/i18n/resources/ar.ts | 2 + packages/app/src/i18n/resources/en.ts | 2 + packages/app/src/i18n/resources/es.ts | 2 + packages/app/src/i18n/resources/fr.ts | 2 + packages/app/src/i18n/resources/ja.ts | 2 + packages/app/src/i18n/resources/pt-BR.ts | 2 + packages/app/src/i18n/resources/ru.ts | 2 + packages/app/src/i18n/resources/zh-CN.ts | 2 + packages/app/src/panels/diff-panel.tsx | 4 + packages/client/src/daemon-client.ts | 1 + packages/protocol/src/messages.ts | 2 + packages/relay/src/encrypted-channel.ts | 14 +- packages/relay/src/index.ts | 8 +- .../src/server/checkout-diff-manager.ts | 8 + .../checkout-diff-subscription.e2e.test.ts | 31 +++ .../src/server/websocket/physical-socket.ts | 7 +- .../server/src/utils/checkout-git.test.ts | 15 ++ packages/server/src/utils/checkout-git.ts | 217 ++++++++++++------ 24 files changed, 321 insertions(+), 81 deletions(-) create mode 100644 packages/app/src/git/diff-too-large-state.tsx diff --git a/packages/app/src/data/push-router.test.ts b/packages/app/src/data/push-router.test.ts index 466ffabff..157d29c69 100644 --- a/packages/app/src/data/push-router.test.ts +++ b/packages/app/src/data/push-router.test.ts @@ -204,25 +204,34 @@ describe("server data push router", () => { fake.emit({ type: "subscribe_checkout_diff_response", - payload: { subscriptionId, cwd, files: [], error: null, requestId: "diff-1" }, + payload: { + subscriptionId, + cwd, + files: [], + error: null, + diffTooLarge: true, + requestId: "diff-1", + }, }); expect(queryClient.getQueryData(queryKey)).toEqual({ cwd, files: [], error: null, + diffTooLarge: true, requestId: "diff-1", }); fake.emit({ type: "checkout_diff_update", - payload: { subscriptionId, cwd, files: [], error: null }, + payload: { subscriptionId, cwd, files: [], error: null, diffTooLarge: true }, }); expect(queryClient.getQueryData(queryKey)).toEqual({ cwd, files: [], error: null, + diffTooLarge: true, requestId: `subscription:${subscriptionId}`, }); diff --git a/packages/app/src/data/push-router.ts b/packages/app/src/data/push-router.ts index b4dca8f11..aa7e55b1c 100644 --- a/packages/app/src/data/push-router.ts +++ b/packages/app/src/data/push-router.ts @@ -420,6 +420,9 @@ function applyCheckoutDiffUpdate(input: { cwd: input.message.payload.cwd, files: orderCheckoutDiffFiles(input.message.payload.files), error: input.message.payload.error, + ...(input.message.payload.diffTooLarge !== undefined + ? { diffTooLarge: input.message.payload.diffTooLarge } + : {}), requestId: `subscription:${input.message.payload.subscriptionId}`, }, }); @@ -440,6 +443,9 @@ function applyCheckoutDiffSubscribeResponse(input: { cwd: input.message.payload.cwd, files: orderCheckoutDiffFiles(input.message.payload.files), error: input.message.payload.error, + ...(input.message.payload.diffTooLarge !== undefined + ? { diffTooLarge: input.message.payload.diffTooLarge } + : {}), requestId: input.message.payload.requestId, }, }); diff --git a/packages/app/src/git/diff-pane.tsx b/packages/app/src/git/diff-pane.tsx index a6fc61861..9ddeeee43 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -119,6 +119,7 @@ import { type InlineReviewActions, } from "@/review"; import { usePublishWorkingDiffAttachment, useWorkingDiff } from "@/git/use-working-diff"; +import { DiffTooLargeState } from "@/git/diff-too-large-state"; export type { GitActionId, GitAction, GitActions } from "@/git/policy"; @@ -1755,6 +1756,7 @@ interface DiffBodyContentProps { notGit: boolean; isDiffLoading: boolean; diffErrorMessage: string | null; + diffTooLarge: boolean; hasChanges: boolean; emptyMessage: string; children: ReactElement; @@ -1768,6 +1770,7 @@ function DiffBodyContent({ notGit, isDiffLoading, diffErrorMessage, + diffTooLarge, hasChanges, emptyMessage, children, @@ -1803,6 +1806,9 @@ function DiffBodyContent({ ); } + if (diffTooLarge) { + return ; + } if (diffErrorMessage) { return ( @@ -2693,6 +2699,7 @@ export function GitDiffPane({ selectBase: handleSelectBase, files, diffPayloadError, + diffTooLarge, isDiffLoading, reviewActions, reviewAttachment, @@ -2855,6 +2862,7 @@ export function GitDiffPane({ notGit={notGit} isDiffLoading={isDiffLoading} diffErrorMessage={diffErrorMessage} + diffTooLarge={diffTooLarge} hasChanges={hasChanges} emptyMessage={emptyMessage} checkingRepositoryLabel={t("workspace.git.diff.checkingRepository")} diff --git a/packages/app/src/git/diff-too-large-state.tsx b/packages/app/src/git/diff-too-large-state.tsx new file mode 100644 index 000000000..d47812597 --- /dev/null +++ b/packages/app/src/git/diff-too-large-state.tsx @@ -0,0 +1,49 @@ +import { useTranslation } from "react-i18next"; +import { Text, View } from "react-native"; +import { FileWarning } from "lucide-react-native"; +import { StyleSheet, withUnistyles } from "react-native-unistyles"; +import { ICON_SIZE, type Theme } from "@/styles/theme"; + +const ThemedFileWarning = withUnistyles(FileWarning); +const foregroundMutedIconColorMapping = (theme: Theme) => ({ color: theme.colors.foregroundMuted }); + +export function DiffTooLargeState() { + const { t } = useTranslation(); + + return ( + + + + {t("workspace.git.diff.previewTooLargeTitle")} + {t("workspace.git.diff.previewTooLargeDescription")} + + + ); +} + +const styles = StyleSheet.create((theme) => ({ + container: { + flex: 1, + alignItems: "center", + justifyContent: "center", + gap: theme.spacing[3], + paddingHorizontal: theme.spacing[6], + paddingTop: theme.spacing[16], + }, + copy: { + alignItems: "center", + gap: theme.spacing[1], + maxWidth: 360, + }, + title: { + color: theme.colors.foreground, + fontSize: theme.fontSize.base, + fontWeight: theme.fontWeight.normal, + textAlign: "center", + }, + description: { + color: theme.colors.foregroundMuted, + fontSize: theme.fontSize.xs, + textAlign: "center", + }, +})); diff --git a/packages/app/src/git/use-diff-query.ts b/packages/app/src/git/use-diff-query.ts index ca3a0b5a8..93ea35c70 100644 --- a/packages/app/src/git/use-diff-query.ts +++ b/packages/app/src/git/use-diff-query.ts @@ -92,6 +92,7 @@ export function useCheckoutDiffQuery({ return { files: payload?.files ?? [], payloadError, + diffTooLarge: payload?.diffTooLarge === true, isLoading: payload === null && enabled && isConnected, isFetching: false, isError: Boolean(payloadError), diff --git a/packages/app/src/git/use-working-diff.ts b/packages/app/src/git/use-working-diff.ts index 55e8709bd..494a0f95a 100644 --- a/packages/app/src/git/use-working-diff.ts +++ b/packages/app/src/git/use-working-diff.ts @@ -79,6 +79,7 @@ export function useWorkingDiff({ const { files, payloadError: diffPayloadError, + diffTooLarge, isLoading: isDiffLoading, } = useCheckoutDiffQuery({ serverId, @@ -123,6 +124,7 @@ export function useWorkingDiff({ selectBase, files, diffPayloadError, + diffTooLarge, isDiffLoading, reviewActions, reviewAttachment, diff --git a/packages/app/src/i18n/resources/ar.ts b/packages/app/src/i18n/resources/ar.ts index e275c72dc..48d82e425 100644 --- a/packages/app/src/i18n/resources/ar.ts +++ b/packages/app/src/i18n/resources/ar.ts @@ -764,6 +764,8 @@ export const ar: TranslationResources = { closeChangesTab: "إغلاق علامة تبويب التغييرات", binaryFile: "ملف ثنائي", tooLarge: "الفرق كبير جدًا بحيث لا يمكن عرضه", + previewTooLargeTitle: "هذا الفرق كبير جدًا بحيث لا يمكن معاينته", + previewTooLargeDescription: "ضيّق نطاق المقارنة لمعاينتها هنا", unified: "الفرق الموحدة", split: "فرق جنبًا إلى جنب", switchToUnified: "التبديل إلى الفرق الموحد", diff --git a/packages/app/src/i18n/resources/en.ts b/packages/app/src/i18n/resources/en.ts index 8842cd9da..a7fd2d1aa 100644 --- a/packages/app/src/i18n/resources/en.ts +++ b/packages/app/src/i18n/resources/en.ts @@ -774,6 +774,8 @@ export const en = { closeChangesTab: "Close Changes tab", binaryFile: "Binary file", tooLarge: "Diff too large to display", + previewTooLargeTitle: "This diff is too large to preview", + previewTooLargeDescription: "Narrow the comparison to preview it here", unified: "Unified diff", split: "Side-by-side diff", switchToUnified: "Switch to unified diff", diff --git a/packages/app/src/i18n/resources/es.ts b/packages/app/src/i18n/resources/es.ts index 7dd54d9b9..1abd7cc05 100644 --- a/packages/app/src/i18n/resources/es.ts +++ b/packages/app/src/i18n/resources/es.ts @@ -795,6 +795,8 @@ export const es: TranslationResources = { closeChangesTab: "Cerrar la pestaña Cambios", binaryFile: "archivo binario", tooLarge: "La diferencia es demasiado grande para mostrarse", + previewTooLargeTitle: "Este diff es demasiado grande para previsualizarlo", + previewTooLargeDescription: "Reduce la comparación para previsualizarla aquí", unified: "Diferencia unificada", split: "Diferencia de lado a lado", switchToUnified: "Cambiar a diferencia unificada", diff --git a/packages/app/src/i18n/resources/fr.ts b/packages/app/src/i18n/resources/fr.ts index 821604b50..58dc392cf 100644 --- a/packages/app/src/i18n/resources/fr.ts +++ b/packages/app/src/i18n/resources/fr.ts @@ -794,6 +794,8 @@ export const fr: TranslationResources = { closeChangesTab: "Fermer l'onglet Modifications", binaryFile: "Fichier binaire", tooLarge: "Diff trop grand pour être affiché", + previewTooLargeTitle: "Ce diff est trop volumineux pour être prévisualisé", + previewTooLargeDescription: "Réduisez la comparaison pour la prévisualiser ici", unified: "Différentiel unifié", split: "Différent côte à côte", switchToUnified: "Passer au diff unifié", diff --git a/packages/app/src/i18n/resources/ja.ts b/packages/app/src/i18n/resources/ja.ts index 821e92416..0be4e6e98 100644 --- a/packages/app/src/i18n/resources/ja.ts +++ b/packages/app/src/i18n/resources/ja.ts @@ -775,6 +775,8 @@ export const ja: TranslationResources = { closeChangesTab: "変更タブを閉じる", binaryFile: "バイナリファイル", tooLarge: "差分が大きすぎて表示できません", + previewTooLargeTitle: "この差分は大きすぎるためプレビューできません", + previewTooLargeDescription: "比較範囲を狭めると、ここでプレビューできます", unified: "ユニファイド差分", split: "左右比較", switchToUnified: "ユニファイド差分に切り替え", diff --git a/packages/app/src/i18n/resources/pt-BR.ts b/packages/app/src/i18n/resources/pt-BR.ts index b32cb791b..51c9553fa 100644 --- a/packages/app/src/i18n/resources/pt-BR.ts +++ b/packages/app/src/i18n/resources/pt-BR.ts @@ -786,6 +786,8 @@ export const ptBR: TranslationResources = { closeChangesTab: "Fechar a aba Alterações", binaryFile: "Arquivo binário", tooLarge: "Diff grande demais para exibir", + previewTooLargeTitle: "Este diff é grande demais para visualizar", + previewTooLargeDescription: "Reduza a comparação para visualizá-la aqui", unified: "Diff unificado", split: "Diff lado a lado", switchToUnified: "Alternar para diff unificado", diff --git a/packages/app/src/i18n/resources/ru.ts b/packages/app/src/i18n/resources/ru.ts index f175927cf..d546a9218 100644 --- a/packages/app/src/i18n/resources/ru.ts +++ b/packages/app/src/i18n/resources/ru.ts @@ -786,6 +786,8 @@ export const ru: TranslationResources = { closeChangesTab: "Закрыть вкладку «Изменения»", binaryFile: "Бинарный файл", tooLarge: "Разница слишком велика для отображения", + previewTooLargeTitle: "Этот diff слишком велик для предпросмотра", + previewTooLargeDescription: "Сузьте сравнение, чтобы просмотреть его здесь", unified: "Единый дифференциал", split: "Параллельная разница", switchToUnified: "Переключить на объединенный diff", diff --git a/packages/app/src/i18n/resources/zh-CN.ts b/packages/app/src/i18n/resources/zh-CN.ts index 52579675c..eb22734bd 100644 --- a/packages/app/src/i18n/resources/zh-CN.ts +++ b/packages/app/src/i18n/resources/zh-CN.ts @@ -756,6 +756,8 @@ export const zhCN: TranslationResources = { closeChangesTab: "关闭“更改”标签页", binaryFile: "二进制文件", tooLarge: "Diff 过大，无法显示", + previewTooLargeTitle: "此 diff 过大，无法预览", + previewTooLargeDescription: "缩小比较范围即可在此预览", unified: "Unified diff", split: "Side-by-side diff", switchToUnified: "切换到统一差异视图", diff --git a/packages/app/src/panels/diff-panel.tsx b/packages/app/src/panels/diff-panel.tsx index df14c3cb9..f315dd5bc 100644 --- a/packages/app/src/panels/diff-panel.tsx +++ b/packages/app/src/panels/diff-panel.tsx @@ -17,6 +17,7 @@ import { resolveDiffLayout, SharedDiffView, } from "@/git/diff-pane"; +import { DiffTooLargeState } from "@/git/diff-too-large-state"; import { useCommitDiffFiles } from "@/git/use-diff-files"; import { usePublishWorkingDiffAttachment, useWorkingDiff } from "@/git/use-working-diff"; import { useChangesPreferences } from "@/hooks/use-changes-preferences"; @@ -120,6 +121,9 @@ function WorkingDiffBody({ if (workingDiff.notGit) { return ; } + if (workingDiff.diffTooLarge) { + return ; + } if (workingDiff.diffPayloadError) { return ( diff --git a/packages/client/src/daemon-client.ts b/packages/client/src/daemon-client.ts index 8e67ea4b5..ecfd68a47 100644 --- a/packages/client/src/daemon-client.ts +++ b/packages/client/src/daemon-client.ts @@ -3490,6 +3490,7 @@ export class DaemonClient { cwd: payload.cwd, files: payload.files, error: payload.error, + diffTooLarge: payload.diffTooLarge, requestId: payload.requestId, }; } finally { diff --git a/packages/protocol/src/messages.ts b/packages/protocol/src/messages.ts index c269321f6..7c1e2bc06 100644 --- a/packages/protocol/src/messages.ts +++ b/packages/protocol/src/messages.ts @@ -4128,6 +4128,8 @@ const CheckoutDiffSubscriptionPayloadSchema = z.object({ cwd: z.string(), files: z.array(ParsedDiffFileSchema), error: CheckoutErrorSchema.nullable(), + // COMPAT(diffTooLarge): added in v0.2.4, keep optional until the daemon floor is v0.2.4. + diffTooLarge: z.boolean().optional(), }); export const SubscribeCheckoutDiffResponseSchema = z.object({ diff --git a/packages/relay/src/encrypted-channel.ts b/packages/relay/src/encrypted-channel.ts index de953616a..0139cc13e 100644 --- a/packages/relay/src/encrypted-channel.ts +++ b/packages/relay/src/encrypted-channel.ts @@ -118,6 +118,15 @@ function buildInvalidHelloError(rawText: string, parsed?: unknown): Error { const HANDSHAKE_RETRY_MS = 1000; const MAX_PENDING_SENDS = 200; const REHANDSHAKE_KEY_MISMATCH_CLOSE_CODE = 1008; +const ENCRYPTED_PAYLOAD_OVERHEAD_BYTES = 40; + +export function base64EncryptedWireByteLength(plaintextBytes: number): number { + return 4 * Math.ceil((plaintextBytes + ENCRYPTED_PAYLOAD_OVERHEAD_BYTES) / 3); +} + +export function maxBase64EncryptedPlaintextByteLength(wireBytes: number): number { + return Math.floor(wireBytes / 4) * 3 - ENCRYPTED_PAYLOAD_OVERHEAD_BYTES; +} const REHANDSHAKE_KEY_MISMATCH_CLOSE_REASON = "E2EE re-handshake key mismatch"; interface TimeoutWithUnref { @@ -474,11 +483,12 @@ export class EncryptedChannel { } outboundWireByteLength(data: string | ArrayBuffer): number { - const encryptedBytes = utf8ByteLength(data) + 40; + const plaintextBytes = utf8ByteLength(data); + const encryptedBytes = plaintextBytes + ENCRYPTED_PAYLOAD_OVERHEAD_BYTES; if (this.options.binaryCiphertext && data instanceof ArrayBuffer) { return encryptedBytes; } - return 4 * Math.ceil(encryptedBytes / 3); + return base64EncryptedWireByteLength(plaintextBytes); } private async flushPendingSends(): Promise { diff --git a/packages/relay/src/index.ts b/packages/relay/src/index.ts index 79b575c71..b9f455e0f 100644 --- a/packages/relay/src/index.ts +++ b/packages/relay/src/index.ts @@ -9,5 +9,11 @@ export { decrypt, } from "./crypto.js"; -export { createClientChannel, createDaemonChannel, EncryptedChannel } from "./encrypted-channel.js"; +export { + base64EncryptedWireByteLength, + createClientChannel, + createDaemonChannel, + EncryptedChannel, + maxBase64EncryptedPlaintextByteLength, +} from "./encrypted-channel.js"; export type { Transport, EncryptedChannelEvents } from "./encrypted-channel.js"; diff --git a/packages/server/src/server/checkout-diff-manager.ts b/packages/server/src/server/checkout-diff-manager.ts index b4f90a62a..01e7a8284 100644 --- a/packages/server/src/server/checkout-diff-manager.ts +++ b/packages/server/src/server/checkout-diff-manager.ts @@ -208,6 +208,14 @@ export class CheckoutDiffManager { ? { force: true, reason: options.reason ?? "checkout-diff-refresh" } : undefined, ); + if (diffResult.diffTooLarge) { + return { + cwd, + files: [], + diffTooLarge: true, + error: toCheckoutError(new Error("Diff too large to display")), + }; + } const files = [...(diffResult.structured ?? [])]; files.sort((a, b) => { if (a.path === b.path) return 0; diff --git a/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts b/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts index 43a347901..7ce8cbb3a 100644 --- a/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/checkout-diff-subscription.e2e.test.ts @@ -138,3 +138,34 @@ test("pushes updates when subscribed from a subdirectory and files change outsid rmSync(cwd, { recursive: true, force: true }); } }, 60000); + +test("keeps the socket usable after rejecting an oversized structured diff", async () => { + const cwd = tmpCwd(); + + try { + initGitRepo(cwd); + commitFile(cwd, "large-a.js", "const value = 0;\n"); + commitFile(cwd, "large-b.js", "const value = 0;\n"); + const denseExpression = `const value = ${"a+".repeat(450_000)}a;\n`; + writeFileSync(path.join(cwd, "large-a.js"), denseExpression); + writeFileSync(path.join(cwd, "large-b.js"), denseExpression); + + const initial = await ctx.client.subscribeCheckoutDiff( + cwd, + { mode: "uncommitted" }, + { subscriptionId: "oversized-checkout-diff" }, + ); + + expect(initial).toMatchObject({ + cwd, + files: [], + diffTooLarge: true, + error: { code: "UNKNOWN" }, + }); + + const status = await ctx.client.getCheckoutStatus(cwd); + expect(status).toMatchObject({ cwd, isGit: true }); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } +}, 120000); diff --git a/packages/server/src/server/websocket/physical-socket.ts b/packages/server/src/server/websocket/physical-socket.ts index 491a7c612..abb0407e9 100644 --- a/packages/server/src/server/websocket/physical-socket.ts +++ b/packages/server/src/server/websocket/physical-socket.ts @@ -1,6 +1,7 @@ -// Terminal streams begin snapshot catch-up at 4 MiB. The physical socket gets -// another 4 MiB to recover before the daemon enforces the hard memory bound. -export const MAX_PHYSICAL_SOCKET_BUFFERED_BYTES = 8 * 1024 * 1024; +// OOM backstop for a socket whose client stopped draining. A daemon normally has +// 1-10 physical sockets (tens at the outside), so 64 MiB bounds abandoned queues +// without treating ordinary large frames as a protocol or frame-size violation. +export const MAX_PHYSICAL_SOCKET_BUFFERED_BYTES = 64 * 1024 * 1024; // Current clients ping every 10 seconds. Four delayed cycles fit inside the // lease without making an abandoned application socket linger for minutes. export const APPLICATION_SOCKET_LEASE_MS = 45_000; diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index 2720b0d0a..110d3599f 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -14,11 +14,13 @@ import { join } from "path"; import { win32 } from "node:path"; import { tmpdir } from "os"; import pino from "pino"; +import { base64EncryptedWireByteLength } from "@getpaseo/relay"; import { __resetCheckoutShortstatCacheForTests, __resetPullRequestStatusCacheForTests, __setPullRequestStatusCacheTtlForTests, commitAll, + CHECKOUT_DIFF_MAX_STRUCTURED_BYTES, createPullRequest, getCachedCheckoutShortstat, getCheckoutSnapshotFacts, @@ -1333,6 +1335,19 @@ const x = 1; ); }); + it("keeps the structured diff cap below the relay frame limit", () => { + const relayFrameBytes = 32 * 1024 * 1024; + const frameEnvelopeHeadroomBytes = 1024 * 1024; + const diffWireBytes = base64EncryptedWireByteLength(CHECKOUT_DIFF_MAX_STRUCTURED_BYTES); + const maximumFrameWireBytes = base64EncryptedWireByteLength( + CHECKOUT_DIFF_MAX_STRUCTURED_BYTES + frameEnvelopeHeadroomBytes, + ); + + expect(CHECKOUT_DIFF_MAX_STRUCTURED_BYTES).toBe(24_117_208); + expect(diffWireBytes).toBeLessThan(relayFrameBytes); + expect(maximumFrameWireBytes).toBe(relayFrameBytes); + }); + it("marks tracked generated one-line diffs as too_large by content size", async () => { writeFileSync(join(repoDir, "generated.js"), `const data = "old";\n`); execFileSync("git", ["add", "generated.js"], { cwd: repoDir }); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 6d09eee0d..d2e7ae29c 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -3,9 +3,14 @@ import { existsSync, realpathSync } from "fs"; import { open as openFile, readFile, stat as statFile } from "fs/promises"; import { TTLCache } from "@isaacs/ttlcache"; import type { CheckoutCommit, CheckoutCommitFile } from "@getpaseo/protocol/messages"; +import { maxBase64EncryptedPlaintextByteLength } from "@getpaseo/relay"; import type { Logger } from "pino"; import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js"; -import { parseAndHighlightDiff } from "../server/utils/diff-highlighter.js"; +import { + highlightDiffWithFileContent, + parseAndHighlightDiff, + parseDiff, +} from "../server/utils/diff-highlighter.js"; import { parseGitHubRepoFromRemote } from "../server/workspace-git-metadata.js"; import { createGitHubService } from "../services/github-service.js"; import type { @@ -583,6 +588,7 @@ function buildGitDiffArgs(args: { ignoreWhitespace?: boolean; extra: string[] }) } const TRACKED_DIFF_NUMSTAT_MAX_BYTES = 2 * 1024 * 1024; // 2MB +const TRACKED_DIFF_BATCH_SIZE = 8; const EMPTY_TREE_OBJECT_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; function isUnbornHeadDiffError(error: unknown): boolean { @@ -759,10 +765,9 @@ export type CheckoutStatusGit = CheckoutStatusGitNonPaseo | CheckoutStatusGitPas export type CheckoutStatusResult = CheckoutStatus | CheckoutStatusGit; -export interface CheckoutDiffResult { - diff: string; - structured?: ParsedDiffFile[]; -} +export type CheckoutDiffResult = + | { diff: string; structured?: ParsedDiffFile[]; diffTooLarge?: false } + | { diff: ""; structured: []; diffTooLarge: true }; export interface CheckoutDiffCompare { mode: "uncommitted" | "base"; @@ -1823,6 +1828,37 @@ export async function getCheckoutSnapshotFacts( const PER_FILE_DIFF_MAX_BYTES = 1024 * 1024; // 1MB const TOTAL_DIFF_MAX_BYTES = 2 * 1024 * 1024; // 2MB +const RELAY_MAX_FRAME_BYTES = 32 * 1024 * 1024; +const CHECKOUT_DIFF_FRAME_HEADROOM_BYTES = 1024 * 1024; +// Temporary until diffs load lazily per file. The Paseo relay's 32 MiB frame limit is +// binding: string frames are encrypted and base64-encoded. Reserve 1 MiB plaintext for +// the surrounding WebSocket JSON envelope after inverting that exact wire expansion. +export const CHECKOUT_DIFF_MAX_STRUCTURED_BYTES = + maxBase64EncryptedPlaintextByteLength(RELAY_MAX_FRAME_BYTES) - CHECKOUT_DIFF_FRAME_HEADROOM_BYTES; + +interface StructuredDiffAccumulator { + files: ParsedDiffFile[]; + serializedBytes: number; +} + +function createStructuredDiffAccumulator(): StructuredDiffAccumulator { + return { files: [], serializedBytes: Buffer.byteLength("[]", "utf8") }; +} + +function appendStructuredFile( + structured: StructuredDiffAccumulator, + file: ParsedDiffFile, +): boolean { + const separatorBytes = structured.files.length > 0 ? 1 : 0; + const fileBytes = Buffer.byteLength(JSON.stringify(file), "utf8"); + const nextBytes = structured.serializedBytes + separatorBytes + fileBytes; + if (nextBytes > CHECKOUT_DIFF_MAX_STRUCTURED_BYTES) { + return false; + } + structured.files.push(file); + structured.serializedBytes = nextBytes; + return true; +} const UNTRACKED_BINARY_SNIFF_BYTES = 16 * 1024; async function isLikelyBinaryFile(absolutePath: string): Promise { @@ -2543,27 +2579,62 @@ export function warmCheckoutShortstatInBackground( interface AppendStructuredTrackedDiffsInput { cwd: string; trackedChanges: CheckoutFileChange[]; - trackedChangeByPath: Map; trackedNumstatByPath: Map; trackedPlaceholderByPath: Map; trackedDiffText: string; refsForDiff: CheckoutDiffRefs; ignoreWhitespace: boolean; - structured: ParsedDiffFile[]; - appendDiff: (text: string) => void; + structured: StructuredDiffAccumulator; appendTrackedPlaceholderComment: ( change: CheckoutFileChange, status: "binary" | "too_large", ) => void; } +async function buildHighlightedTrackedDiffFile(input: { + cwd: string; + change: CheckoutFileChange; + parsedFile: ParsedDiffFile; + refsForDiff: CheckoutDiffRefs; +}): Promise { + const { cwd, change, parsedFile, refsForDiff } = input; + const refPath = change.oldPath ?? change.path; + const [oldFileContent, newFileContent] = await Promise.all([ + change.isNew ? null : readGitFileContentAtRef(cwd, refsForDiff.baseRef, refPath), + refsForDiff.targetRef ? readGitFileContentAtRef(cwd, refsForDiff.targetRef, change.path) : null, + ]); + const highlightedFile = await highlightDiffWithFileContent(parsedFile, cwd, { + oldFileContent, + newFileContent, + }); + return { + ...highlightedFile, + path: change.path, + isNew: change.isNew, + isDeleted: change.isDeleted, + status: "ok", + }; +} + +function isWhitespaceOnlyTrackedChange(input: { + change: CheckoutFileChange; + stat: FileStat; + ignoreWhitespace: boolean; +}): boolean { + const { change, stat, ignoreWhitespace } = input; + return ( + ignoreWhitespace && + change.status.startsWith("M") && + (!stat || (!stat.isBinary && stat.additions === 0 && stat.deletions === 0)) + ); +} + async function appendStructuredTrackedDiffs( input: AppendStructuredTrackedDiffsInput, -): Promise { +): Promise { const { cwd, trackedChanges, - trackedChangeByPath, trackedNumstatByPath, trackedPlaceholderByPath, trackedDiffText, @@ -2573,36 +2644,19 @@ async function appendStructuredTrackedDiffs( appendTrackedPlaceholderComment, } = input; - const parsedTrackedFiles = - trackedDiffText.length > 0 - ? await parseAndHighlightDiff(trackedDiffText, cwd, { - getOldFileContent: async (file) => { - const change = trackedChangeByPath.get(file.path); - if (!change || change.isNew) { - return null; - } - const refPath = change.oldPath ?? change.path; - return readGitFileContentAtRef(cwd, refsForDiff.baseRef, refPath); - }, - getNewFileContent: async (file) => { - if (!refsForDiff.targetRef) { - return null; - } - return readGitFileContentAtRef(cwd, refsForDiff.targetRef, file.path); - }, - }) - : []; + const parsedTrackedFiles = trackedDiffText.length > 0 ? parseDiff(trackedDiffText) : []; const parsedTrackedByPath = new Map(parsedTrackedFiles.map((file) => [file.path, file])); for (const change of trackedChanges) { const placeholder = trackedPlaceholderByPath.get(change.path); if (placeholder) { - structured.push( - buildPlaceholderParsedDiffFile(change, { - status: placeholder.status, - stat: placeholder.stat, - }), - ); + const file = buildPlaceholderParsedDiffFile(change, { + status: placeholder.status, + stat: placeholder.stat, + }); + if (!appendStructuredFile(structured, file)) { + return false; + } appendTrackedPlaceholderComment(change, placeholder.status); continue; } @@ -2610,28 +2664,26 @@ async function appendStructuredTrackedDiffs( const stat = trackedNumstatByPath.get(change.path) ?? null; const parsedFile = parsedTrackedByPath.get(change.path); if (parsedFile) { - structured.push({ - ...parsedFile, - path: change.path, - isNew: change.isNew, - isDeleted: change.isDeleted, - status: "ok", + const file = await buildHighlightedTrackedDiffFile({ + cwd, + change, + parsedFile, + refsForDiff, }); + if (!appendStructuredFile(structured, file)) { + return false; + } continue; } // `git diff -w --name-status` can still report a modified path even when the // whitespace-filtered patch and numstat are both empty. Skip emitting a // structured placeholder in that case so whitespace-only edits truly disappear. - if ( - ignoreWhitespace && - change.status.startsWith("M") && - (!stat || (!stat.isBinary && stat.additions === 0 && stat.deletions === 0)) - ) { + if (isWhitespaceOnlyTrackedChange({ change, stat, ignoreWhitespace })) { continue; } - structured.push({ + const file = { path: change.path, isNew: change.isNew, isDeleted: change.isDeleted, @@ -2639,8 +2691,13 @@ async function appendStructuredTrackedDiffs( deletions: stat?.deletions ?? 0, hunks: [], status: "ok", - }); + } satisfies ParsedDiffFile; + if (!appendStructuredFile(structured, file)) { + return false; + } } + + return true; } interface ProcessUntrackedChangeInput { @@ -2648,11 +2705,11 @@ interface ProcessUntrackedChangeInput { change: CheckoutFileChange; ignoreWhitespace: boolean; includeStructured: boolean; - structured: ParsedDiffFile[]; + structured: StructuredDiffAccumulator; appendDiff: (text: string) => void; } -async function processUntrackedChange(input: ProcessUntrackedChangeInput): Promise { +async function processUntrackedChange(input: ProcessUntrackedChangeInput): Promise { const { cwd, change, ignoreWhitespace, includeStructured, structured, appendDiff } = input; const { text, truncated, stat } = await getUntrackedDiffText(cwd, change, ignoreWhitespace); @@ -2664,19 +2721,33 @@ async function processUntrackedChange(input: ProcessUntrackedChangeInput): Promi } else { appendDiff(text); } - return; + return true; } if (stat?.isBinary) { - structured.push(buildPlaceholderParsedDiffFile(change, { status: "binary", stat })); + if ( + !appendStructuredFile( + structured, + buildPlaceholderParsedDiffFile(change, { status: "binary", stat }), + ) + ) { + return false; + } appendDiff(`# ${change.path}: binary diff omitted\n`); - return; + return true; } if (truncated) { - structured.push(buildPlaceholderParsedDiffFile(change, { status: "too_large", stat })); + if ( + !appendStructuredFile( + structured, + buildPlaceholderParsedDiffFile(change, { status: "too_large", stat }), + ) + ) { + return false; + } appendDiff(`# ${change.path}: diff too large omitted\n`); - return; + return true; } appendDiff(text); @@ -2692,13 +2763,14 @@ async function processUntrackedChange(input: ProcessUntrackedChangeInput): Promi hunks: [], } satisfies ParsedDiffFile); - structured.push({ + const file = { ...parsedFile, path: change.path, isNew: change.isNew, isDeleted: change.isDeleted, status: "ok", - }); + } satisfies ParsedDiffFile; + return appendStructuredFile(structured, file); } interface ProcessTrackedChangesInput { @@ -2710,7 +2782,6 @@ interface ProcessTrackedChangesInput { } interface ProcessTrackedChangesResult { - trackedChangeByPath: Map; trackedNumstatByPath: Map; trackedPlaceholderByPath: Map; trackedDiffText: string; @@ -2720,7 +2791,6 @@ async function processTrackedChanges( input: ProcessTrackedChangesInput, ): Promise { const { cwd, refsForDiff, trackedChanges, ignoreWhitespace, appendDiff } = input; - const trackedChangeByPath = new Map(trackedChanges.map((change) => [change.path, change])); const trackedNumstatByPath = trackedChanges.length > 0 ? await getTrackedNumstatByPath(cwd, refsForDiff, ignoreWhitespace) @@ -2742,9 +2812,10 @@ async function processTrackedChanges( let trackedDiffText = ""; let trackedDiffBytes = 0; - if (trackedDiffPaths.length > 0) { + for (let start = 0; start < trackedDiffPaths.length; start += TRACKED_DIFF_BATCH_SIZE) { + const paths = trackedDiffPaths.slice(start, start + TRACKED_DIFF_BATCH_SIZE); const trackedDiffs = await Promise.all( - trackedDiffPaths.map((path) => + paths.map((path) => getTrackedDiffTextForPath({ cwd, refsForDiff, @@ -2754,7 +2825,6 @@ async function processTrackedChanges( ), ); - const visibleTrackedDiffs: string[] = []; for (const fileDiff of trackedDiffs) { if (fileDiff.truncated) { trackedPlaceholderByPath.set(fileDiff.path, { @@ -2772,15 +2842,12 @@ async function processTrackedChanges( continue; } trackedDiffBytes += diffBytes; - visibleTrackedDiffs.push(fileDiff.text); + trackedDiffText += fileDiff.text; } - - trackedDiffText = visibleTrackedDiffs.join(""); - appendDiff(trackedDiffText); } + appendDiff(trackedDiffText); return { - trackedChangeByPath, trackedNumstatByPath, trackedPlaceholderByPath, trackedDiffText, @@ -2840,7 +2907,7 @@ export async function getCheckoutDiff( return a.path < b.path ? -1 : 1; }); - const structured: ParsedDiffFile[] = []; + const structured = createStructuredDiffAccumulator(); let diffText = ""; let diffBytes = 0; const appendDiff = (text: string) => { @@ -2881,19 +2948,20 @@ export async function getCheckoutDiff( }; if (compare.includeStructured) { - await appendStructuredTrackedDiffs({ + const didAppendTrackedDiffs = await appendStructuredTrackedDiffs({ cwd, trackedChanges, - trackedChangeByPath: trackedDiff.trackedChangeByPath, trackedNumstatByPath: trackedDiff.trackedNumstatByPath, trackedPlaceholderByPath: trackedDiff.trackedPlaceholderByPath, trackedDiffText: trackedDiff.trackedDiffText, refsForDiff: effectiveRefsForDiff, ignoreWhitespace, structured, - appendDiff, appendTrackedPlaceholderComment, }); + if (!didAppendTrackedDiffs) { + return { diff: "", structured: [], diffTooLarge: true }; + } } else { for (const change of trackedChanges) { const placeholder = trackedDiff.trackedPlaceholderByPath.get(change.path); @@ -2907,7 +2975,7 @@ export async function getCheckoutDiff( if (diffBytes >= TOTAL_DIFF_MAX_BYTES) { break; } - await processUntrackedChange({ + const didAppendUntrackedDiff = await processUntrackedChange({ cwd, change, ignoreWhitespace, @@ -2915,10 +2983,13 @@ export async function getCheckoutDiff( structured, appendDiff, }); + if (!didAppendUntrackedDiff) { + return { diff: "", structured: [], diffTooLarge: true }; + } } if (compare.includeStructured) { - return { diff: diffText, structured }; + return { diff: diffText, structured: structured.files }; } return { diff: diffText }; }