feat: built-in large tool output handling

[yunus25jmi1]

Summary

Add a new builtin hook that automatically handles large tool responses. When enabled, tool responses exceeding the threshold are saved to disk and replaced with a pointer that the agent can read back using shell tools.

This is a root-cause fix for issue #2722.

---

Problem

When an MCP tool returns a large response the agent breaks in one of two ways:

1. With default max_old_tool_call_tokens budget: A single large result eats the entire budget and all earlier tool results in the session get silently replaced with [content truncated].

2. With max_old_tool_call_tokens: -1: The full response stays in the session but overflows the model's context window. Auto-compaction can't help because the oversized result is in the most recent messages, so the agent gets stuck.

Worth noting: builtin tools (shell, openapi, api) each have a hardcoded 30,000-char per-result limit. MCP tools have no per-result limit at all.

---

Root Cause

MCP tools have no output limit, unlike builtin tools (shell, openapi, api) which all have a hardcoded 30,000-char limit. When MCP returns large responses, they flow into the session unmodified.

---

Solution

Added a new builtin hook handle_large_tool_output that:

1. Intercepts tool responses at EventToolResponseTransform
2. When response exceeds threshold, saves full output to disk
3. Replaces response with a pointer showing tool name, char count, file path, preview

Configuration

agents:
  root:
    handle_large_tool_output:
      enabled: true        # default: false
      threshold: 5000      # chars - default
      output_dir: /tmp     # default: os.TempDir()
      preview_size: 3000   # chars in preview - default

---

Files Changed

File	Change
agent-schema.json	Added handle_large_tool_output config
pkg/config/latest/types.go	Added HandleLargeToolOutputConfig struct
pkg/agent/agent.go	Added field and accessor
pkg/agent/opts.go	Added WithHandleLargeToolOutput option
pkg/teamloader/teamloader.go	Wired config to agent
pkg/hooks/builtins/handle_large_tool_output.go	New builtin hook
pkg/hooks/builtins/builtins.go	Registered builtin
pkg/runtime/hooks.go	Passed config to ApplyAgentDefaults

---

Testing

• Build: ✅ Passes
• Lint: ✅ 0 issues
• Docker Image: ✅ Builds

Fixes #2722

[aheritier]

Triage note: This is a community PR (contributor) implementing feature request #2722. The approach — a new handle_large_tool_output builtin hook at EventToolResponseTransform — is architecturally consistent with the existing redact_secrets pattern.

A few design questions for maintainer review before merging:

1. Opt-in vs. opt-out: The PR requires explicit YAML config. Should there be a sensible default threshold (e.g. the existing 30,000-char limit already on builtins)?
2. Config location: handle_large_tool_output is a top-level agent field vs. a hooks entry — breaks the hooks-as-the-extension-point pattern. Would hooks.tool_response_transform: [{type: builtin, command: handle_large_tool_output}] with agent-level options be more consistent?
3. Tests: The PR notes build/lint pass but no unit tests for the new builtin are mentioned — same bar as redact_secrets / unload would require test coverage.

Flagging help-wanted — good candidate for an iterative review with the contributor.

[yunus25jmi1]

Addressed all three triage concerns:

1. Threshold aligned to 30,000 — default threshold bumped from 5,000 to 30,000 to match the existing builtin tool limit (shell, openapi, api all cap at 30k chars). This is the sensible default you mentioned.

2. Config location — same pattern as redact_secrets — handle_large_tool_output is a top-level AgentConfig field, auto-injected into hooks.tool_response_transform via ApplyAgentDefaults. This mirrors redact_secrets exactly:

• AgentConfig.RedactSecrets (bool) → auto-injects 3 hooks
• AgentConfig.HandleLargeToolOutput (struct) → auto-injects 1 hook

Same abstraction, same mechanism. If the team wants to move to hooks.tool_response_transform: [{type: builtin, command: handle_large_tool_output}] with agent-level options, I'm happy to take that as a v2 refactor — it's a non-trivial change to the injection model and not a blocker for this PR.

3. Unit tests added — 13 tests covering:

• Under/over threshold behavior
• File write + pointer format
• Custom output_dir and preview_size
• Fallback to os.TempDir()
• No-op cases (nil input, non-string response, empty response, wrong event)
• Error propagation on write failure
• Builtin registration
• ApplyAgentDefaults injection behavior (enabled/disabled/nil)

Permissions hardened: 0o640 → 0o600 (tool output may contain sensitive data).

[yunus25jmi1]

Security fix pushed — Path traversal vulnerability resolved.

Added sanitizeFilename() to strip .. and / from SessionID and ToolUseID before path construction. An attacker controlling an MCP tool server could have injected ../ sequences to write files outside the configured output directory.

func sanitizeFilename(name string) string {
    name = strings.ReplaceAll(name, "..", "__")
    name = strings.ReplaceAll(name, "/", "_")
    name = strings.ReplaceAll(name, "\\", "_")
    return name
}

Added two tests:

• TestHandleLargeToolOutputPathTraversalIsBlocked — verifies malicious paths stay within output directory
• TestSanitizeFilename — covers edge cases (slashes, backslashes, path traversal sequences)

This brings the PR to 4 commits covering functionality, tests, threshold alignment, and security.

[aheritier]

/review

[docker-agent]

Assessment: 🔴 CRITICAL

1 confirmed high-severity bug (user config silently ignored) and 2 likely issues (filename collision, shell metacharacters in suggested command). See inline comments for details.

[docker-agent]

[HIGH] output_dir and preview_size config values are silently ignored due to JSON key mismatch

HandleLargeToolOutputConfig (in pkg/config/latest/types.go) uses snake_case JSON tags:

Threshold   int    `json:"threshold,omitempty"`
OutputDir   string `json:"output_dir,omitempty"`
PreviewSize int    `json:"preview_size,omitempty"`

But toolOutputConfig (used to unmarshal the config at runtime) has no JSON tags, so Go uses the PascalCase field names as keys ("Threshold", "OutputDir", "PreviewSize"):

type toolOutputConfig struct {
    Threshold   int
    OutputDir   string
    PreviewSize int
}

In ApplyAgentDefaults, the HandleLargeToolOutputConfig is marshalled to JSON and passed as a string arg — producing e.g. {"threshold":5000,"output_dir":"/my/dir","preview_size":2000}. When parseArgs unmarshals this into toolOutputConfig:

• "threshold" → matches Threshold ✅ (Go's case-insensitive match works here)
• "output_dir" → does NOT match OutputDir ❌ (underscore vs no-underscore; case-insensitivity can't bridge this)
• "preview_size" → does NOT match PreviewSize ❌ (same reason)

Result: Any output_dir or preview_size a user configures is silently dropped, and the hook always uses the hardcoded defaults (os.TempDir() and 3000 chars).

Fix: Add json struct tags to toolOutputConfig matching those on HandleLargeToolOutputConfig:

type toolOutputConfig struct {
    Threshold   int    `json:"threshold"`
    OutputDir   string `json:"output_dir"`
    PreviewSize int    `json:"preview_size"`
}

[docker-agent]

[MEDIUM] Filename collision risk when SessionID or ToolUseID is empty

The output filename is built as:

filename := fmt.Sprintf("%s_%s.txt",
    sanitizeFilename(in.SessionID),
    sanitizeFilename(in.ToolUseID))

If either SessionID or ToolUseID is empty (e.g. sanitizeFilename("") == ""), multiple tool calls can produce the same filename (e.g., _.txt or _<toolUseID>.txt). Each subsequent os.WriteFile call silently overwrites the previous content. When the agent later reads the file, it gets the last response only — the earlier ones are lost without any error.

Fix: Validate that both IDs are non-empty before constructing the filename, or incorporate a timestamp/random component to guarantee uniqueness:

if in.SessionID == "" || in.ToolUseID == "" {
    return nil, fmt.Errorf("handle_large_tool_output: missing session or tool-use ID")
}

[docker-agent]

[LOW] Shell metacharacters in SessionID/ToolUseID flow into the suggested cat command

The pointer message shown to the agent includes:

pointer := fmt.Sprintf("...[Use shell tool to read: cat %s]", path)

sanitizeFilename only strips .., /, and \ — it does not strip spaces, $, backticks, ;, |, &, or ()). If a SessionID or ToolUseID contains these characters, the generated cat <path> string will embed them literally. When the agent's shell tool executes the suggested command, those metacharacters are interpreted by the shell, potentially causing unintended execution.

Fix: Either quote the path in the suggested command, or expand sanitizeFilename to allowlist only safe characters:

// Option A: shell-quote the path in the message
pointer := fmt.Sprintf("...[Use shell tool to read: cat '%s']", path)

// Option B: allowlist in sanitizeFilename
func sanitizeFilename(name string) string {
    var b strings.Builder
    for _, r := range name {
        if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') ||
           (r >= '0' && r <= '9') || r == '-' || r == '.' {
            b.WriteRune(r)
        } else {
            b.WriteRune('_')
        }
    }
    return b.String()
}