fix(harness): truncation marker could push output past its char budget

.changeset/fix-harness-truncation-marker-budget.md

@@ -0,0 +1,5 @@
+---
+"@sapiom/harness": patch
+---
+
+Fix truncation helpers that could return output slightly longer than their configured character budget.

packages/harness/src/core/collector/normalizer.test.ts

@@ -150,10 +150,22 @@ describe("truncateForPayload", () => {
     expect(truncateForPayload({ a: 1 })).toBe(JSON.stringify({ a: 1 }));
   });
 
-  it("truncates long strings with a marker", () => {
-    const long = "y".repeat(20);
-    const result = truncateForPayload(long, 10);
-    expect(result.startsWith("y".repeat(10))).toBe(true);
-    expect(result).toContain("[truncated 10 chars]");
+  it("truncates long strings with a marker, bounding the total length", () => {
+    const long = "y".repeat(200);
+    const result = truncateForPayload(long, 50);
+    // The marker's own length has to come out of the 50-char budget too, so
+    // less than 50 chars of content survive -- the total (content + marker)
+    // is what's bounded, not the content alone.
+    expect(result).toBe(`${"y".repeat(28)}…[truncated 172 chars]`);
+    expect(result.length).toBe(50);
+  });
+
+  it("still respects the bound when maxLength is smaller than the marker itself", () => {
+    // Degenerate case: production budgets (MAX_FIELD_LENGTH, MAX_TOOL_RESPONSE_LENGTH)
+    // are always far larger than a marker, but the bound has to hold even
+    // here -- there's no content left to keep, so the marker itself gets
+    // clipped too, rather than the output exceeding maxLength.
+    const result = truncateForPayload("y".repeat(20), 10);
+    expect(result.length).toBe(10);
   });
 });

packages/harness/src/core/collector/normalizer.ts

@@ -50,11 +50,31 @@ const MAX_FIELD_LENGTH = 4000;
 /** Tool output can be much larger than other fields; cap it separately. */
 const MAX_TOOL_RESPONSE_LENGTH = 16 * 1024;
 
-/** Stringify + truncate a value so a giant tool payload can't blow up storage. */
+/**
+ * Stringify + truncate a value so a giant tool payload can't blow up storage.
+ * Bounds the TOTAL output (content + marker) to `maxLength`: the marker's own
+ * length depends on the dropped-char count, which depends on where we slice,
+ * which depends on the marker's length, so this converges over a few passes
+ * the same way {@link clip} in record-archive.ts does (kept in sync with it
+ * intentionally — both write the same {@link PAYLOAD_TRUNCATION_MARKER}
+ * format and both need the same fix for the same reason).
+ */
 export function truncateForPayload(value: unknown, maxLength = MAX_FIELD_LENGTH): string {
   const str = typeof value === "string" ? value : JSON.stringify(value ?? null);
   if (str.length <= maxLength) return str;
-  return `${str.slice(0, maxLength)}…[truncated ${str.length - maxLength} chars]`;
+  let sliceLen = maxLength;
+  for (let i = 0; i < 5; i++) {
+    const dropped = str.length - sliceLen;
+    const marker = `…[truncated ${dropped} chars]`;
+    const nextSliceLen = Math.max(0, maxLength - marker.length);
+    if (nextSliceLen === sliceLen) {
+      return `${str.slice(0, sliceLen)}${marker}`.slice(0, maxLength);
+    }
+    sliceLen = nextSliceLen;
+  }
+  const dropped = str.length - sliceLen;
+  const marker = `…[truncated ${dropped} chars]`;
+  return `${str.slice(0, sliceLen)}${marker}`.slice(0, maxLength);
 }
 
 /**

packages/harness/src/core/record-archive.test.ts

@@ -79,7 +79,10 @@ describe("compactSessionRecord", () => {
     const call = compacted.turns[0].toolCalls[0];
     expect(call.input).toMatch(/…\[truncated \d+ chars\]$/);
     expect(call.input?.length).toBeLessThan(200);
-    expect(call.responseSummary).toBe(`${"y".repeat(100)}…[truncated 3900 chars]`);
+    // Bounded to maxToolResponseChars total (content + marker), not content
+    // alone -- 100 chars minus the marker's own length leaves 77 y's here.
+    expect(call.responseSummary).toBe(`${"y".repeat(77)}…[truncated 3923 chars]`);
+    expect(call.responseSummary?.length).toBe(100);
     // A result this pass shortened is truncated, whoever shortened it.
     expect(call.responseTruncated).toBe(true);
     // The conversation itself is untouched — that's the part worth archiving.
@@ -111,10 +114,13 @@ describe("compactSessionRecord", () => {
       { maxToolResponseChars: 100 },
     );
 
-    // 300 - 100 taken here, plus the 1000 the collector had already dropped.
+    // 300 z's clipped to 77 (100-char budget minus the marker's own length),
+    // plus the 1000 the collector had already dropped: 223 newly dropped
+    // here + 1000 folded in = 1223.
     expect(compacted.turns[0].toolCalls[0].responseSummary).toBe(
-      `${"z".repeat(100)}…[truncated 1200 chars]`,
+      `${"z".repeat(77)}…[truncated 1223 chars]`,
     );
+    expect(compacted.turns[0].toolCalls[0].responseSummary?.length).toBe(100);
   });
 
   it("claims no compaction when nothing was actually clipped", () => {

packages/harness/src/core/record-archive.ts

@@ -157,9 +157,27 @@ function clip(value: string | null, maxChars: number): string | null {
   if (value === null) return null;
   const marker = PAYLOAD_TRUNCATION_MARKER.exec(value);
   const body = marker ? value.slice(0, value.length - marker[0].length) : value;
+  const alreadyOmitted = marker ? Number(marker[1]) : 0;
   if (body.length <= maxChars) return value;
-  const omitted = body.length - maxChars + (marker ? Number(marker[1]) : 0);
-  return `${body.slice(0, maxChars)}…[truncated ${omitted} chars]`;
+  // Same converging-slice fix as truncateForPayload, plus the unwrap step
+  // above: the omitted count folds in whatever a prior pass already cut, and
+  // idempotency holds because the fixed point always lands the body at
+  // exactly `maxChars - newMarker.length`, which a second pass's unwrap step
+  // reproduces exactly (see truncateForPayload's doc comment for why this
+  // needs a loop instead of a single subtraction).
+  let sliceLen = maxChars;
+  for (let i = 0; i < 5; i++) {
+    const omitted = body.length - sliceLen + alreadyOmitted;
+    const newMarker = `…[truncated ${omitted} chars]`;
+    const nextSliceLen = Math.max(0, maxChars - newMarker.length);
+    if (nextSliceLen === sliceLen) {
+      return `${body.slice(0, sliceLen)}${newMarker}`.slice(0, maxChars);
+    }
+    sliceLen = nextSliceLen;
+  }
+  const omitted = body.length - sliceLen + alreadyOmitted;
+  const newMarker = `…[truncated ${omitted} chars]`;
+  return `${body.slice(0, sliceLen)}${newMarker}`.slice(0, maxChars);
 }
 
 export interface CompactionOptions {

packages/harness/src/core/resume-brief.test.ts

@@ -261,6 +261,16 @@ describe("buildResumeBrief", () => {
     it("returns null for a tool call with no recorded input at all", () => {
       expect(describeToolTarget(toolCall("Bash", null as unknown as string, { input: null }), null)).toBeNull();
     });
+
+    it("bounds a clamped target to MAX_TOOL_TARGET_CHARS total, not content alone", () => {
+      // A command longer than the 80-char cap forces describeToolTarget's
+      // internal clamp() to truncate. The returned string (content + marker)
+      // must not exceed the cap -- the marker's own length has to come out
+      // of the budget too.
+      const target = describeToolTarget(toolCall("Bash", { command: "x".repeat(200) }), "/Users/dev/project");
+      expect(target?.length).toBeLessThanOrEqual(80);
+      expect(target).toContain("…[truncated]");
+    });
   });
 
   describe("token budget", () => {

packages/harness/src/core/resume-brief.ts

@@ -85,7 +85,13 @@ const MAX_FILES = 30;
 const MAX_COMMANDS = 10;
 const MAX_COMMAND_CHARS = 120;
 
-/** The marker `truncateForPayload` (core/collector/normalizer.ts) leaves. */
+/**
+ * Fixed marker `clamp` appends when it cuts text short. Deliberately distinct
+ * from `truncateForPayload`'s `…[truncated N chars]` (core/collector/normalizer.ts)
+ * — that one records a dropped-char count computed by the collector; this one
+ * has no count to report (the resume brief clamps for display, not storage
+ * accounting), so it stays a plain, fixed string.
+ */
 const CLAMP_MARKER = "…[truncated]";
 
 /** The workflow the prior session was bound to, resolved by the caller against
@@ -120,12 +126,18 @@ export function estimateBriefTokens(text: string): number {
   return Math.ceil(text.length / CHARS_PER_TOKEN);
 }
 
-/** Clamp `text` to `maxChars`, marking the cut so nothing reads as complete
- *  when it isn't. */
+/**
+ * Clamp `text` to `maxChars` TOTAL (content + marker), marking the cut so
+ * nothing reads as complete when it isn't. CLAMP_MARKER's length is fixed
+ * (unlike a "chars dropped" marker whose length depends on where the slice
+ * lands), so a single subtraction — not a converging loop — correctly
+ * reserves room for it within maxChars.
+ */
 function clamp(text: string, maxChars: number): string {
   const trimmed = text.trim();
   if (trimmed.length <= maxChars) return trimmed;
-  return `${trimmed.slice(0, maxChars).trimEnd()}${CLAMP_MARKER}`;
+  const sliceLen = Math.max(0, maxChars - CLAMP_MARKER.length);
+  return `${trimmed.slice(0, sliceLen).trimEnd()}${CLAMP_MARKER}`.slice(0, maxChars);
 }
 
 /**
@@ -409,7 +421,9 @@ export function buildResumeBrief(
   //    pass strictly shortens `summaryText`, so this terminates.
   while (summaryText.length > 0 && over()) {
     const overflow = assemble().length - budgetChars;
-    summaryText = clamp(summaryText, Math.max(0, summaryText.length - overflow - CLAMP_MARKER.length));
+    // clamp() now self-enforces its own total-length bound, so no need to
+    // pre-subtract CLAMP_MARKER.length here the way this had to before.
+    summaryText = clamp(summaryText, Math.max(0, summaryText.length - overflow));
     if (summaryText === CLAMP_MARKER) summaryText = "";
   }