diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java index 8f2554a2..7ea31c01 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolExecutionExecutor.java @@ -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 * * 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); } diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolResultStorage.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolResultStorage.java index dccd7ec3..cb85bd48 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolResultStorage.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/executor/ToolResultStorage.java @@ -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 diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java index 2f40ab6e..618ae10d 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java @@ -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. + * + *

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); diff --git a/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/LaneDExecutorAndConfigTest.java b/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/LaneDExecutorAndConfigTest.java index 29ba7ed1..5b82e3b4 100644 --- a/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/LaneDExecutorAndConfigTest.java +++ b/mateclaw-server/src/test/java/vip/mate/agent/graph/executor/LaneDExecutorAndConfigTest.java @@ -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 diff --git a/mateclaw-server/src/test/java/vip/mate/tool/builtin/SkillFileToolTest.java b/mateclaw-server/src/test/java/vip/mate/tool/builtin/SkillFileToolTest.java index 5dc01775..057c9d14 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/builtin/SkillFileToolTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/builtin/SkillFileToolTest.java @@ -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())