From 75f11a7dc933e45812e25d994f6e6a3bcf6eec2d Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Mon, 27 Jul 2026 13:02:45 +0200 Subject: [PATCH] enforce 128KB tool result ceiling unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tool result truncation had two bypasses: 1. Error results were exempt (!isErr condition) — a command returning output in its error message could flood the context with megabytes of untruncated data. 2. The streaming callback emitted chunks to the event handler (and thus D-Bus signals) with no size check, flooding the bus even when the final stored result would be truncated. Fix: - Remove the configurable ToolResultMaxBytes field; use the constant defaultToolResultMaxBytes (128KB) unconditionally for all results regardless of success/error status. - Cap the streaming callback at 128KB so chunks stop being emitted once the ceiling is reached. - Cap output embedded in execute server error messages to 8KB as defense in depth. --- pkg/agent/core.go | 2 +- pkg/agent/loop.go | 32 ++++++++++----- pkg/agent/loop_truncate_test.go | 72 +++++++++------------------------ pkg/agent/runtime.go | 7 ---- pkg/tools/execute/server.go | 17 +++++++- 5 files changed, 56 insertions(+), 74 deletions(-) diff --git a/pkg/agent/core.go b/pkg/agent/core.go index c24738d..1e19dcb 100644 --- a/pkg/agent/core.go +++ b/pkg/agent/core.go @@ -764,6 +764,7 @@ func (s *agent) SetSessionID(newID string) error { const defaultContextLength = 128000 const defaultToolResultMaxBytes = 131072 + // autoCompactLimit returns the token threshold for auto-compaction (75%). func (s *agent) autoCompactLimit(ctx context.Context) int { ctxLen := s.runtime.Backend.ContextLength(ctx) @@ -1200,7 +1201,6 @@ func (s *agent) executeTurn(ctx context.Context, input string) string { Exec: s.runtime.Exec, ClassifyTool: s.runtime.ClassifyTool, ClassifyTier: s.runtime.ClassifyTier, - ToolResultMaxBytes: s.runtime.toolResultMaxBytes(), GenerationParams: s.runtime.GenParams, MaxSteps: s.runtime.MaxSteps, ReadPlanStep: s.readPlanStep, diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 99d838a..b3655b9 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -49,7 +49,6 @@ type agentConfig struct { Exec toolExecutor ClassifyTool func(name string) bool // nil=treat all as serial; true=parallel-read-safe ClassifyTier func(name string, args json.RawMessage) ResultTier // nil=TierHot for all; tools self-classify retention - ToolResultMaxBytes int // 0=unlimited; truncate tool results beyond this size Output EventHandler preamble string // compiled system+agent prompt sent as the system role GenerationParams backend.GenerationParams @@ -307,13 +306,23 @@ func run(ctx context.Context, cfg agentConfig, state state) error { var isErr bool streamed := false if cfg.Exec != nil { - streamCtx := tools.WithOutputStream(ctx, func(data string) { - if !streamed { - emit(cfg, Event{Role: "tool", Name: tc.Name}) - streamed = true - } - emit(cfg, Event{Role: "tool", Name: tc.Name, Content: data}) - }) + streamBytes := 0 + streamCtx := tools.WithOutputStream(ctx, func(data string) { + if streamBytes >= defaultToolResultMaxBytes { + return // already at ceiling, drop further chunks + } + if !streamed { + emit(cfg, Event{Role: "tool", Name: tc.Name}) + streamed = true + } + streamBytes += len(data) + if streamBytes > defaultToolResultMaxBytes { + // Emit only the portion within the ceiling. + excess := streamBytes - defaultToolResultMaxBytes + data = data[:len(data)-excess] + } + emit(cfg, Event{Role: "tool", Name: tc.Name, Content: data}) + }) out, blocks, err := cfg.Exec(streamCtx, tc.Name, tc.Arguments) if err != nil { isErr = true @@ -352,11 +361,12 @@ func run(ctx context.Context, cfg agentConfig, state state) error { result += "\n\n\n" + injected + "\n" } } - if !isErr && cfg.ToolResultMaxBytes > 0 && len(result) > cfg.ToolResultMaxBytes { + // Safety ceiling: unconditionally cap all tool results at 128KB. + if len(result) > defaultToolResultMaxBytes { orig := len(result) - result = strings.ToValidUTF8(result[:cfg.ToolResultMaxBytes], "") + result = strings.ToValidUTF8(result[:defaultToolResultMaxBytes], "") result += fmt.Sprintf("\n\n[HARD LIMIT: %s output truncated — %d of %d bytes shown. This is a safety ceiling, not a semantic boundary.]", - tc.Name, cfg.ToolResultMaxBytes, orig) + tc.Name, defaultToolResultMaxBytes, orig) } if readSafe && !isErr { resultCache.Store(tc.Name+"\x00"+string(tc.Arguments), result) diff --git a/pkg/agent/loop_truncate_test.go b/pkg/agent/loop_truncate_test.go index 196889f..c9d62c1 100644 --- a/pkg/agent/loop_truncate_test.go +++ b/pkg/agent/loop_truncate_test.go @@ -10,10 +10,9 @@ import ( ) // TestTruncation_LargeResultTruncated verifies that a tool result exceeding -// ToolResultMaxBytes is truncated and a continuation hint is appended. +// the 128KB safety limit is truncated and a hint is appended. func TestTruncation_LargeResultTruncated(t *testing.T) { - const maxBytes = 64 - large := strings.Repeat("x", 200) + large := strings.Repeat("x", defaultToolResultMaxBytes+10_000) be := defaultBE() be.respond = toolsStream([]backend.ToolCall{ @@ -21,10 +20,8 @@ func TestTruncation_LargeResultTruncated(t *testing.T) { }) c := newCore(t, be, nil) - c.runtime.ClassifyTool = nil - c.runtime.ToolResultMaxBytes = maxBytes c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) { -return large, nil, nil + return large, nil, nil } evs := collectEvents(context.Background(), c, "big result") @@ -34,14 +31,14 @@ return large, nil, nil t.Fatalf("tool events = %d; want 1", len(toolEvs)) } result := toolEvs[0] - if !strings.HasPrefix(result, strings.Repeat("x", maxBytes)) { - t.Errorf("result does not start with %d x's: %q", maxBytes, result[:min(len(result), 80)]) + if !strings.HasPrefix(result, strings.Repeat("x", defaultToolResultMaxBytes)) { + t.Errorf("result does not start with %d x's", defaultToolResultMaxBytes) } if !strings.Contains(result, "HARD LIMIT") { - t.Errorf("truncation hint missing from result: %q", result) + t.Errorf("truncation hint missing from result: %q", result[:min(len(result), 80)]) } if !strings.Contains(result, "execute_code") { - t.Errorf("tool name missing from truncation hint: %q", result) + t.Errorf("tool name missing from truncation hint: %q", result[:min(len(result), 120)]) } } @@ -54,9 +51,8 @@ func TestTruncation_SmallResultNotTruncated(t *testing.T) { }) c := newCore(t, be, nil) - c.runtime.ToolResultMaxBytes = 8192 c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) { -return "short result", nil, nil + return "short result", nil, nil } evs := collectEvents(context.Background(), c, "small result") @@ -70,10 +66,10 @@ return "short result", nil, nil } } -// TestTruncation_ZeroDisabled verifies that ToolResultMaxBytes=0 disables -// truncation entirely regardless of result size. -func TestTruncation_ZeroDisabled(t *testing.T) { - large := strings.Repeat("y", 100_000) +// TestTruncation_ErrorTruncated verifies that error results are subject to the +// same 128KB safety limit (the original bug: errors bypassed all truncation). +func TestTruncation_ErrorTruncated(t *testing.T) { + longErr := strings.Repeat("e", defaultToolResultMaxBytes+10_000) be := defaultBE() be.respond = toolsStream([]backend.ToolCall{ @@ -81,35 +77,6 @@ func TestTruncation_ZeroDisabled(t *testing.T) { }) c := newCore(t, be, nil) - c.runtime.ToolResultMaxBytes = 0 // disabled - c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) { -return large, nil, nil - } - - evs := collectEvents(context.Background(), c, "no limit") - - toolEvs := byRole(evs, "tool") - if len(toolEvs) != 1 { - t.Fatalf("tool events = %d; want 1", len(toolEvs)) - } - if toolEvs[0] != large { - t.Errorf("result was truncated when limit=0 (len=%d, want %d)", len(toolEvs[0]), len(large)) - } -} - -// TestTruncation_ErrorNotTruncated verifies that error results are never -// truncated (the model needs the full error message). -func TestTruncation_ErrorNotTruncated(t *testing.T) { - const maxBytes = 10 - longErr := strings.Repeat("e", 200) - - be := defaultBE() - be.respond = toolsStream([]backend.ToolCall{ - {ID: "1", Name: "execute_code", Arguments: json.RawMessage(`{}`)}, - }) - - c := newCore(t, be, nil) - c.runtime.ToolResultMaxBytes = maxBytes c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) { return "", nil, &mockErr{longErr} } @@ -120,16 +87,18 @@ func TestTruncation_ErrorNotTruncated(t *testing.T) { if len(toolEvs) != 1 { t.Fatalf("tool events = %d; want 1", len(toolEvs)) } - if strings.Contains(toolEvs[0], "HARD LIMIT") { - t.Errorf("error result was truncated; should be preserved in full") + if !strings.Contains(toolEvs[0], "HARD LIMIT") { + t.Errorf("error result was NOT truncated: len=%d", len(toolEvs[0])) + } + if len(toolEvs[0]) > defaultToolResultMaxBytes+200 { + t.Errorf("error result too large after truncation: len=%d", len(toolEvs[0])) } } // TestTruncation_CachedResultAlreadyTruncated verifies that a cache hit on a // previously-truncated result returns the truncated form, not the original. func TestTruncation_CachedResultAlreadyTruncated(t *testing.T) { - const maxBytes = 64 - large := strings.Repeat("z", 200) + large := strings.Repeat("z", defaultToolResultMaxBytes+10_000) be := defaultBE() be.respond = multiTurnToolsStream([][]backend.ToolCall{ @@ -139,9 +108,8 @@ func TestTruncation_CachedResultAlreadyTruncated(t *testing.T) { c := newCore(t, be, nil) c.runtime.ClassifyTool = func(string) bool { return true } // read-safe → cacheable - c.runtime.ToolResultMaxBytes = maxBytes c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) { -return large, nil, nil + return large, nil, nil } evs := collectEvents(context.Background(), c, "cached truncated") @@ -150,11 +118,9 @@ return large, nil, nil if len(toolEvs) != 2 { t.Fatalf("tool events = %d; want 2", len(toolEvs)) } - // Both should be the truncated form. for i, ev := range toolEvs { if !strings.Contains(ev, "HARD LIMIT") { t.Errorf("event[%d] missing truncation hint: %q", i, ev[:min(len(ev), 80)]) } } } - diff --git a/pkg/agent/runtime.go b/pkg/agent/runtime.go index 7e64629..bd6417d 100644 --- a/pkg/agent/runtime.go +++ b/pkg/agent/runtime.go @@ -7,12 +7,6 @@ import ( "ollie/pkg/tools" ) -func (r *Runtime) toolResultMaxBytes() int { - if r.ToolResultMaxBytes > 0 { - return r.ToolResultMaxBytes - } - return defaultToolResultMaxBytes -} // Runtime holds the swappable per-agent configuration. It contains everything // that changes on an /agent switch but is stable across turns within the same @@ -29,7 +23,6 @@ type Runtime struct { ClassifyTier func(string, json.RawMessage) ResultTier GenParams backend.GenerationParams MaxSteps int - ToolResultMaxBytes int // 0 means use defaultToolResultMaxBytes // CfgBackend and CfgModel are the backend/model overrides from the agent // config JSON. Empty means no override. Applied by the caller after // BuildRuntime returns. diff --git a/pkg/tools/execute/server.go b/pkg/tools/execute/server.go index 5308eeb..bd284d8 100644 --- a/pkg/tools/execute/server.go +++ b/pkg/tools/execute/server.go @@ -570,7 +570,13 @@ func (e *Server) executeElevated(ctx context.Context, cmd, dir string, timeout i if combined == "" { return "", fmt.Errorf("elevated execution failed (exit %d)", exitCode) } - return combined, fmt.Errorf("elevated execution failed (exit %d)\nOutput: %s", exitCode, combined) + // Cap output in error message to avoid flooding the agent context. + errOutput := combined + const maxErrOutput = 8192 + if len(errOutput) > maxErrOutput { + errOutput = errOutput[:maxErrOutput] + fmt.Sprintf("\n[...truncated %d bytes in error]", len(combined)-maxErrOutput) + } + return combined, fmt.Errorf("elevated execution failed (exit %d)\nOutput: %s", exitCode, errOutput) } return combined, nil } @@ -801,7 +807,14 @@ func (e *Server) executeWithStdin(ctx context.Context, code, language string, ti return "", fmt.Errorf("execution timeout after %d seconds", timeout) } if err != nil { - return string(output), fmt.Errorf("execution failed: %v\nOutput: %s", err, string(output)) + // Cap output in error message to avoid flooding the agent context. + // The full output is still returned as the first return value. + errOutput := string(output) + const maxErrOutput = 8192 + if len(errOutput) > maxErrOutput { + errOutput = errOutput[:maxErrOutput] + fmt.Sprintf("\n[...truncated %d bytes in error]", len(output)-maxErrOutput) + } + return string(output), fmt.Errorf("execution failed: %v\nOutput: %s", err, errOutput) } return string(output), nil