fix(agent): stop lossy-truncating retrieval tool results that must arrive whole

load_skill returns SKILL.md in full by design — it is the model's behavioral
contract, and pagination by default would let the model silently miss later
mandatory sections. read_file / readSkillFile / load_skill are therefore on
the spill-exclusion list so their output is never replaced by a disk pointer.

The exclusion only covered half the path. In spillRawOrTruncate, an excluded
tool's result came back from persistIfOversized unchanged (no spill), failed
the SPILL_MARKER_PREFIX check, and fell through to truncateToolResult(8000) —
so an 8261-char SKILL.md was hard-cut through the middle and stamped with a
'[TRUNCATED: ... middle omitted]' marker. Weaker models ignore the attached
fidelity note and fabricate the removed span, inventing tool calls against
endpoints the skill never described.

- spillRawOrTruncate now returns retrieval-excluded results raw; the per-turn
  aggregate budget stays the backstop.
- Outsized SKILL.md degrades to resumable pagination instead of an unbounded
  inline dump. Never a lossy middle-cut.
This commit is contained in:
matevip 2026-07-28 05:06:24 -04:00
parent 3643aed756
commit fcf5f3960b
5 changed files with 122 additions and 13 deletions

View File

@ -99,9 +99,12 @@ public class ToolExecutionExecutor {
* when size > perResultThresholdChars and tool is not
* in the spill exclusion list. Returns a SPILL_MARKER preview
* on success, or the original string otherwise.
* if no SPILL_MARKER on the return, truncateToolResult(...)
* caps inline to MAX_TOOL_RESULT_CHARS so a multi-MB raw
* body never enters the model prompt.
* retrieval-excluded tool (load_skill / read_file / ...)
* returned RAW, never inline-truncated: a partial SKILL.md
* invites the model to fabricate the omitted span.
* otherwise truncateToolResult(...) caps inline to
* MAX_TOOL_RESULT_CHARS so a multi-MB raw body never
* enters the model prompt.
* enforceTurnBudget(..., perTurnBudgetChars=32000) // per-turn aggregate
* </pre>
* Spill must see the RAW result so the full output is preserved on disk
@ -132,8 +135,10 @@ public class ToolExecutionExecutor {
* @param toolUseId unique within the conversation; becomes the file name
* @param conversationId spill files are scoped per conversation; blank/null falls back to "unknown"
* @param workspaceBasePath where the spill directory lives when set
* @return the SPILL_MARKER preview when spill succeeded, otherwise the
* original string (when threshold) or the inline-truncated string.
* @return the SPILL_MARKER preview when spill succeeded; the original
* string when threshold or when the tool is retrieval-excluded
* (those must reach the model whole); otherwise the
* inline-truncated string.
*/
static String spillRawOrTruncate(ToolResultStorage storage, int maxTruncateChars,
String result, String toolName, String toolUseId,
@ -147,6 +152,20 @@ public class ToolExecutionExecutor {
if (candidate != null && candidate.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX)) {
return candidate;
}
// Retrieval-style tools (load_skill, read_file, readSkillFile,
// memory reads) are on the spill-exclusion list precisely so their
// full output reaches the model. persistIfOversized returns them
// unchanged (no spill), so control reaches here but inline
// hard-truncation would silently re-introduce exactly the
// incompleteness the exclusion prevents: a chopped SKILL.md makes
// the model act on partial instructions, and weak models fabricate
// the omitted middle instead of heeding the fidelity note. Return
// the raw body; enforceTurnBudget (Layer 3) already skips these
// tools and only compacts them as a last resort when the whole
// turn blows its aggregate budget and nothing else can be freed.
if (storage.isRetrievalExcluded(toolName)) {
return result;
}
}
return truncateToolResult(result, maxTruncateChars);
}

View File

@ -105,6 +105,21 @@ public class ToolResultStorage {
return spillCount.get();
}
/**
* Public view of the exclusion list for callers outside this class (e.g.
* the executor's spill/truncate dispatcher). Retrieval-style tools on this
* list must have their full output preserved: they are never spilled here
* (Layer 2) and never inline-truncated by the executor a partial
* {@code SKILL.md} / {@code read_file} body makes the model act on
* incomplete data, and weak models fabricate the omitted span instead of
* heeding the fidelity note. The aggregate turn budget (Layer 3) remains
* the only place an excluded result may be compacted, and only as a last
* resort when nothing else can free budget.
*/
public boolean isRetrievalExcluded(String toolName) {
return isExcluded(toolName);
}
/**
* Returns true when {@code toolName} is in the configured exclusion list.
* Excluded tools (typically retrieval tools like {@code read_file}) are

View File

@ -40,6 +40,21 @@ public class SkillFileTool {
private static final int DEFAULT_MAX_LINES = 200;
private static final int MAX_OUTPUT_CHARS = 8_000;
/**
* Ceiling for returning SKILL.md in one piece. Below it the full contract
* is returned verbatim the common case, and the only way the model sees
* every mandatory section. Above it the read degrades to resumable
* pagination (page + "continue with startLine=N" banner) rather than an
* unbounded inline dump.
*
* <p>Deliberately far above {@link #MAX_OUTPUT_CHARS} so ordinary skills
* (a few thousand chars) are never split: splitting a contract the model
* can silently under-read is the more expensive failure. It matches the
* per-turn aggregate budget, the point past which a single result would
* dominate the turn regardless.
*/
private static final int MAX_FULL_SKILL_CHARS = 32_000;
private final SkillRuntimeService runtimeService;
private final SkillFileAccessPolicy accessPolicy;
private final SkillUsageService usageService;
@ -107,8 +122,14 @@ public class SkillFileTool {
// pagination via startLine or maxLines. References / scripts are
// still paginated below because they can be large supplementary
// material the model loads on demand.
// Safety valve: an outsized SKILL.md degrades to resumable
// pagination instead of an unbounded inline dump. Never a
// lossy middle-cut a contract with its middle silently
// removed is what makes models fabricate the missing span;
// a page plus an explicit "continue with startLine=N" banner
// keeps the read complete-able.
boolean paginationRequested = startLine != null || maxLines != null;
if (!paginationRequested) {
if (!paginationRequested && skill.getContent().length() <= MAX_FULL_SKILL_CHARS) {
return skill.getContent();
}
return paginateSkillContent(skillName, "SKILL.md", skill.getContent(), startLine, maxLines);

View File

@ -177,18 +177,23 @@ class LaneDExecutorAndConfigTest {
}
@Test
@DisplayName("raw body > threshold but tool is on exclusion list → no spill, inline hard cap")
void rawOverThresholdExcludedToolTruncatesOnly() {
ToolResultStorage st = storage(1000, List.of("read_file"));
String raw = "x".repeat(20000);
@DisplayName("raw body > threshold but tool is on exclusion list → no spill AND no inline truncation (full body preserved)")
void rawOverThresholdExcludedToolReturnedWhole() {
ToolResultStorage st = storage(1000, List.of("load_skill"));
// A ~8261-char SKILL.md is the real-world case: over the 8000 hard
// cap but the model must see it whole, or it fabricates the gap.
String raw = "SKILL contract line\n".repeat(500); // ~10000 chars
String out = ToolExecutionExecutor.spillRawOrTruncate(
st, 8000, raw, "read_file", "call-1", "conv-x", tempDir.toString());
st, 8000, raw, "load_skill", "call-1", "conv-x", tempDir.toString());
assertFalse(out.startsWith(ToolResultStorage.SPILL_MARKER_PREFIX),
"excluded tool must not be spilled");
assertTrue(out.length() <= 8000,
"excluded body still must fit the inline hard cap (was " + out.length() + ")");
assertEquals(raw, out,
"excluded retrieval tool must be returned WHOLE — inline-truncating it "
+ "re-introduces the incompleteness the exclusion list exists to prevent");
assertFalse(out.contains("TRUNCATED"),
"no fabrication-inducing truncation marker may be injected into an excluded tool result");
}
@Test

View File

@ -251,6 +251,55 @@ class SkillFileToolTest {
assertFalse(result.contains("is not available for this agent"));
}
@Test
@DisplayName("SKILL.md just over the 8KB tool-result cap is still returned whole (no lossy cut)")
void readSkillFileReturnsFullSkillMdJustOverToolResultCap() {
// Regression: an 8261-char SKILL.md used to be hard-cut to 8000 chars
// downstream with a "[TRUNCATED: ... middle omitted]" marker, and the
// model fabricated the removed middle. Anything under the full-return
// ceiling must arrive intact so every mandatory section is visible.
SkillRuntimeService runtimeService = mock(SkillRuntimeService.class);
SkillFileAccessPolicy accessPolicy = mock(SkillFileAccessPolicy.class);
SkillUsageService usageService = mock(SkillUsageService.class);
AgentWorkspaceResolver workspaceResolver = mock(AgentWorkspaceResolver.class);
when(workspaceResolver.resolve(any())).thenReturn(1L);
SkillFileTool tool = new SkillFileTool(runtimeService, accessPolicy, usageService, workspaceResolver);
ResolvedSkill skill = skill("meeting-skill", "database", true);
String body = "# Contract\n" + "instruction line\n".repeat(500) + "\nFINAL_MANDATORY_SECTION";
skill.setContent(body);
when(runtimeService.findActiveSkill(eq("meeting-skill"), any())).thenReturn(skill);
String content = tool.readSkillFile("meeting-skill", "SKILL.md", null, null, null);
assertTrue(body.length() > 8_000, "fixture must exceed the 8KB cap to be a valid regression");
assertEquals(body, content, "SKILL.md under the ceiling must be returned byte-for-byte");
assertTrue(content.contains("FINAL_MANDATORY_SECTION"),
"the trailing mandatory section must survive — losing it is what caused fabrication");
assertFalse(content.contains("truncated"), "no truncation banner may be injected");
}
@Test
@DisplayName("SKILL.md past the full-return ceiling degrades to resumable pagination, never a middle-cut")
void readSkillFileOversizedSkillMdPaginatesResumably() {
SkillRuntimeService runtimeService = mock(SkillRuntimeService.class);
SkillFileAccessPolicy accessPolicy = mock(SkillFileAccessPolicy.class);
SkillUsageService usageService = mock(SkillUsageService.class);
AgentWorkspaceResolver workspaceResolver = mock(AgentWorkspaceResolver.class);
when(workspaceResolver.resolve(any())).thenReturn(1L);
SkillFileTool tool = new SkillFileTool(runtimeService, accessPolicy, usageService, workspaceResolver);
ResolvedSkill skill = skill("giant-skill", "database", true);
skill.setContent("instruction line\n".repeat(4000)); // ~68KB, past the 32KB ceiling
when(runtimeService.findActiveSkill(eq("giant-skill"), any())).thenReturn(skill);
String content = tool.readSkillFile("giant-skill", "SKILL.md", null, null, null);
assertTrue(content.startsWith("instruction line"), "page one must begin at the top of the contract");
assertTrue(content.contains("startLine="),
"must hand back a resumable continuation cursor so the model can finish the read");
assertFalse(content.contains("middle omitted"),
"degradation must stay resumable — never a lossy middle-cut");
}
private static ResolvedSkill skill(String name, String source, boolean builtin) {
return ResolvedSkill.builder()
.id((long) name.hashCode())