enforce 128KB tool result ceiling unconditionally
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.
This commit is contained in:
parent
8973881c0a
commit
75f11a7dc9
|
|
@ -764,6 +764,7 @@ func (s *agent) SetSessionID(newID string) error {
|
||||||
const defaultContextLength = 128000
|
const defaultContextLength = 128000
|
||||||
const defaultToolResultMaxBytes = 131072
|
const defaultToolResultMaxBytes = 131072
|
||||||
|
|
||||||
|
|
||||||
// autoCompactLimit returns the token threshold for auto-compaction (75%).
|
// autoCompactLimit returns the token threshold for auto-compaction (75%).
|
||||||
func (s *agent) autoCompactLimit(ctx context.Context) int {
|
func (s *agent) autoCompactLimit(ctx context.Context) int {
|
||||||
ctxLen := s.runtime.Backend.ContextLength(ctx)
|
ctxLen := s.runtime.Backend.ContextLength(ctx)
|
||||||
|
|
@ -1200,7 +1201,6 @@ func (s *agent) executeTurn(ctx context.Context, input string) string {
|
||||||
Exec: s.runtime.Exec,
|
Exec: s.runtime.Exec,
|
||||||
ClassifyTool: s.runtime.ClassifyTool,
|
ClassifyTool: s.runtime.ClassifyTool,
|
||||||
ClassifyTier: s.runtime.ClassifyTier,
|
ClassifyTier: s.runtime.ClassifyTier,
|
||||||
ToolResultMaxBytes: s.runtime.toolResultMaxBytes(),
|
|
||||||
GenerationParams: s.runtime.GenParams,
|
GenerationParams: s.runtime.GenParams,
|
||||||
MaxSteps: s.runtime.MaxSteps,
|
MaxSteps: s.runtime.MaxSteps,
|
||||||
ReadPlanStep: s.readPlanStep,
|
ReadPlanStep: s.readPlanStep,
|
||||||
|
|
|
||||||
|
|
@ -49,7 +49,6 @@ type agentConfig struct {
|
||||||
Exec toolExecutor
|
Exec toolExecutor
|
||||||
ClassifyTool func(name string) bool // nil=treat all as serial; true=parallel-read-safe
|
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
|
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
|
Output EventHandler
|
||||||
preamble string // compiled system+agent prompt sent as the system role
|
preamble string // compiled system+agent prompt sent as the system role
|
||||||
GenerationParams backend.GenerationParams
|
GenerationParams backend.GenerationParams
|
||||||
|
|
@ -307,13 +306,23 @@ func run(ctx context.Context, cfg agentConfig, state state) error {
|
||||||
var isErr bool
|
var isErr bool
|
||||||
streamed := false
|
streamed := false
|
||||||
if cfg.Exec != nil {
|
if cfg.Exec != nil {
|
||||||
streamCtx := tools.WithOutputStream(ctx, func(data string) {
|
streamBytes := 0
|
||||||
if !streamed {
|
streamCtx := tools.WithOutputStream(ctx, func(data string) {
|
||||||
emit(cfg, Event{Role: "tool", Name: tc.Name})
|
if streamBytes >= defaultToolResultMaxBytes {
|
||||||
streamed = true
|
return // already at ceiling, drop further chunks
|
||||||
}
|
}
|
||||||
emit(cfg, Event{Role: "tool", Name: tc.Name, Content: data})
|
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)
|
out, blocks, err := cfg.Exec(streamCtx, tc.Name, tc.Arguments)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
isErr = true
|
isErr = true
|
||||||
|
|
@ -352,11 +361,12 @@ func run(ctx context.Context, cfg agentConfig, state state) error {
|
||||||
result += "\n\n<system-user-interruption>\n" + injected + "\n</system-user-interruption>"
|
result += "\n\n<system-user-interruption>\n" + injected + "\n</system-user-interruption>"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
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)
|
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.]",
|
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 {
|
if readSafe && !isErr {
|
||||||
resultCache.Store(tc.Name+"\x00"+string(tc.Arguments), result)
|
resultCache.Store(tc.Name+"\x00"+string(tc.Arguments), result)
|
||||||
|
|
|
||||||
|
|
@ -10,10 +10,9 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
// TestTruncation_LargeResultTruncated verifies that a tool result exceeding
|
// 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) {
|
func TestTruncation_LargeResultTruncated(t *testing.T) {
|
||||||
const maxBytes = 64
|
large := strings.Repeat("x", defaultToolResultMaxBytes+10_000)
|
||||||
large := strings.Repeat("x", 200)
|
|
||||||
|
|
||||||
be := defaultBE()
|
be := defaultBE()
|
||||||
be.respond = toolsStream([]backend.ToolCall{
|
be.respond = toolsStream([]backend.ToolCall{
|
||||||
|
|
@ -21,10 +20,8 @@ func TestTruncation_LargeResultTruncated(t *testing.T) {
|
||||||
})
|
})
|
||||||
|
|
||||||
c := newCore(t, be, nil)
|
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) {
|
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")
|
evs := collectEvents(context.Background(), c, "big result")
|
||||||
|
|
@ -34,14 +31,14 @@ return large, nil, nil
|
||||||
t.Fatalf("tool events = %d; want 1", len(toolEvs))
|
t.Fatalf("tool events = %d; want 1", len(toolEvs))
|
||||||
}
|
}
|
||||||
result := toolEvs[0]
|
result := toolEvs[0]
|
||||||
if !strings.HasPrefix(result, strings.Repeat("x", maxBytes)) {
|
if !strings.HasPrefix(result, strings.Repeat("x", defaultToolResultMaxBytes)) {
|
||||||
t.Errorf("result does not start with %d x's: %q", maxBytes, result[:min(len(result), 80)])
|
t.Errorf("result does not start with %d x's", defaultToolResultMaxBytes)
|
||||||
}
|
}
|
||||||
if !strings.Contains(result, "HARD LIMIT") {
|
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") {
|
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 := newCore(t, be, nil)
|
||||||
c.runtime.ToolResultMaxBytes = 8192
|
|
||||||
c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) {
|
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")
|
evs := collectEvents(context.Background(), c, "small result")
|
||||||
|
|
@ -70,10 +66,10 @@ return "short result", nil, nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestTruncation_ZeroDisabled verifies that ToolResultMaxBytes=0 disables
|
// TestTruncation_ErrorTruncated verifies that error results are subject to the
|
||||||
// truncation entirely regardless of result size.
|
// same 128KB safety limit (the original bug: errors bypassed all truncation).
|
||||||
func TestTruncation_ZeroDisabled(t *testing.T) {
|
func TestTruncation_ErrorTruncated(t *testing.T) {
|
||||||
large := strings.Repeat("y", 100_000)
|
longErr := strings.Repeat("e", defaultToolResultMaxBytes+10_000)
|
||||||
|
|
||||||
be := defaultBE()
|
be := defaultBE()
|
||||||
be.respond = toolsStream([]backend.ToolCall{
|
be.respond = toolsStream([]backend.ToolCall{
|
||||||
|
|
@ -81,35 +77,6 @@ func TestTruncation_ZeroDisabled(t *testing.T) {
|
||||||
})
|
})
|
||||||
|
|
||||||
c := newCore(t, be, nil)
|
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) {
|
c.runtime.Exec = func(_ context.Context, _ string, _ json.RawMessage) (string, []backend.ContentBlock, error) {
|
||||||
return "", nil, &mockErr{longErr}
|
return "", nil, &mockErr{longErr}
|
||||||
}
|
}
|
||||||
|
|
@ -120,16 +87,18 @@ func TestTruncation_ErrorNotTruncated(t *testing.T) {
|
||||||
if len(toolEvs) != 1 {
|
if len(toolEvs) != 1 {
|
||||||
t.Fatalf("tool events = %d; want 1", len(toolEvs))
|
t.Fatalf("tool events = %d; want 1", len(toolEvs))
|
||||||
}
|
}
|
||||||
if strings.Contains(toolEvs[0], "HARD LIMIT") {
|
if !strings.Contains(toolEvs[0], "HARD LIMIT") {
|
||||||
t.Errorf("error result was truncated; should be preserved in full")
|
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
|
// TestTruncation_CachedResultAlreadyTruncated verifies that a cache hit on a
|
||||||
// previously-truncated result returns the truncated form, not the original.
|
// previously-truncated result returns the truncated form, not the original.
|
||||||
func TestTruncation_CachedResultAlreadyTruncated(t *testing.T) {
|
func TestTruncation_CachedResultAlreadyTruncated(t *testing.T) {
|
||||||
const maxBytes = 64
|
large := strings.Repeat("z", defaultToolResultMaxBytes+10_000)
|
||||||
large := strings.Repeat("z", 200)
|
|
||||||
|
|
||||||
be := defaultBE()
|
be := defaultBE()
|
||||||
be.respond = multiTurnToolsStream([][]backend.ToolCall{
|
be.respond = multiTurnToolsStream([][]backend.ToolCall{
|
||||||
|
|
@ -139,9 +108,8 @@ func TestTruncation_CachedResultAlreadyTruncated(t *testing.T) {
|
||||||
|
|
||||||
c := newCore(t, be, nil)
|
c := newCore(t, be, nil)
|
||||||
c.runtime.ClassifyTool = func(string) bool { return true } // read-safe → cacheable
|
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) {
|
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")
|
evs := collectEvents(context.Background(), c, "cached truncated")
|
||||||
|
|
@ -150,11 +118,9 @@ return large, nil, nil
|
||||||
if len(toolEvs) != 2 {
|
if len(toolEvs) != 2 {
|
||||||
t.Fatalf("tool events = %d; want 2", len(toolEvs))
|
t.Fatalf("tool events = %d; want 2", len(toolEvs))
|
||||||
}
|
}
|
||||||
// Both should be the truncated form.
|
|
||||||
for i, ev := range toolEvs {
|
for i, ev := range toolEvs {
|
||||||
if !strings.Contains(ev, "HARD LIMIT") {
|
if !strings.Contains(ev, "HARD LIMIT") {
|
||||||
t.Errorf("event[%d] missing truncation hint: %q", i, ev[:min(len(ev), 80)])
|
t.Errorf("event[%d] missing truncation hint: %q", i, ev[:min(len(ev), 80)])
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -7,12 +7,6 @@ import (
|
||||||
"ollie/pkg/tools"
|
"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
|
// Runtime holds the swappable per-agent configuration. It contains everything
|
||||||
// that changes on an /agent switch but is stable across turns within the same
|
// 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
|
ClassifyTier func(string, json.RawMessage) ResultTier
|
||||||
GenParams backend.GenerationParams
|
GenParams backend.GenerationParams
|
||||||
MaxSteps int
|
MaxSteps int
|
||||||
ToolResultMaxBytes int // 0 means use defaultToolResultMaxBytes
|
|
||||||
// CfgBackend and CfgModel are the backend/model overrides from the agent
|
// CfgBackend and CfgModel are the backend/model overrides from the agent
|
||||||
// config JSON. Empty means no override. Applied by the caller after
|
// config JSON. Empty means no override. Applied by the caller after
|
||||||
// BuildRuntime returns.
|
// BuildRuntime returns.
|
||||||
|
|
|
||||||
|
|
@ -570,7 +570,13 @@ func (e *Server) executeElevated(ctx context.Context, cmd, dir string, timeout i
|
||||||
if combined == "" {
|
if combined == "" {
|
||||||
return "", fmt.Errorf("elevated execution failed (exit %d)", exitCode)
|
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
|
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)
|
return "", fmt.Errorf("execution timeout after %d seconds", timeout)
|
||||||
}
|
}
|
||||||
if err != nil {
|
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
|
return string(output), nil
|
||||||
|
|
||||||
|
|
|
||||||
Reference in New Issue