From 96e43690ee4baff0fb0b72a94d67f0a058f28051 Mon Sep 17 00:00:00 2001 From: jiancheng <1729303158@qq.com> Date: Thu, 25 Jun 2026 19:59:35 +0800 Subject: [PATCH 1/2] fix(session): bound diff summary payload --- packages/opencode/src/session/summary-diffs.ts | 7 +++++++ packages/opencode/src/session/summary.ts | 5 +++-- .../test/session/summary-diffs.test.ts | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 packages/opencode/src/session/summary-diffs.ts create mode 100644 packages/opencode/test/session/summary-diffs.test.ts diff --git a/packages/opencode/src/session/summary-diffs.ts b/packages/opencode/src/session/summary-diffs.ts new file mode 100644 index 000000000000..4617c8f9c3d0 --- /dev/null +++ b/packages/opencode/src/session/summary-diffs.ts @@ -0,0 +1,7 @@ +const MAX_SUMMARY_DIFFS = 200 + +export function limitSummaryDiffs(diffs: T[]) { + // Keep turn summaries bounded so a pathological workspace diff cannot bloat message JSON. + if (diffs.length <= MAX_SUMMARY_DIFFS) return diffs + return diffs.slice(0, MAX_SUMMARY_DIFFS) +} diff --git a/packages/opencode/src/session/summary.ts b/packages/opencode/src/session/summary.ts index 370870935ad6..e67e35b1bf49 100644 --- a/packages/opencode/src/session/summary.ts +++ b/packages/opencode/src/session/summary.ts @@ -6,6 +6,7 @@ import { Snapshot } from "@/snapshot" import { Session } from "./session" import { SessionID, MessageID } from "./schema" import { Config } from "@/config/config" +import { limitSummaryDiffs } from "./summary-diffs" function unquoteGitPath(input: string) { if (!input.startsWith('"')) return input @@ -121,7 +122,7 @@ export const layer = Layer.effect( ) const target = messages.find((m) => m.info.id === input.messageID) if (!target || target.info.role !== "user") return - const msgDiffs = yield* computeDiff({ messages }) + const msgDiffs = limitSummaryDiffs(yield* computeDiff({ messages })) target.info.summary = { ...target.info.summary, diffs: msgDiffs } yield* sessions.updateMessage(target.info) }) @@ -132,7 +133,7 @@ export const layer = Layer.effect( (item) => item.info.id === input.messageID, ) if (!message || message.info.role !== "user") return [] - const diffs = message.info.summary?.diffs ?? [] + const diffs = limitSummaryDiffs(message.info.summary?.diffs ?? []) return diffs.map((item) => { if (item.file === undefined) return item const file = unquoteGitPath(item.file) diff --git a/packages/opencode/test/session/summary-diffs.test.ts b/packages/opencode/test/session/summary-diffs.test.ts new file mode 100644 index 000000000000..ad56e802d6ff --- /dev/null +++ b/packages/opencode/test/session/summary-diffs.test.ts @@ -0,0 +1,18 @@ +import { expect, test } from "bun:test" +import { limitSummaryDiffs } from "../../src/session/summary-diffs" + +test("caps summary diffs to a bounded payload", () => { + const diffs = Array.from({ length: 201 }, (_, index) => ({ + file: `src/file-${index}.ts`, + patch: "@@ -1 +1 @@\n-old\n+new\n", + additions: 1, + deletions: 1, + status: "modified" as const, + })) + + const limited = limitSummaryDiffs(diffs) + + expect(limited).toHaveLength(200) + expect(limited[0]).toEqual(diffs[0]) + expect(limited[199]).toEqual(diffs[199]) +}) From cc54d2ce7ea0304c8a69b72566b3d2be113083aa Mon Sep 17 00:00:00 2001 From: jiancheng <1729303158@qq.com> Date: Fri, 26 Jun 2026 10:33:09 +0800 Subject: [PATCH 2/2] fix(session): bound diff summary payload --- packages/opencode/src/session/message-v2.ts | 11 ++- packages/opencode/src/session/session.ts | 5 +- .../opencode/src/session/summary-diffs.ts | 43 ++++++++- packages/opencode/src/session/summary.ts | 34 ++++++- packages/opencode/src/snapshot/index.ts | 92 ++++++++++++++----- .../test/session/summary-diffs.test.ts | 28 +++++- .../opencode/test/snapshot/snapshot.test.ts | 83 ++++++++++++++++- 7 files changed, 254 insertions(+), 42 deletions(-) diff --git a/packages/opencode/src/session/message-v2.ts b/packages/opencode/src/session/message-v2.ts index 798518d0ba57..488deb5894bf 100644 --- a/packages/opencode/src/session/message-v2.ts +++ b/packages/opencode/src/session/message-v2.ts @@ -35,6 +35,7 @@ import { isMedia } from "@/util/media" import type { SystemError } from "bun" import type { Provider } from "@/provider/provider" import { Effect, Schema } from "effect" +import { limitSummaryDiffs } from "./summary-diffs" export const node = LayerNode.group([Database.node]) @@ -79,12 +80,16 @@ export const cursor = { }, } -const info = (row: typeof MessageTable.$inferSelect) => - ({ +const info = (row: typeof MessageTable.$inferSelect) => { + const summary = row.data.summary + const diffs = summary?.diffs ? limitSummaryDiffs(summary.diffs) : undefined + return { ...row.data, + ...(summary ? { summary: diffs ? { ...summary, diffs } : summary } : {}), id: row.id, sessionID: row.session_id, - }) as Info + } as Info +} const part = (row: typeof PartTable.$inferSelect) => ({ diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 2d13c2e2f316..8ec0ad0ecfb0 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -44,6 +44,7 @@ import { RuntimeFlags } from "@/effect/runtime-flags" import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" import { SessionMessageID } from "@opencode-ai/schema/session-message-id" +import { limitSummaryDiffs } from "./summary-diffs" const runtime = makeRuntime(Database.Service, Database.defaultLayer) @@ -65,7 +66,7 @@ export function fromRow(row: SessionRow): Info { additions: row.summary_additions ?? 0, deletions: row.summary_deletions ?? 0, files: row.summary_files ?? 0, - diffs: row.summary_diffs ?? undefined, + diffs: row.summary_diffs ? limitSummaryDiffs(row.summary_diffs) : undefined, } : undefined const share = row.share_url ? { url: row.share_url } : undefined @@ -136,7 +137,7 @@ export function toRow(info: Info) { summary_additions: info.summary?.additions, summary_deletions: info.summary?.deletions, summary_files: info.summary?.files, - summary_diffs: info.summary?.diffs, + summary_diffs: info.summary?.diffs ? limitSummaryDiffs(info.summary.diffs) : undefined, metadata: info.metadata, cost: info.cost ?? 0, tokens_input: (info.tokens ?? EmptyTokens).input, diff --git a/packages/opencode/src/session/summary-diffs.ts b/packages/opencode/src/session/summary-diffs.ts index 4617c8f9c3d0..65c916302197 100644 --- a/packages/opencode/src/session/summary-diffs.ts +++ b/packages/opencode/src/session/summary-diffs.ts @@ -1,7 +1,40 @@ -const MAX_SUMMARY_DIFFS = 200 +export const MAX_SUMMARY_DIFFS = 200 +export const MAX_SUMMARY_PATCH_BYTES = 10_000_000 +export const MAX_SUMMARY_TOTAL_PATCH_BYTES = 10_000_000 -export function limitSummaryDiffs(diffs: T[]) { - // Keep turn summaries bounded so a pathological workspace diff cannot bloat message JSON. - if (diffs.length <= MAX_SUMMARY_DIFFS) return diffs - return diffs.slice(0, MAX_SUMMARY_DIFFS) +type SummaryDiff = { + readonly patch?: string +} + +// Bound both the number of files and the total patch payload so one huge +// workspace diff cannot balloon a stored session/message record. +export function limitSummaryDiffs(diffs: readonly T[]) { + const result: T[] = [] + let totalPatchBytes = 0 + + for (const diff of diffs) { + if (result.length >= MAX_SUMMARY_DIFFS) break + + const patch = diff.patch + if (typeof patch !== "string") { + result.push(diff) + continue + } + + const patchBytes = Buffer.byteLength(patch) + if (patchBytes > MAX_SUMMARY_PATCH_BYTES) { + result.push({ ...diff, patch: "" }) + continue + } + + if (totalPatchBytes + patchBytes > MAX_SUMMARY_TOTAL_PATCH_BYTES) { + result.push({ ...diff, patch: "" }) + continue + } + + totalPatchBytes += patchBytes + result.push(diff) + } + + return result } diff --git a/packages/opencode/src/session/summary.ts b/packages/opencode/src/session/summary.ts index e67e35b1bf49..9f81d66e0611 100644 --- a/packages/opencode/src/session/summary.ts +++ b/packages/opencode/src/session/summary.ts @@ -6,7 +6,12 @@ import { Snapshot } from "@/snapshot" import { Session } from "./session" import { SessionID, MessageID } from "./schema" import { Config } from "@/config/config" -import { limitSummaryDiffs } from "./summary-diffs" +import { + MAX_SUMMARY_DIFFS, + MAX_SUMMARY_PATCH_BYTES, + MAX_SUMMARY_TOTAL_PATCH_BYTES, + limitSummaryDiffs, +} from "./summary-diffs" function unquoteGitPath(input: string) { if (!input.startsWith('"')) return input @@ -67,7 +72,10 @@ function unquoteGitPath(input: string) { export interface Interface { readonly summarize: (input: { sessionID: SessionID; messageID: MessageID }) => Effect.Effect readonly diff: (input: { sessionID: SessionID; messageID?: MessageID }) => Effect.Effect - readonly computeDiff: (input: { messages: SessionV1.WithParts[] }) => Effect.Effect + readonly computeDiff: (input: { + messages: SessionV1.WithParts[] + limit?: number + }) => Effect.Effect } export class Service extends Context.Service()("@opencode/SessionSummary") {} @@ -80,7 +88,10 @@ export const layer = Layer.effect( const events = yield* EventV2Bridge.Service const config = yield* Config.Service - const computeDiff = Effect.fn("SessionSummary.computeDiff")(function* (input: { messages: SessionV1.WithParts[] }) { + const computeDiff = Effect.fn("SessionSummary.computeDiff")(function* (input: { + messages: SessionV1.WithParts[] + limit?: number + }) { let from: string | undefined let to: string | undefined for (const item of input.messages) { @@ -96,7 +107,18 @@ export const layer = Layer.effect( if (part.type === "step-finish" && part.snapshot) to = part.snapshot } } - if (from && to) return yield* snapshot.diffFull(from, to) + if (from && to) + return yield* snapshot.diffFull( + from, + to, + input.limit === undefined + ? undefined + : { + limit: input.limit, + maxPatchBytes: MAX_SUMMARY_PATCH_BYTES, + maxTotalPatchBytes: MAX_SUMMARY_TOTAL_PATCH_BYTES, + }, + ) return [] }) @@ -122,7 +144,9 @@ export const layer = Layer.effect( ) const target = messages.find((m) => m.info.id === input.messageID) if (!target || target.info.role !== "user") return - const msgDiffs = limitSummaryDiffs(yield* computeDiff({ messages })) + const msgDiffs = limitSummaryDiffs( + yield* computeDiff({ messages, limit: MAX_SUMMARY_DIFFS }), + ) target.info.summary = { ...target.info.summary, diffs: msgDiffs } yield* sessions.updateMessage(target.info) }) diff --git a/packages/opencode/src/snapshot/index.ts b/packages/opencode/src/snapshot/index.ts index c425d08dba7b..4a6443832832 100644 --- a/packages/opencode/src/snapshot/index.ts +++ b/packages/opencode/src/snapshot/index.ts @@ -29,10 +29,17 @@ interface GitResult { readonly code: ChildProcessSpawner.ExitCode readonly text: string readonly stderr: string + readonly truncated: boolean } type State = Omit +type DiffOptions = { + readonly limit?: number + readonly maxPatchBytes?: number + readonly maxTotalPatchBytes?: number +} + export interface Interface { readonly init: () => Effect.Effect readonly cleanup: () => Effect.Effect @@ -41,7 +48,7 @@ export interface Interface { readonly restore: (snapshot: string) => Effect.Effect readonly revert: (patches: Patch[]) => Effect.Effect readonly diff: (hash: string) => Effect.Effect - readonly diffFull: (from: string, to: string) => Effect.Effect + readonly diffFull: (from: string, to: string, options?: DiffOptions) => Effect.Effect } export class Service extends Context.Service()("@opencode/Snapshot") {} @@ -79,15 +86,19 @@ export const layer: Layer.Layer `:(top,literal)${file}`)) const git = Effect.fnUntraced( - function* (cmd: string[], opts?: { cwd?: string; env?: Record; stdin?: string }) { + function* ( + cmd: string[], + opts?: { cwd?: string; env?: Record; stdin?: string; maxOutputBytes?: number }, + ) { const result = yield* appProcess.run( ChildProcess.make("git", cmd, { cwd: opts?.cwd, env: opts?.env, extendEnv: true }), - { stdin: opts?.stdin }, + { stdin: opts?.stdin, maxOutputBytes: opts?.maxOutputBytes }, ) return { code: ChildProcessSpawner.ExitCode(result.exitCode), text: result.stdout.toString("utf8"), stderr: result.stderr.toString("utf8"), + truncated: result.stdoutTruncated, } satisfies GitResult }, Effect.catch((err) => @@ -95,6 +106,7 @@ export const layer: Layer.Layer item.text)), - ] + return ["", yield* read(`${to}:${row.file}`)] } if (row.status === "deleted") { - return [ - yield* git([...cfg, ...args(["show", `${from}:${row.file}`])]).pipe( - Effect.map((item) => item.text), - ), - "", - ] + return [yield* read(`${from}:${row.file}`), ""] } return yield* Effect.all( - [ - git([...cfg, ...args(["show", `${from}:${row.file}`])]).pipe(Effect.map((item) => item.text)), - git([...cfg, ...args(["show", `${to}:${row.file}`])]).pipe(Effect.map((item) => item.text)), - ], + [read(`${from}:${row.file}`), read(`${to}:${row.file}`)], { concurrency: 2 }, ) }) @@ -606,9 +619,12 @@ export const layer: Layer.Layer item.ref).join("\n") + "\n" }, + { + stdin: refs.map((item) => item.ref).join("\n") + "\n", + ...(maxTotalPatchBytes === undefined ? {} : { maxOutputBytes: maxTotalPatchBytes }), + }, ) - if (batch.exitCode !== 0) { + if (batch.exitCode !== 0 || batch.stdoutTruncated) { yield* Effect.logInfo( "git cat-file --batch failed during snapshot diff, falling back to per-file git show", { @@ -683,6 +699,7 @@ export const layer: Layer.Layer() + let totalPatchBytes = 0 const statuses = yield* git( [...quote, ...args(["diff", "--no-ext-diff", "--name-status", "--no-renames", from, to, "--", "."])], @@ -736,16 +753,41 @@ export const layer: Layer.Layer formatPatch(structuredPatch(file, file, before, after, "", "", { context: Number.MAX_SAFE_INTEGER })) - for (let i = 0; i < rows.length; i += step) { - const run = rows.slice(i, i + step) + for (let i = 0; i < rows.length && result.length < limit; i += step) { + const remaining = limit - result.length + const run = rows.slice(i, i + Math.min(step, remaining)) + if (!run.length) break const text = yield* load(run) for (const row of run) { const hit = text?.get(row.file) ?? { before: "", after: "" } const [before, after] = row.binary ? ["", ""] : text ? [hit.before, hit.after] : yield* show(row) + const nextPatch = row.binary ? "" : patch(row.file, before, after) + const patchBytes = Buffer.byteLength(nextPatch) + if (maxPatchBytes !== undefined && patchBytes > maxPatchBytes) { + result.push({ + file: row.file, + patch: "", + additions: row.additions, + deletions: row.deletions, + status: row.status, + }) + continue + } + if (maxTotalPatchBytes !== undefined && totalPatchBytes + patchBytes > maxTotalPatchBytes) { + result.push({ + file: row.file, + patch: "", + additions: row.additions, + deletions: row.deletions, + status: row.status, + }) + return result + } + totalPatchBytes += patchBytes result.push({ file: row.file, - patch: row.binary ? "" : patch(row.file, before, after), + patch: nextPatch, additions: row.additions, deletions: row.deletions, status: row.status, @@ -791,8 +833,8 @@ export const layer: Layer.Layer s.diff(hash)) }), - diffFull: Effect.fn("Snapshot.diffFull")(function* (from: string, to: string) { - return yield* InstanceState.useEffect(state, (s) => s.diffFull(from, to)) + diffFull: Effect.fn("Snapshot.diffFull")(function* (from: string, to: string, options?: DiffOptions) { + return yield* InstanceState.useEffect(state, (s) => s.diffFull(from, to, options)) }), }) }), diff --git a/packages/opencode/test/session/summary-diffs.test.ts b/packages/opencode/test/session/summary-diffs.test.ts index ad56e802d6ff..d4ce1e9e931c 100644 --- a/packages/opencode/test/session/summary-diffs.test.ts +++ b/packages/opencode/test/session/summary-diffs.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test" -import { limitSummaryDiffs } from "../../src/session/summary-diffs" +import { MAX_SUMMARY_TOTAL_PATCH_BYTES, limitSummaryDiffs } from "../../src/session/summary-diffs" test("caps summary diffs to a bounded payload", () => { const diffs = Array.from({ length: 201 }, (_, index) => ({ @@ -16,3 +16,29 @@ test("caps summary diffs to a bounded payload", () => { expect(limited[0]).toEqual(diffs[0]) expect(limited[199]).toEqual(diffs[199]) }) + +test("caps summary diffs by total patch bytes", () => { + const patch = "x".repeat(MAX_SUMMARY_TOTAL_PATCH_BYTES / 2 + 1) + const diffs = [ + { + file: "src/large-1.ts", + patch, + additions: 1, + deletions: 1, + status: "modified" as const, + }, + { + file: "src/large-2.ts", + patch, + additions: 1, + deletions: 1, + status: "modified" as const, + }, + ] + + const limited = limitSummaryDiffs(diffs) + + expect(limited).toHaveLength(2) + expect(limited[0].patch).toBe(patch) + expect(limited[1].patch).toBe("") +}) diff --git a/packages/opencode/test/snapshot/snapshot.test.ts b/packages/opencode/test/snapshot/snapshot.test.ts index 21ec0872b824..43f36216fa0e 100644 --- a/packages/opencode/test/snapshot/snapshot.test.ts +++ b/packages/opencode/test/snapshot/snapshot.test.ts @@ -1,10 +1,13 @@ import { afterEach, expect } from "bun:test" import { $ } from "bun" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { AppProcess } from "@opencode-ai/core/process" import { FSUtil } from "@opencode-ai/core/fs-util" import fs from "fs/promises" import path from "path" -import { Effect, Fiber, Layer } from "effect" +import { Effect, Fiber, Layer, Stream } from "effect" +import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" +import { Config } from "@/config/config" import { Snapshot } from "../../src/snapshot" import { disposeAllInstances, @@ -45,6 +48,33 @@ const exists = (file: string) => FSUtil.Service.use((fs) => fs.existsSafe(file)) const mkdirp = (dir: string) => FSUtil.Service.use((fs) => fs.ensureDir(dir)) const rm = (file: string) => FSUtil.Service.use((fs) => fs.remove(file, { recursive: true, force: true }).pipe(Effect.ignore)) +const encoder = new TextEncoder() + +function mockSpawner( + handler: (cmd: string, args: readonly string[]) => string | { code: number; stdout?: string; stderr?: string }, +) { + const spawner = ChildProcessSpawner.make((command) => { + const std = ChildProcess.isStandardCommand(command) ? command : undefined + const result = handler(std?.command ?? "", std?.args ?? []) + const output = typeof result === "string" ? { code: 0, stdout: result, stderr: "" } : result + return Effect.succeed( + ChildProcessSpawner.makeHandle({ + pid: ChildProcessSpawner.ProcessId(0), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(output.code)), + isRunning: Effect.succeed(false), + kill: () => Effect.void, + stdin: { [Symbol.for("effect/Sink/TypeId")]: Symbol.for("effect/Sink/TypeId") } as any, + stdout: output.stdout ? Stream.make(encoder.encode(output.stdout)) : Stream.empty, + stderr: output.stderr ? Stream.make(encoder.encode(output.stderr)) : Stream.empty, + all: Stream.empty, + getInputFd: () => ({ [Symbol.for("effect/Sink/TypeId")]: Symbol.for("effect/Sink/TypeId") }) as any, + getOutputFd: () => Stream.empty, + unref: Effect.succeed(Effect.void), + }), + ) + }) + return Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner) +} const initialize = Effect.fn("SnapshotTest.initialize")(function* (dir: string) { const unique = Math.random().toString(36).slice(2) @@ -102,6 +132,40 @@ const withGitConfigGlobal = (config: string, self: Effect.Effect { + if (cmd !== "git") return "" + if (args.includes("cat-file")) { + diffFullCommandCounts.catFile += 1 + return { code: 1, stderr: "batch unavailable" } + } + if (args.includes("show")) { + diffFullCommandCounts.show += 1 + const ref = args.at(-1) ?? "" + return `${ref}\ncontent\n` + } + if (args.includes("check-ignore")) return { code: 1, stdout: "" } + if (args.includes("--name-status")) + return "M\tfile-1.txt\nM\tfile-2.txt\nM\tfile-3.txt\n" + if (args.includes("--numstat")) + return "1\t1\tfile-1.txt\n1\t1\tfile-2.txt\n1\t1\tfile-3.txt\n" + return "" + }), + ), + ), + ), + Layer.provide(FSUtil.defaultLayer), + Layer.provide(Config.defaultLayer), + ), + testInstanceStoreLayer, +) +const diffFullIt = testEffect(diffFullLayer) + it.instance( "tracks deleted files correctly", withTrackedSnapshot(({ tmp, snapshot, before }) => @@ -1212,3 +1276,20 @@ it.instance( }), { git: true }, ) + +diffFullIt.instance( + "diffFull stops loading rows once the limit is reached", + Effect.gen(function* () { + diffFullCommandCounts.show = 0 + diffFullCommandCounts.catFile = 0 + + const snapshot = yield* Snapshot.Service + const diffs = yield* snapshot.diffFull("before", "after", { limit: 2 }) + + expect(diffs).toHaveLength(2) + expect(diffs.map((item) => item.file)).toEqual(["file-1.txt", "file-2.txt"]) + expect(diffFullCommandCounts.catFile).toBe(1) + expect(diffFullCommandCounts.show).toBe(4) + }), + { git: true }, +)