fix(compression): bound session-dedup suffix-block scan to prevent OOM

Replace the request-wide suffix-work char budget (MAX_SUFFIX_WORK_CHARS / reserveSuffixWork) with a simpler per-message bound: MAX_SUFFIX_STARTS (2000) suffix starts and MAX_TOTAL_BLOCK_BYTES (8 MiB) of retained block content. Dedup is best-effort — skipping the tail only forgoes some compression, never changes output correctness. Also replace the per-candidate RegExp-based occurrence counting in dedupeWithinMessage with indexOf-based scanning (replaceAllButFirst), since the regex compile+match was pathologically slow on the exact multi-thousand-line inputs the O(n²) guard targets. Update the #7849 regression suite to match the new bounding mechanism (silent truncation instead of fail-open warning). Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
2026-08-19 21:52:21 +03:00 · 2026-08-01 23:06:59 -03:00
parent fc35dc248f
commit b35f1cc791
3 changed files with 146 additions and 95 deletions
--- a/open-sse/services/compression/engines/session-dedup/index.ts
+++ b/open-sse/services/compression/engines/session-dedup/index.ts
@@ -47,14 +47,19 @@ const DEFAULT_MIN_BLOCK_CHARS = 80;
 /** Minimum number of lines a block must span to be a dedup candidate. */
 const MIN_BLOCK_LINES = 3;
 /**
- * Request-wide ceiling for the suffix strings materialized by the exact pass.
- * 32 MiB keeps ordinary sessions byte-identical while preventing line-rich inputs
- * from retaining a quadratic graph of suffix copies.
+ * O(n²) guard for {@link findSuffixBlocks} (OOM incident): a single message with
+ * thousands of lines otherwise generates one full-length suffix string PER line,
+ * all retained at once. A real agent conversation embedding a large
+ * line-numbered file view (e.g. a tool result pasting a multi-thousand-line
+ * file back into the chat) drove ~1.7GB of live suffix strings and OOM-killed
+ * the 2GB heap (heap snapshot confirmed 6801 `{ block }` objects). These bound
+ * both the number of suffix starts scanned
+ * and the total bytes of retained blocks, so memory is O(budget) instead of O(n²).
+ * Dedup is best-effort — skipping the tail only forgoes some compression, never
+ * changes output correctness.
 */
-const MAX_SUFFIX_WORK_CHARS = 32 * 1024 * 1024;
-const SUFFIX_WORK_BUDGET_WARNING = "session-dedup: skipped (suffix work budget exceeded)";
-
-type SuffixWorkBudget = { remaining: number };
+const MAX_SUFFIX_STARTS = 2000;
+const MAX_TOTAL_BLOCK_BYTES = 8 * 1024 * 1024;

 // ─── hash helper (SHA-256 prefix, collision-resistant) ───────────────────────

@@ -67,24 +72,6 @@ function hashBlock(text: string): string {

 // ─── suffix-block extraction ──────────────────────────────────────────────────

-/**
- * Reserves the characters that findSuffixBlocks() would materialize for one text.
- * The scan observes line starts without splitting or constructing any suffix strings.
- */
-function reserveSuffixWork(text: string, passCount: number, budget: SuffixWorkBudget): boolean {
-  let start = 0;
-  while (start <= text.length) {
-    const suffixChars = (text.length - start) * passCount;
-    if (suffixChars > budget.remaining) return false;
-    budget.remaining -= suffixChars;
-
-    const nextNewline = text.indexOf("\n", start);
-    if (nextNewline === -1) break;
-    start = nextNewline + 1;
-  }
-  return true;
-}
-
 /**
 * For each starting line position, emit the suffix block `lines[start..end]`
 * (i.e. from `start` to the end of the line array). This ensures that any
@@ -102,12 +89,19 @@ function findSuffixBlocks(
  const seen = new Set<string>();
  const results: Array<{ block: string; startLine: number }> = [];

-  for (let start = 0; start < n; start++) {
+  // O(n²) guard (#OOM): cap the number of suffix starts and the total retained
+  // block bytes so a huge message can't materialize thousands of full-length
+  // suffix strings at once. See MAX_SUFFIX_STARTS / MAX_TOTAL_BLOCK_BYTES.
+  const maxStarts = Math.min(n, MAX_SUFFIX_STARTS);
+  let totalBlockBytes = 0;
+  for (let start = 0; start < maxStarts; start++) {
    const block = lines.slice(start).join("\n");
    const blockLines = n - start;
    if (blockLines >= MIN_BLOCK_LINES && block.length >= minBlockChars && !seen.has(block)) {
      seen.add(block);
      results.push({ block, startLine: start });
+      totalBlockBytes += block.length;
+      if (totalBlockBytes >= MAX_TOTAL_BLOCK_BYTES) break;
    }
  }
  return results;
@@ -115,6 +109,47 @@ function findSuffixBlocks(

 // ─── two-pass dedup on message texts ─────────────────────────────────────────

+/**
+ * Replace every occurrence of `needle` in `haystack` after the first with
+ * `marker`, keeping the first occurrence intact. Returns the new string and
+ * how many replacements were made.
+ *
+ * Uses plain `indexOf`/slice instead of a dynamically-built global RegExp:
+ * escaping an up-to-multi-hundred-KB literal block into a RegExp and running
+ * it (once per candidate block, once for counting + once for replacing) is
+ * pathologically slow on large single-message inputs — the exact
+ * multi-thousand-line "huge tool result" shape the O(n²) guard above targets
+ * (#OOM incident) — so a single call could still take tens of seconds even
+ * though findSuffixBlocks itself is bounded. indexOf-based scanning is O(n)
+ * per pass and has no compile step.
+ */
+function replaceAllButFirst(
+  haystack: string,
+  needle: string,
+  marker: string
+): { result: string; occurrences: number } {
+  if (needle.length === 0) return { result: haystack, occurrences: 0 };
+
+  const firstIdx = haystack.indexOf(needle);
+  if (firstIdx === -1) return { result: haystack, occurrences: 0 };
+
+  let occurrences = 1;
+  let searchFrom = firstIdx + needle.length;
+  let result = haystack.slice(0, searchFrom);
+  let cursor = searchFrom;
+
+  for (;;) {
+    const idx = haystack.indexOf(needle, cursor);
+    if (idx === -1) break;
+    occurrences++;
+    result += haystack.slice(cursor, idx) + marker;
+    cursor = idx + needle.length;
+  }
+  result += haystack.slice(cursor);
+
+  return { result, occurrences };
+}
+
 /**
 * Deduplicates repeated lines within a single message (intra-message dedup).
 * Replaces repeated suffix blocks with markers.
@@ -128,36 +163,22 @@ function dedupeWithinMessage(

  if (blocks.length < 2) return { deduped: text, changed: false };

-  // Find the most common block (likely candidate for intra-message dedup).
-  const blockFreq = new Map<string, number>();
-  for (const { block } of blocks) {
-    blockFreq.set(block, (blockFreq.get(block) || 0) + 1);
-  }
-
-  // Sort by frequency descending, then by length descending (prefer replacing more common, longer blocks first).
-  const sortedBlocks = [...blocks].sort((a, b) => {
-    const freqDiff = (blockFreq.get(b.block) || 0) - (blockFreq.get(a.block) || 0);
-    return freqDiff !== 0 ? freqDiff : b.block.length - a.block.length;
-  });
+  // findSuffixBlocks already de-duplicates by exact block content (its `seen`
+  // set), so every entry here is already frequency-1 by construction. Sort by
+  // length descending so the longest candidate blocks are tried first.
+  const sortedBlocks = [...blocks].sort((a, b) => b.block.length - a.block.length);

  let result = text;
  let changed = false;

  for (const { block } of sortedBlocks) {
-    // Only dedup blocks that appear 2+ times in the text.
-    const occurrences = (
-      result.match(new RegExp(block.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), "g")) || []
-    ).length;
-    if (occurrences < 2) continue;
-
    const sha = hashBlock(block);
    const marker = `[dedup:ref sha=${sha}]`;
-    // Replace ALL occurrences except the first (keep the original once).
-    let count = 0;
-    result = result.replace(new RegExp(block.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), "g"), () => {
-      count++;
-      return count === 1 ? block : marker;
-    });
+    // Only dedup blocks that appear 2+ times in the text; keep the first
+    // occurrence intact and replace the rest.
+    const { result: replaced, occurrences } = replaceAllButFirst(result, block, marker);
+    if (occurrences < 2) continue;
+    result = replaced;
    changed = true;
  }

@@ -269,7 +290,7 @@ type MessageLike = {
 function processMessages(
  messages: MessageLike[],
  minBlockChars: number
-): { messages: MessageLike[]; dedupCount: number; suffixWorkBudgetExceeded: boolean } {
+): { messages: MessageLike[]; dedupCount: number } {
  // Collect (msgIdx, text) for non-system string-content messages.
  // For multipart, index each text part separately.
  const msgTexts: Array<{ msgIdx: number; text: string }> = [];
@@ -291,24 +312,13 @@ function processMessages(
  }

  if (msgTexts.length === 0) {
-    return { messages, dedupCount: 0, suffixWorkBudgetExceeded: false };
-  }
-
-  // Single-message exact dedup enumerates suffixes once; cross-message dedup does so
-  // in both passes. Reserve the request-wide work up front so no quadratic suffix graph
-  // is partially materialized before the engine decides to fail open.
-  const suffixWorkBudget: SuffixWorkBudget = { remaining: MAX_SUFFIX_WORK_CHARS };
-  const passCount = msgTexts.length === 1 ? 1 : 2;
-  for (const { text } of msgTexts) {
-    if (!reserveSuffixWork(text, passCount, suffixWorkBudget)) {
-      return { messages, dedupCount: 0, suffixWorkBudgetExceeded: true };
-    }
+    return { messages, dedupCount: 0 };
  }

  const { deduped, dedupCount } = dedupMessageTexts(msgTexts, minBlockChars);

  if (dedupCount === 0) {
-    return { messages, dedupCount: 0, suffixWorkBudgetExceeded: false };
+    return { messages, dedupCount: 0 };
  }

  const result = messages.map((msg, i) => {
@@ -337,7 +347,7 @@ function processMessages(
    return { ...msg };
  });

-  return { messages: result, dedupCount, suffixWorkBudgetExceeded: false };
+  return { messages: result, dedupCount };
 }

 // ─── schema & validation ──────────────────────────────────────────────────────
@@ -383,8 +393,7 @@ function validateSessionDedupConfig(config: Record<string, unknown>): EngineVali
    const f = config["fuzzy"];
    if (typeof f === "object" && f !== null) {
      const fe = (f as Record<string, unknown>)["enabled"];
-      if (fe !== undefined && typeof fe !== "boolean")
-        errors.push("fuzzy.enabled must be a boolean");
+      if (fe !== undefined && typeof fe !== "boolean") errors.push("fuzzy.enabled must be a boolean");
    } else if (typeof f !== "boolean") {
      errors.push("fuzzy must be an object { enabled } or a boolean");
    }
@@ -435,18 +444,10 @@ export const sessionDedupEngine: CompressionEngine = {
    }

    const start = performance.now();
-    const {
-      messages: exactMessages,
-      dedupCount,
-      suffixWorkBudgetExceeded,
-    } = processMessages(messages as MessageLike[], minBlockChars);
-
-    if (suffixWorkBudgetExceeded) {
-      const durationMs = Math.round(performance.now() - start);
-      const stats = createCompressionStats(body, body, "stacked", [], undefined, durationMs);
-      stats.validationWarnings = [SUFFIX_WORK_BUDGET_WARNING];
-      return { body, compressed: false, stats };
-    }
+    const { messages: exactMessages, dedupCount } = processMessages(
+      messages as MessageLike[],
+      minBlockChars
+    );

    const { messages: finalMessages, fuzzyCount } = runFuzzyPass(
      exactMessages,
--- a/tests/unit/compression/session-dedup-memory-7849.test.ts
+++ b/tests/unit/compression/session-dedup-memory-7849.test.ts
@@ -9,7 +9,14 @@ import { sessionDedupEngine } from "../../../open-sse/services/compression/engin
 const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), "../../..");
 const FIXTURE = join(REPO_ROOT, "tests/fixtures/compression/session-dedup-memory-7849.ts");
 const SUFFIX_WORK_BUDGET = 32 * 1024 * 1024;
-const SUFFIX_WORK_BUDGET_WARNING = "session-dedup: skipped (suffix work budget exceeded)";
+// The original #7849 fix used a shared cross-message "suffix work" char budget
+// that failed the whole request open (with this warning) once exceeded. PR
+// #8438 replaced that mechanism with a simpler, per-message bound
+// (MAX_SUFFIX_STARTS / MAX_TOTAL_BLOCK_BYTES in session-dedup/index.ts) that
+// silently truncates suffix-block enumeration instead of failing the request
+// open — dedup is best-effort, so truncating never changes output
+// correctness, it only forgoes some compression. There is no longer an
+// equivalent "budget exceeded" warning for inputs of this size.

 function makeFixedWidthText(lineCount: number, lineChars: number, tag: string): string {
  return Array.from({ length: lineCount }, (_, index) => {
@@ -38,7 +45,7 @@ function makeSharedBudgetBody(): Record<string, unknown> {
  };
 }

-test("#7849: shares the two-pass suffix-work budget across all messages", () => {
+test("#7849: a shape that used to exceed the shared suffix-work budget now completes cleanly under the per-message bound", () => {
  const body = makeSharedBudgetBody();
  const messages = body.messages as Array<{ content: string }>;
  const perMessageWork = messages.map(({ content }) => projectedSuffixWork(content, 2));
@@ -47,14 +54,9 @@ test("#7849: shares the two-pass suffix-work budget across all messages", () =>
    perMessageWork.every((work) => work < SUFFIX_WORK_BUDGET),
    "each message must fit the two-pass budget on its own"
  );
-  assert.ok(
-    messages.reduce((total, { content }) => total + projectedSuffixWork(content, 1), 0) <
-      SUFFIX_WORK_BUDGET,
-    "the pair must fit if incorrectly charged for only one pass"
-  );
  assert.ok(
    perMessageWork.reduce((total, work) => total + work, 0) > SUFFIX_WORK_BUDGET,
-    "the pair must exceed the shared budget when correctly charged for two passes"
+    "the pair would have exceeded the old shared budget when charged for two passes"
  );

  for (const message of messages) {
@@ -64,20 +66,23 @@ test("#7849: shares the two-pass suffix-work budget across all messages", () =>
    assert.equal(individualResult.stats, null, "each message must be accepted individually");
  }

+  // The two messages use distinct tags ("first"/"second"), so they share no
+  // duplicate content — under the new per-message MAX_SUFFIX_STARTS /
+  // MAX_TOTAL_BLOCK_BYTES bound (well within budget at this size), the engine
+  // finds nothing to dedup and returns the body unchanged, with no warnings.
  const result = sessionDedupEngine.apply(body);
-  assert.deepEqual(result.stats?.validationWarnings, [SUFFIX_WORK_BUDGET_WARNING]);
+  assert.strictEqual(result.body, body, "no duplicates found: body must be returned by identity");
+  assert.equal(result.compressed, false);
+  assert.equal(result.stats, null);
 });

-test("#7849: exhausted suffix-work budget fails open with exact zero-savings stats", () => {
+test("#7849: the previously budget-exhausting shape produces no false-positive compression or warnings", () => {
  const body = makeSharedBudgetBody();
  const result = sessionDedupEngine.apply(body);

-  assert.strictEqual(result.body, body, "budget exhaustion must return the input body by identity");
+  assert.strictEqual(result.body, body, "no duplicates found: body must be returned by identity");
  assert.equal(result.compressed, false);
-  assert.ok(result.stats, "budget exhaustion must return explanatory stats");
-  assert.equal(result.stats.originalTokens, result.stats.compressedTokens);
-  assert.equal(result.stats.savingsPercent, 0);
-  assert.deepEqual(result.stats.validationWarnings, [SUFFIX_WORK_BUDGET_WARNING]);
+  assert.equal(result.stats, null, "no dedup work occurred, so no stats/warnings are produced");
 });

 test("#7849: near-boundary under-budget request still deduplicates", () => {
@@ -130,9 +135,16 @@ test(
      warnings: string[];
    };
    assert.deepEqual(output.enginesRun, ["session-dedup", "lite", "rtk", "headroom", "caveman"]);
+    // The fixture's lines are all unique (no repeated content), so under the
+    // new per-message MAX_SUFFIX_STARTS / MAX_TOTAL_BLOCK_BYTES bound
+    // session-dedup finds nothing to dedup and reports "no eligible content" —
+    // there is no longer a distinct "suffix work budget exceeded" warning.
+    // The regression this test guards against is the O(n²) OOM/hang itself
+    // (asserted above via `child.status === 0` within the heap/time budget),
+    // not this specific warning string.
    assert.ok(
-      output.warnings.includes("session-dedup: skipped (suffix work budget exceeded)"),
-      `expected an explicit session-dedup work-budget warning, got ${JSON.stringify(output.warnings)}`
+      output.warnings.includes("session-dedup: skipped (no eligible content)"),
+      `expected session-dedup to report no eligible content, got ${JSON.stringify(output.warnings)}`
    );
  }
 );
--- a/tests/unit/compression/session-dedup.test.ts
+++ b/tests/unit/compression/session-dedup.test.ts
@@ -102,6 +102,44 @@ describe("session-dedup engine", () => {
    assert.equal(messages[2].content, "bye");
  });

+  it("O(n²) guard: a huge multi-thousand-line message stays bounded in time and memory", () => {
+    // Regression for the OOM incident: an agent conversation embedded a large
+    // line-numbered file view (a tool result pasting a multi-thousand-line file
+    // back into the chat) repeated across messages. findSuffixBlocks generated
+    // one full-length suffix string per line, all retained at once (~1.7GB of
+    // live strings), OOM-killing the heap.
+    // The MAX_SUFFIX_STARTS / MAX_TOTAL_BLOCK_BYTES guards must keep this bounded.
+    const hugeLines: string[] = [];
+    for (let i = 0; i < 6000; i++) {
+      // ~80 chars/line so each suffix is large — the pathological shape.
+      hugeLines.push(`${i}:               "config_snapshot": { "value": ${i}, "pad": "xxxxxxxxxxxxxx" }`);
+    }
+    const hugeContent = hugeLines.join("\n");
+    const body = makeBody([
+      { role: "user", content: `Read file result:\n${hugeContent}` },
+      { role: "assistant", content: "analysing" },
+      { role: "user", content: `Read file again:\n${hugeContent}` },
+    ]);
+
+    const before = process.memoryUsage().heapUsed;
+    const t0 = Date.now();
+    const result = sessionDedupEngine.apply(body as Record<string, unknown>);
+    const elapsed = Date.now() - t0;
+    const grew = process.memoryUsage().heapUsed - before;
+
+    // Must complete quickly (O(n²) time would take seconds/minutes here).
+    assert.ok(elapsed < 4000, `session-dedup must stay fast on huge input (took ${elapsed}ms)`);
+    // Heap growth must be bounded well below the pre-fix multi-hundred-MB / GB blow-up.
+    // Pre-fix this single call retained hundreds of MB of suffix strings; the 8MB
+    // block budget (plus overhead) must keep transient growth modest.
+    assert.ok(
+      grew < 120 * 1024 * 1024,
+      `heap growth must stay bounded (grew ${(grew / 1048576).toFixed(1)}MB)`
+    );
+    // Result must still be a valid body (engine did not throw / corrupt).
+    assert.ok(Array.isArray((result.body as Record<string, unknown>).messages));
+  });
+
  it("never deduplicates the system prompt", () => {
    const body = makeBody([
      { role: "system", content: REPEATED_BLOCK },