Skip to content

Commit 87ae8d5

Browse files
saphidcodexjuliusmarminge
authored
fix(server): cap checkpoint diffs across all batches (#18395)
Co-authored-by: SAPHID <saphid@users.noreply.github.com> Co-authored-by: GPT-6.1 Sol <noreply@openai.com> Co-authored-by: Julius Marminge <julius0216@outlook.com>
1 parent 51cb848 commit 87ae8d5

5 files changed

Lines changed: 200 additions & 17 deletions

File tree

‎apps/server/src/checkpointing/CheckpointDiffQuery.test.ts‎

Lines changed: 160 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
import * as NodeCrypto from "@effect/platform-node/NodeCrypto";
2+
import * as NodeServices from "@effect/platform-node/NodeServices";
23
import { assert, it, vi } from "@effect/vitest";
34
import { CheckpointRef, CheckpointScopeId, RunId, ThreadId } from "@t3tools/contracts";
45
import * as Effect from "effect/Effect";
6+
import * as FileSystem from "effect/FileSystem";
57
import * as Layer from "effect/Layer";
8+
import * as Path from "effect/Path";
69

710
import { checkpointRefForScopeOrdinal } from "../orchestration-v2/CheckpointService.ts";
811
import { OrchestratorProjectionError } from "../orchestration-v2/Orchestrator.ts";
912
import type { ProjectionCheckpointContext } from "../orchestration-v2/ProjectionStore.ts";
1013
import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts";
14+
import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts";
15+
import * as VcsProcess from "../vcs/VcsProcess.ts";
1116
import * as CheckpointDiffQuery from "./CheckpointDiffQuery.ts";
1217
import * as CheckpointStore from "./CheckpointStore.ts";
1318
import {
@@ -83,7 +88,7 @@ it.effect("computes V2 run diffs from projected checkpoint scopes", () => {
8388
toTurnCount: 2,
8489
diff: "diff --git a/file b/file",
8590
});
86-
assert.deepEqual(diffCheckpoints.mock.calls[0]?.[0], {
91+
assert.containSubset(diffCheckpoints.mock.calls[0]?.[0], {
8792
cwd: "/repo",
8893
fromCheckpointRef: yield* checkpointRefForScopeOrdinal({
8994
scopeId: firstScopeId,
@@ -310,3 +315,157 @@ it.effect("preserves the typed missing-baseline-ref error contract", () => {
310315
);
311316
}).pipe(Effect.provide(layer));
312317
});
318+
319+
const layerVcsProcess = VcsProcess.layer.pipe(Layer.provide(NodeServices.layer));
320+
const layerStore = CheckpointStore.layer.pipe(
321+
Layer.provide(VcsDriverRegistry.layer.pipe(Layer.provide(layerVcsProcess))),
322+
Layer.provide(NodeServices.layer),
323+
);
324+
const layerTest = Layer.merge(layerStore, layerVcsProcess).pipe(
325+
Layer.provideMerge(NodeServices.layer),
326+
Layer.provideMerge(NodeCrypto.layer),
327+
);
328+
329+
it.effect("shares the turn and full-thread diff budget across real Git batches", () =>
330+
Effect.gen(function* () {
331+
const fs = yield* FileSystem.FileSystem;
332+
const path = yield* Path.Path;
333+
const process = yield* VcsProcess.VcsProcess;
334+
const store = yield* CheckpointStore.CheckpointStore;
335+
const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-diff-aggregate-" });
336+
const git = (args: ReadonlyArray<string>, env?: NodeJS.ProcessEnv) =>
337+
process.run({
338+
operation: "CheckpointDiffQuery.test.git",
339+
command: "git",
340+
cwd,
341+
args,
342+
...(env === undefined ? {} : { env }),
343+
});
344+
yield* git(["init"]);
345+
yield* git(["config", "user.email", "test@test.com"]);
346+
yield* git(["config", "user.name", "Test"]);
347+
yield* fs.writeFileString(path.join(cwd, "README.md"), "initial\n");
348+
yield* git(["add", "."]);
349+
yield* git(["commit", "-m", "initial"]);
350+
351+
const threadId = ThreadId.make("thread:aggregate-runtime");
352+
const runId = RunId.make("run:aggregate-runtime");
353+
const scopeId = CheckpointScopeId.make("scope:aggregate-runtime");
354+
const fromCheckpointRef = yield* checkpointRefForScopeOrdinal({
355+
scopeId,
356+
ordinalWithinScope: 0,
357+
});
358+
const toCheckpointRef = CheckpointRef.make("refs/t3/test/aggregate-after");
359+
yield* store.captureCheckpoint({ cwd, checkpointRef: fromCheckpointRef });
360+
// The old commit models files imported by a pull after the baseline was captured.
361+
yield* fs.writeFileString(path.join(cwd, "upstream.ts"), "imported\n");
362+
yield* git(["add", "."]);
363+
yield* git(["commit", "-m", "imported"], {
364+
...globalThis.process.env,
365+
GIT_COMMITTER_DATE: "2000-01-01T00:00:00Z",
366+
});
367+
const ownPaths = Array.from({ length: 1_600 }, (_, index) => `src/module-${index}/file.ts`);
368+
yield* Effect.forEach(
369+
ownPaths,
370+
(file) =>
371+
Effect.gen(function* () {
372+
yield* fs.makeDirectory(path.dirname(path.join(cwd, file)), { recursive: true });
373+
yield* fs.writeFileString(path.join(cwd, file), `${"x".repeat(12_000)}\n`);
374+
}),
375+
{ concurrency: 16, discard: true },
376+
);
377+
yield* store.captureCheckpoint({ cwd, checkpointRef: toCheckpointRef });
378+
379+
const patchSizes: number[] = [];
380+
const query = yield* CheckpointDiffQuery.make.pipe(
381+
Effect.provide(
382+
Layer.mock(ThreadManagement.ThreadManagementService)({
383+
getCheckpointContext: () =>
384+
Effect.succeed({
385+
runs: [{ id: runId, ordinal: 1, status: "completed" }],
386+
checkpointScopes: [{ id: scopeId, runId, kind: "root_run", cwd }],
387+
checkpoints: [
388+
{
389+
scopeId,
390+
runId,
391+
ordinalWithinScope: 1,
392+
appRunOrdinal: 1,
393+
status: "ready",
394+
ref: toCheckpointRef,
395+
},
396+
],
397+
}),
398+
}),
399+
),
400+
Effect.provideService(CheckpointStore.CheckpointStore, {
401+
...store,
402+
diffCheckpoints: (input) =>
403+
store.diffCheckpoints(input).pipe(
404+
Effect.tap((patch) =>
405+
Effect.sync(() => {
406+
if (input.format !== "numstat") {
407+
patchSizes.push(Buffer.byteLength(patch));
408+
}
409+
}),
410+
),
411+
),
412+
}),
413+
);
414+
const turn = yield* query.getTurnDiff({ threadId, fromTurnCount: 0, toTurnCount: 1 });
415+
const turnPatchSizes = patchSizes.splice(0);
416+
const fullThread = yield* query.getFullThreadDiff({ threadId, toTurnCount: 1 });
417+
assert.isTrue(fullThread.diff === turn.diff);
418+
assert.isAtMost(Buffer.byteLength(turn.diff), 10_000_000);
419+
assert.isTrue(turn.diff.endsWith("\n\n[truncated]"));
420+
assert.include(turn.diff, "src/module-0/file.ts");
421+
assert.notInclude(turn.diff, "upstream.ts");
422+
assert.lengthOf(turnPatchSizes, 2);
423+
assert.deepEqual(patchSizes, turnPatchSizes);
424+
assert.isBelow(turnPatchSizes[0]!, 10_000_000);
425+
assert.strictEqual(
426+
turnPatchSizes.reduce((sum, size) => sum + size, 0),
427+
10_000_001,
428+
);
429+
}).pipe(Effect.provide(layerTest), Effect.scoped),
430+
);
431+
432+
it.effect("truncates a storage-cut UTF-8 character without replacement text", () => {
433+
const prefix = "x".repeat(9_999_985);
434+
const layer = layerFor({
435+
projection: Effect.succeed(makeProjection()),
436+
diffCheckpoints: (input) =>
437+
Effect.succeed(
438+
Buffer.from(`${prefix}😀${"x".repeat(9)}😀tail`)
439+
.subarray(0, input.maxOutputBytes)
440+
.toString("utf8"),
441+
),
442+
});
443+
return Effect.gen(function* () {
444+
const query = yield* CheckpointDiffQuery.CheckpointDiffQuery;
445+
const result = yield* query.getTurnDiff({ threadId, fromTurnCount: 0, toTurnCount: 2 });
446+
assert.isAtMost(Buffer.byteLength(result.diff), 10_000_000);
447+
assert.isTrue(result.diff === `${prefix}\n\n[truncated]`);
448+
}).pipe(Effect.provide(layer));
449+
});
450+
451+
it.effect("preserves an exact-fit filtered patch across batches without a marker", () => {
452+
const ownPaths = Array.from({ length: 1_000 }, (_, index) => `src/module-${index}/file.ts`);
453+
const patches = ["é".repeat(3_000_000), "x".repeat(4_000_000)];
454+
let patchReads = 0;
455+
const layer = layerFor({
456+
projection: Effect.succeed(makeProjection()),
457+
authoredPaths: new Set(ownPaths),
458+
diffCheckpoints: (input) =>
459+
Effect.sync(() =>
460+
input.format === "numstat"
461+
? [...ownPaths, "upstream.ts"].map((path) => `1\t0\t${path}\0`).join("")
462+
: patches[patchReads++]!,
463+
),
464+
});
465+
return Effect.gen(function* () {
466+
const query = yield* CheckpointDiffQuery.CheckpointDiffQuery;
467+
const result = yield* query.getFullThreadDiff({ threadId, toTurnCount: 2 });
468+
assert.isTrue(result.diff === patches.join(""));
469+
assert.strictEqual(patchReads, 2);
470+
}).pipe(Effect.provide(layer));
471+
});

‎apps/server/src/checkpointing/CheckpointDiffQuery.ts‎

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as Effect from "effect/Effect";
1212
import * as Layer from "effect/Layer";
1313
import * as Schema from "effect/Schema";
1414

15+
import { CHECKPOINT_DIFF_MAX_OUTPUT_BYTES } from "../vcs/VcsDriver.ts";
1516
import { checkpointRefForScopeOrdinal } from "../orchestration-v2/CheckpointService.ts";
1617
import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts";
1718
import {
@@ -27,6 +28,7 @@ import { isGitImport, parseTurnDiffFilesFromNumstat, type TurnDiffFileSummary }
2728

2829
// Windows limits a command line to 32,767 characters. Leave room for the rest of git's arguments.
2930
const MAX_PATHSPEC_CHARS = 24_000;
31+
const DIFF_TRUNCATED_MARKER = "\n\n[truncated]";
3032

3133
function batchFilePaths(files: ReadonlyArray<TurnDiffFileSummary>) {
3234
const batches: string[][] = [];
@@ -219,19 +221,36 @@ export const make = Effect.gen(function* () {
219221
);
220222
const retainedFiles = files.filter((file) => !isGitImport(file, authoredPaths));
221223
// Select retained paths before generating a patch, so imported bulk cannot exhaust its output limit.
222-
const diff =
223-
retainedFiles.length === files.length
224-
? yield* checkpointStore
225-
.diffCheckpoints(comparison)
226-
.pipe(Effect.withSpan("checkpoint.turnDiff.diffCheckpoints"))
227-
: (yield* Effect.forEach(
228-
batchFilePaths(retainedFiles),
229-
(filePaths) =>
230-
checkpointStore
231-
.diffCheckpoints({ ...comparison, filePaths })
232-
.pipe(Effect.withSpan("checkpoint.turnDiff.diffCheckpoints")),
233-
{ concurrency: 4 },
234-
)).join("");
224+
const pathBatches =
225+
retainedFiles.length === files.length ? [undefined] : batchFilePaths(retainedFiles);
226+
let diff = "";
227+
let remainingBytes = CHECKPOINT_DIFF_MAX_OUTPUT_BYTES;
228+
for (const filePaths of pathBatches) {
229+
const patch = yield* checkpointStore
230+
.diffCheckpoints({
231+
...comparison,
232+
...(filePaths === undefined ? {} : { filePaths }),
233+
// One extra byte distinguishes an exact fit from a truncated patch.
234+
maxOutputBytes: remainingBytes + 1,
235+
})
236+
.pipe(Effect.withSpan("checkpoint.turnDiff.diffCheckpoints"));
237+
const patchBytes = Buffer.byteLength(patch);
238+
diff += patch;
239+
if (patchBytes > remainingBytes) {
240+
// Reserve marker space only on overflow, which may trim an earlier batch.
241+
// Streaming decode drops an incomplete final UTF-8 character at the byte boundary.
242+
diff =
243+
new TextDecoder().decode(
244+
Buffer.from(diff).subarray(
245+
0,
246+
CHECKPOINT_DIFF_MAX_OUTPUT_BYTES - Buffer.byteLength(DIFF_TRUNCATED_MARKER),
247+
),
248+
{ stream: true },
249+
) + DIFF_TRUNCATED_MARKER;
250+
break;
251+
}
252+
remainingBytes -= patchBytes;
253+
}
235254

236255
const turnDiff = buildTurnDiffResult(input, diff);
237256
if (!isTurnDiffResult(turnDiff)) {

‎apps/server/src/checkpointing/CheckpointStore.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ export interface DiffCheckpointsInput {
2525
readonly fallbackFromToHead?: boolean;
2626
readonly ignoreWhitespace: boolean;
2727
readonly format?: "patch" | "numstat";
28+
/** Maximum bytes collected from the diff command. */
29+
readonly maxOutputBytes?: number;
2830
/** Limits the diff to these exact paths. An empty list yields an empty diff. */
2931
readonly filePaths?: ReadonlyArray<string>;
3032
}

‎apps/server/src/vcs/GitVcsDriver.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,6 @@ const WORKSPACE_FILES_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
422422
const CHECKPOINT_RECOVERY_MAX_CANDIDATES = 64;
423423
const CHECKPOINT_RECOVERY_TIMEOUT = "5 seconds";
424424
const GIT_CHECK_IGNORE_MAX_STDIN_BYTES = 256 * 1024;
425-
const CHECKPOINT_DIFF_MAX_OUTPUT_BYTES = 10_000_000;
426425
const WORKSPACE_GIT_HARDENED_CONFIG_ARGS = [
427426
"-c",
428427
"core.fsmonitor=false",
@@ -1298,7 +1297,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
12981297
: []),
12991298
],
13001299
allowNonZeroExit: true,
1301-
maxOutputBytes: CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
1300+
maxOutputBytes: input.maxOutputBytes ?? VcsDriver.CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
13021301
outputMode: input.format === "numstat" ? "error" : "truncate",
13031302
});
13041303

@@ -1340,7 +1339,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
13401339
operation,
13411340
cwd: input.cwd,
13421341
args: [...args, "--name-only", "-z", "--no-renames", "--no-ext-diff"],
1343-
maxOutputBytes: CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
1342+
maxOutputBytes: VcsDriver.CHECKPOINT_DIFF_MAX_OUTPUT_BYTES,
13441343
outputMode: "error",
13451344
}).pipe(Effect.map((result) => result.stdout.split("\0")));
13461345
// Remerge diff lists a merge's paths only where the result differs from Git's

‎apps/server/src/vcs/VcsDriver.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,17 @@ export interface VcsRestoreCheckpointInput {
2525
readonly fallbackToHead?: boolean;
2626
}
2727

28+
export const CHECKPOINT_DIFF_MAX_OUTPUT_BYTES = 10_000_000;
29+
2830
export interface VcsDiffCheckpointsInput {
2931
readonly cwd: string;
3032
readonly fromCheckpointRef: CheckpointRef;
3133
readonly toCheckpointRef: CheckpointRef;
3234
readonly fallbackFromToHead?: boolean;
3335
readonly ignoreWhitespace: boolean;
3436
readonly format?: "patch" | "numstat";
37+
/** Maximum bytes collected from the diff command. */
38+
readonly maxOutputBytes?: number;
3539
/** An empty list yields an empty diff. */
3640
readonly filePaths?: ReadonlyArray<string>;
3741
}

0 commit comments

Comments
 (0)