Add tool pkg

internal/tool/tool.go

@@ -0,0 +1,409 @@
+// Package tool provides configured subprocess tools.
+package tool
+
+import (
+	"bytes"
+	"context"
+	jsonv2 "encoding/json/v2"
+	"errors"
+	"fmt"
+	"net/http"
+	"os"
+	"os/exec"
+	"strings"
+	"text/template"
+	"time"
+
+	"code.chimeric.al/chimerical/odidere/internal/config"
+	"code.chimeric.al/chimerical/odidere/internal/llm"
+)
+
+// errToolTimeout marks a timeout caused by the tool's configured limit.
+var errToolTimeout = errors.New("tool timeout")
+
+// fn provides template functions available to argument templates.
+var fn = template.FuncMap{
+	"json": func(v any) string {
+		b, _ := jsonv2.Marshal(v)
+		return string(b)
+	},
+}
+
+// Tool represents an external tool that can be invoked by LLMs.
+// Tools are executed as subprocesses with templated arguments,
+// and described to the LLM via their JSON Schema parameters.
+type Tool struct {
+	// Name uniquely identifies the tool within a registry.
+	Name string
+	// Description explains the tool's purpose for the LLM.
+	Description string
+	// Command is the executable path or name.
+	Command string
+	// Arguments contains the templates used to construct the subprocess
+	// arguments. See config.ToolConfig.Arguments for examples, behavior, and
+	// limitations.
+	Arguments []string
+	// tmpl holds Arguments compiled to executable templates, one per
+	// element, in the same order. Only NewTool populates it; on a Tool
+	// built any other way it is nil, and ParseArguments returns no
+	// arguments rather than an error.
+	tmpl []*template.Template
+	// Parameters describes the tool's accepted input to the LLM. It is not
+	// used to validate a tool call before its arguments are rendered. See
+	// config.ToolConfig.Parameters.
+	Parameters map[string]any
+	// timeout is the parsed execution duration limit.
+	timeout time.Duration
+}
+
+// NewTool creates a Tool from a ToolConfig. It compiles the argument
+// templates so that a template syntax error is reported here, at
+// construction, rather than on the tool's first execution.
+func NewTool(cfg config.ToolConfig) (*Tool, error) {
+	templates := make([]*template.Template, len(cfg.Arguments))
+	for i, arg := range cfg.Arguments {
+		tmpl, err := template.New("").Funcs(fn).Parse(arg)
+		if err != nil {
+			return nil, fmt.Errorf(
+				"invalid template %q: %w", arg, err,
+			)
+		}
+		templates[i] = tmpl
+	}
+
+	return &Tool{
+		Name:        cfg.Name,
+		Description: cfg.Description,
+		Command:     cfg.Command,
+		Arguments:   cfg.Arguments,
+		Parameters:  cfg.Parameters,
+		tmpl:        templates,
+		timeout:     cfg.Timeout,
+	}, nil
+}
+
+// ParseArguments converts tool-call arguments into program arguments. The
+// input is the JSON object supplied by the LLM when it calls the tool. The
+// returned slice is passed to the external program after the tool's Command;
+// it does not include the command name itself.
+//
+// An empty string or JSON null is treated as an empty object. ParseArguments
+// returns an error for any other input that is not a single valid JSON
+// object, or if an argument template cannot be evaluated. A template that
+// fails to parse is an error of NewTool, reported when the tool is
+// constructed.
+//
+// JSON numbers are decoded as float64, which is encoding/json/v2's default
+// for values decoded into an interface. Integers outside the IEEE 754 safe
+// range (±2^53) do not survive this round trip: they are rounded and
+// rendered in floating-point form (9007199254740993 becomes
+// 9.007199254740992e+15). Tool parameters whose values may exceed that range
+// should be declared as strings in the tool's Parameters.
+//
+// See config.ToolConfig.Arguments for template behavior and limitations.
+func (t *Tool) ParseArguments(args string) ([]string, error) {
+	var data = map[string]any{}
+	if args != "" && args != "{}" {
+		if err := jsonv2.Unmarshal([]byte(args), &data); err != nil {
+			return nil, fmt.Errorf(
+				"invalid arguments JSON: %w", err,
+			)
+		}
+	}
+
+	var result []string
+	for i, tmpl := range t.tmpl {
+		var buf bytes.Buffer
+		if err := tmpl.Execute(&buf, data); err != nil {
+			return nil, fmt.Errorf(
+				"execute template %q: %w", t.Arguments[i], err,
+			)
+		}
+
+		// Filter out empty strings (unused conditional arguments).
+		if s := buf.String(); s != "" {
+			result = append(result, s)
+		}
+	}
+
+	return result, nil
+}
+
+// ToAPI converts the tool definition into the API format suitable for
+// inclusion in a ChatRequest.
+func (t *Tool) ToAPI() llm.APITool {
+	return llm.APITool{
+		Type: llm.ToolTypeFunction,
+		Function: &llm.FunctionDef{
+			Name:        t.Name,
+			Description: t.Description,
+			Parameters:  t.Parameters,
+		},
+	}
+}
+
+// Registry holds tools, indexed by name, and handles their execution.
+type Registry struct {
+	tools map[string]*Tool
+}
+
+// NewRegistry creates a registry from the provided tool definitions.
+// It returns an error if a tool cannot be constructed or a name is duplicated.
+func NewRegistry(tools []config.ToolConfig) (*Registry, error) {
+	var r = &Registry{
+		tools: make(map[string]*Tool),
+	}
+	for _, tc := range tools {
+		t, err := NewTool(tc)
+		if err != nil {
+			return nil, fmt.Errorf(
+				"invalid tool %q: %w", tc.Name, err,
+			)
+		}
+
+		if _, exists := r.tools[t.Name]; exists {
+			return nil, fmt.Errorf(
+				"duplicate tool name: %s", t.Name,
+			)
+		}
+		r.tools[t.Name] = t
+	}
+
+	return r, nil
+}
+
+// Get returns a tool by name and a boolean indicating if it was found.
+func (r *Registry) Get(name string) (*Tool, bool) {
+	tool, ok := r.tools[name]
+	return tool, ok
+}
+
+// List returns all registered tool names in arbitrary order.
+func (r *Registry) List() []string {
+	names := make([]string, 0, len(r.tools))
+	for name := range r.tools {
+		names = append(names, name)
+	}
+	return names
+}
+
+// ToAPI converts all registered tools into API format suitable for
+// inclusion in a ChatRequest.
+func (r *Registry) ToAPI() []llm.APITool {
+	if r == nil {
+		return nil
+	}
+	tools := make([]llm.APITool, 0, len(r.tools))
+	for _, t := range r.tools {
+		tools = append(tools, t.ToAPI())
+	}
+	return tools
+}
+
+// binaryOutputPrefix is the message shown when tool output is detected as binary.
+const binaryOutputPrefix = "error: binary output detected: "
+
+// ToolResult holds the structured output from a tool execution.
+type ToolResult struct {
+	// FinishedAt is the wall-clock time at which the command stopped
+	// running, including the wait for its output pipes to drain after it
+	// exits.
+	FinishedAt time.Time
+	// ProcessState describes how the process ended: its exit code, or the
+	// signal that killed it.
+	ProcessState *os.ProcessState
+	// StartedAt is the wall-clock time at which the command began running.
+	StartedAt time.Time
+	// Stderr holds the bytes the command wrote to standard error.
+	Stderr []byte
+	// Stdout holds the bytes the command wrote to standard output.
+	Stdout []byte
+}
+
+// FormatResult formats the tool result into separate sections for stdout and
+// stderr, and other metadata.
+//
+// Output sections appear even when empty. A missing trailing newline is
+// added after captured output. The exit section contains the process exit
+// code; if it is -1, the section also includes the process-state
+// description.
+//
+// A section contains the captured bytes only when FormatResult classifies
+// them as text; otherwise, it contains a single notice line naming the
+// detected type. Classification uses a MIME type, inferred from the
+// stream's leading bytes: any type beginning with "text/" counts as text.
+// This classification can be wrong in both directions; see
+// [http.DetectContentType] for the rules it follows and
+// https://mimesniff.spec.whatwg.org/ for the algorithm it implements.
+func (r *ToolResult) FormatResult() string {
+	var buf strings.Builder
+
+	fmt.Fprintln(&buf, "▸ stdout:")
+	if len(r.Stdout) > 0 {
+		ctype := http.DetectContentType(r.Stdout)
+		if !strings.HasPrefix(ctype, "text/") {
+			fmt.Fprintf(&buf, binaryOutputPrefix+"%s\n", ctype)
+		} else {
+			buf.Write(r.Stdout)
+			if r.Stdout[len(r.Stdout)-1] != '\n' {
+				fmt.Fprint(&buf, "\n")
+			}
+		}
+	}
+
+	fmt.Fprintln(&buf, "▸ stderr:")
+	if len(r.Stderr) > 0 {
+		ctype := http.DetectContentType(r.Stderr)
+		if !strings.HasPrefix(ctype, "text/") {
+			fmt.Fprintf(&buf, binaryOutputPrefix+"%s\n", ctype)
+		} else {
+			buf.Write(r.Stderr)
+			if r.Stderr[len(r.Stderr)-1] != '\n' {
+				fmt.Fprint(&buf, "\n")
+			}
+		}
+	}
+
+	fmt.Fprintf(&buf, "┗ start: %s\n", r.StartedAt.Format(time.RFC3339Nano))
+	fmt.Fprintf(&buf, "┗ end: %s\n", r.FinishedAt.Format(time.RFC3339Nano))
+	fmt.Fprintf(&buf, "┗ duration: %s\n",
+		r.FinishedAt.Sub(r.StartedAt).Round(time.Millisecond))
+
+	exitCode := r.ProcessState.ExitCode()
+	fmt.Fprintf(&buf, "┗ exit: %d", exitCode)
+	// Include signal info when the process was killed by a signal.
+	if exitCode == -1 {
+		fmt.Fprintf(&buf, " (%s)", r.ProcessState.String())
+	}
+	fmt.Fprintln(&buf)
+
+	return buf.String()
+}
+
+// Execute runs a tool by name with the provided JSON arguments.
+// It expands argument templates, executes the command as a subprocess, and
+// returns a structured result with stdout, stderr, and exit code.
+// The context can be used for cancellation; tool-specific timeouts are
+// applied on top of any context deadline.
+//
+// On timeout, context cancellation, or [exec.ErrWaitDelay], Execute may
+// return both a non-nil ToolResult with partial output and a non-nil error.
+// The ToolResult is nil only for start failures where the process never ran.
+func (r *Registry) Execute(
+	ctx context.Context, name string, args string,
+) (*ToolResult, error) {
+	tool, ok := r.tools[name]
+	if !ok {
+		return nil, fmt.Errorf("unknown tool: %q", name)
+	}
+
+	// Evaluate argument templates.
+	cmdArgs, err := tool.ParseArguments(args)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"tool %q: parse arguments: %w", name, err,
+		)
+	}
+
+	// If defined, use the timeout. WithTimeoutCause attaches errToolTimeout
+	// as the context cause so we can distinguish it from a parent deadline.
+	if tool.timeout > 0 {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeoutCause(
+			ctx, tool.timeout, errToolTimeout,
+		)
+		defer cancel()
+	}
+
+	// Setup and run the command.
+	var (
+		stdout, stderr bytes.Buffer
+		cmd            = exec.CommandContext(
+			ctx, tool.Command, cmdArgs...,
+		)
+	)
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+	// Bound pipe draining after cancellation or process exit; descendants
+	// may retain output descriptors. This does not terminate descendants.
+	// https://pkg.go.dev/os/exec#Cmd.WaitDelay
+	cmd.WaitDelay = time.Second
+
+	// After cmd.Run() returns, the possible error cases are:
+	//
+	// 1. Context error (ctx.Err() != nil): the context was cancelled or
+	//    timed out. If the process had started, it was killed and
+	//    ProcessState is set. If it had not, ProcessState is nil.
+	//
+	// 2. Start failure (cmd.ProcessState == nil): the executable was not
+	//    found, the fork failed, or the context was already done before
+	//    Start. No stdout/stderr to capture.
+	//
+	// 3. Process exited with error (err is *exec.ExitError): the process
+	//    ran but exited with a non-zero status or was killed by a signal.
+	//    ProcessState is set and contains the exit code.
+	//
+	// In all cases where the process started, ProcessState is guaranteed
+	// to be set because Start() succeeded.
+	started := time.Now()
+	err = cmd.Run()
+	finished := time.Now()
+	if err != nil {
+		// Build the result only if the process started. A non-nil result
+		// always has a non-nil ProcessState.
+		var result *ToolResult
+		if cmd.ProcessState != nil {
+			result = &ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmd.ProcessState,
+				StartedAt:    started,
+				Stderr:       stderr.Bytes(),
+				Stdout:       stdout.Bytes(),
+			}
+		}
+
+		switch {
+		// Context was cancelled or timed out. If the process started,
+		// result contains partial output. If it never started, result
+		// is nil. Distinguish tool timeout from other context errors
+		// so we can return an appropriate message.
+		case ctx.Err() != nil:
+			if errors.Is(context.Cause(ctx), errToolTimeout) {
+				return result, fmt.Errorf(
+					"tool %q: timed out after %v: %w",
+					name, tool.timeout, errToolTimeout,
+				)
+			}
+			return result, fmt.Errorf("tool %q: %w", name, ctx.Err())
+
+		// Process never started, and it wasn't a context error.
+		// The error is the raw start failure (e.g., *PathError).
+		case result == nil:
+			return nil, fmt.Errorf("tool %q: %w", name, err)
+
+		// Output may be incomplete even when the process exited
+		// successfully, if the output pipe drained slowly.
+		case errors.Is(err, exec.ErrWaitDelay):
+			return result, fmt.Errorf(
+				"tool %q: output incomplete: %w", name, err,
+			)
+
+		// Process ran and exited with non-zero status or was killed
+		// by a signal. ProcessState is set; return it with output.
+		default:
+			if _, ok := errors.AsType[*exec.ExitError](err); !ok {
+				return nil, fmt.Errorf("tool %q: %w", name, err)
+			}
+			return result, nil
+		}
+	}
+
+	// No error. Process exited with status 0.
+	return &ToolResult{
+		FinishedAt:   finished,
+		ProcessState: cmd.ProcessState,
+		StartedAt:    started,
+		Stderr:       stderr.Bytes(),
+		Stdout:       stdout.Bytes(),
+	}, nil
+}

internal/tool/tool_test.go

@@ -0,0 +1,543 @@
+package tool
+
+import (
+	"bytes"
+	"context"
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+
+	"code.chimeric.al/chimerical/odidere/internal/config"
+)
+
+func TestTool_ParseArguments(t *testing.T) {
+	tool, err := NewTool(config.ToolConfig{
+		Arguments: []string{"--name {{.name}}", "{{if .flag}}--enabled{{end}}"},
+	})
+	if err != nil {
+		t.Fatalf("NewTool() error = %v", err)
+	}
+	tests := []struct {
+		name        string
+		args        string
+		expected    []string
+		wantErr     bool
+		errContains string
+	}{
+		{
+			name:     "basic",
+			args:     `{"name": "test"}`,
+			expected: []string{"--name test"},
+		},
+		{
+			// Documented limitation (see the ParseArguments doc): numbers
+			// decode as float64, so integers past 2^53 render in
+			// floating-point form.
+			name:     "large integer (float64 limitation)",
+			args:     `{"name": 9007199254740993}`,
+			expected: []string{"--name 9.007199254740992e+15"},
+		},
+		{
+			name:     "with flag",
+			args:     `{"name": "test", "flag": true}`,
+			expected: []string{"--name test", "--enabled"},
+		},
+		{
+			name:     "null",
+			args:     "null",
+			expected: []string{"--name <no value>"},
+		},
+		{
+			name:        "invalid JSON",
+			args:        `{invalid`,
+			wantErr:     true,
+			errContains: "invalid arguments JSON",
+		},
+		{
+			name:        "non-object JSON",
+			args:        `[1, 2, 3]`,
+			wantErr:     true,
+			errContains: "invalid arguments JSON",
+		},
+		{
+			name:        "trailing data",
+			args:        `{"name": "x"} trailing`,
+			wantErr:     true,
+			errContains: "invalid arguments JSON",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := tool.ParseArguments(tt.args)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("ParseArguments() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if tt.wantErr {
+				if !strings.Contains(err.Error(), tt.errContains) {
+					t.Errorf("err = %q, want to contain %q", err, tt.errContains)
+				}
+				return
+			}
+			if len(got) != len(tt.expected) {
+				t.Fatalf("expected len %d, got %d", len(tt.expected), len(got))
+			}
+			for i := range got {
+				if got[i] != tt.expected[i] {
+					t.Errorf("expected %q, got %q", tt.expected[i], got[i])
+				}
+			}
+		})
+	}
+}
+
+func TestTool_ParseArguments_ExecutionError(t *testing.T) {
+	// "{{printf}}" parses cleanly but fails at execution: printf
+	// requires at least one argument.
+	tool, err := NewTool(config.ToolConfig{
+		Arguments: []string{"{{printf}}"},
+	})
+	if err != nil {
+		t.Fatalf("NewTool() error = %v", err)
+	}
+	_, err = tool.ParseArguments(`{}`)
+	if err == nil {
+		t.Fatal("ParseArguments() error = nil, want execution error")
+	}
+	if !strings.Contains(err.Error(), "execute template") {
+		t.Errorf("err = %v, want execute template error", err)
+	}
+}
+
+func TestNewTool(t *testing.T) {
+	tool, err := NewTool(config.ToolConfig{
+		Name:        "test",
+		Description: "desc",
+		Command:     "ls",
+		Timeout:     10 * time.Second,
+	})
+	if err != nil {
+		t.Fatalf("NewTool() error = %v", err)
+	}
+	if tool.Name != "test" {
+		t.Errorf("Name = %q, want %q", tool.Name, "test")
+	}
+	if tool.timeout != 10*time.Second {
+		t.Errorf("timeout = %v, want 10s", tool.timeout)
+	}
+}
+
+func TestNewTool_InvalidTemplate(t *testing.T) {
+	tests := []struct {
+		name string
+		arg  string
+	}{
+		{name: "unclosed action", arg: "{{.name"},
+		{name: "undefined function", arg: "{{nosuch .name}}"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := NewTool(config.ToolConfig{
+				Name: "t", Arguments: []string{tt.arg},
+			})
+			if err == nil {
+				t.Fatal("NewTool() error = nil, want template error")
+			}
+			if !strings.Contains(err.Error(), "invalid template") {
+				t.Errorf("err = %v, want invalid template error", err)
+			}
+		})
+	}
+}
+
+func TestRegistry_NewRegistry(t *testing.T) {
+	tests := []struct {
+		name    string
+		tools   []config.ToolConfig
+		wantErr bool
+	}{
+		{
+			name: "valid",
+			tools: []config.ToolConfig{
+				{Name: "t1", Description: "d1", Command: "c1"},
+				{Name: "t2", Description: "d2", Command: "c2"},
+			},
+			wantErr: false,
+		},
+		{
+			name: "duplicate",
+			tools: []config.ToolConfig{
+				{Name: "t1", Description: "d1", Command: "c1"},
+				{Name: "t1", Description: "d2", Command: "c2"},
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := NewRegistry(tt.tools)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NewRegistry() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestRegistry_ToAPI_Nil(t *testing.T) {
+	var reg *Registry
+	got := reg.ToAPI()
+	if got != nil {
+		t.Errorf("Registry.ToAPI(nil) = %v, want nil", got)
+	}
+}
+
+func TestRegistry_ToAPI_Empty(t *testing.T) {
+	reg, err := NewRegistry(nil)
+	if err != nil {
+		t.Fatalf("NewRegistry(nil) error = %v", err)
+	}
+	got := reg.ToAPI()
+	if len(got) != 0 {
+		t.Errorf("Registry.ToAPI(empty) len = %d, want 0", len(got))
+	}
+}
+
+func TestRegistry_ToAPI_Single(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{
+		{
+			Name:        "weather",
+			Description: "Get the weather",
+			Parameters:  map[string]any{"type": "object"},
+		},
+	})
+	if err != nil {
+		t.Fatalf("NewRegistry error = %v", err)
+	}
+	got := reg.ToAPI()
+	if len(got) != 1 {
+		t.Fatalf("len = %d, want 1", len(got))
+	}
+	if got[0].Type != "function" {
+		t.Errorf("type = %q, want %q", got[0].Type, "function")
+	}
+	if got[0].Function.Name != "weather" {
+		t.Errorf("name = %q, want %q", got[0].Function.Name, "weather")
+	}
+	if got[0].Function.Description != "Get the weather" {
+		t.Errorf("description = %q, want %q", got[0].Function.Description, "Get the weather")
+	}
+}
+
+func TestTool_ToAPI(t *testing.T) {
+	tool := &Tool{
+		Name:        "test",
+		Description: "A test tool",
+		Parameters:  map[string]any{"type": "object"},
+		Command:     "echo",
+		Arguments:   []string{"hello"},
+	}
+
+	api := tool.ToAPI()
+	if api.Type != "function" {
+		t.Errorf("type = %q, want %q", api.Type, "function")
+	}
+	if api.Function.Name != "test" {
+		t.Errorf("name = %q, want %q", api.Function.Name, "test")
+	}
+	if api.Function.Description != "A test tool" {
+		t.Errorf("description = %q, want %q", api.Function.Description, "A test tool")
+	}
+	if api.Function.Parameters == nil {
+		t.Error("parameters is nil")
+	}
+}
+
+func TestToolResult_FormatResult(t *testing.T) {
+	// Get real ProcessState values from actual commands.
+	cmdOK := exec.Command("sh", "-c", "exit 0")
+	if err := cmdOK.Run(); err != nil {
+		t.Fatalf("cmdOK: %v", err)
+	}
+	cmdFail := exec.Command("sh", "-c", "exit 3")
+	cmdFail.Run() // expect non-nil error
+	cmdKill := exec.Command("sh", "-c", "kill -9 $$")
+	cmdKill.Run() // expect non-nil error
+
+	// Fixed times, so the rendered timing lines are deterministic.
+	started := time.Date(2026, 9, 23, 20, 31, 0, 123_000_000, time.UTC)
+	finished := started.Add(1500 * time.Millisecond)
+
+	// Every case renders the same timing lines, because every case uses
+	// the same times.
+	wantStart := "┗ start: " + started.Format(time.RFC3339Nano)
+	wantEnd := "┗ end: " + finished.Format(time.RFC3339Nano)
+	wantDuration := "┗ duration: 1.5s"
+
+	tests := []struct {
+		name         string
+		result       ToolResult
+		wantContains []string
+	}{
+		{
+			name: "success with stdout",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stdout:       []byte("hello\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "hello", "▸ stderr:", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "failure with stderr",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdFail.ProcessState,
+				StartedAt:    started,
+				Stderr:       []byte("error occurred\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "▸ stderr:", "error occurred", "┗ exit: 3",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "both stdout and stderr",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stderr:       []byte("warning"),
+				Stdout:       []byte("output"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "output", "▸ stderr:", "warning", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "empty output",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+			},
+			wantContains: []string{
+				"▸ stdout:", "▸ stderr:", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "binary stdout (PNG)",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stdout:       []byte("\x89PNG\r\n\x1a\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", binaryOutputPrefix, "image/png", "▸ stderr:",
+				"┗ exit: 0", wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "binary stderr",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stderr:       []byte("\x89PNG\r\n\x1a\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "▸ stderr:", binaryOutputPrefix, "image/png",
+				"┗ exit: 0", wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "HTML",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stdout:       []byte("<HTML><BODY>Hello</BODY></HTML>\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "<HTML>", "▸ stderr:", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "XML",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stdout:       []byte("<?xml version=\"1.0\"?><root/>\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "<?xml", "▸ stderr:", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "Go source",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stdout:       []byte("package main\n\nfunc main() {}\n"),
+			},
+			wantContains: []string{
+				"▸ stdout:", "package main", "▸ stderr:", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "JSON",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdOK.ProcessState,
+				StartedAt:    started,
+				Stdout:       []byte(`{"key":"value"}`),
+			},
+			wantContains: []string{
+				"▸ stdout:", `{"key":"value"}`, "▸ stderr:", "┗ exit: 0",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+		{
+			name: "killed by signal",
+			result: ToolResult{
+				FinishedAt:   finished,
+				ProcessState: cmdKill.ProcessState,
+				StartedAt:    started,
+			},
+			wantContains: []string{
+				"▸ stdout:", "▸ stderr:", "┗ exit: -1 (signal: killed)",
+				wantStart, wantEnd, wantDuration,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.result.FormatResult()
+			for _, want := range tt.wantContains {
+				if !strings.Contains(got, want) {
+					t.Errorf("FormatResult() = %q, want to contain %q", got, want)
+				}
+			}
+		})
+	}
+}
+
+func TestRegistry_Execute_Success(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{
+		{Name: "echo", Description: "echo", Command: "echo", Arguments: []string{"hello"}},
+	})
+	if err != nil {
+		t.Fatalf("NewRegistry: %v", err)
+	}
+
+	result, err := reg.Execute(context.Background(), "echo", "{}")
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	if result.ProcessState.ExitCode() != 0 {
+		t.Errorf("ExitCode = %d, want 0", result.ProcessState.ExitCode())
+	}
+	if !bytes.Contains(result.Stdout, []byte("hello")) {
+		t.Errorf("Stdout = %q, want to contain hello", result.Stdout)
+	}
+}
+
+func TestRegistry_Execute_NonZeroExit(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{
+		{Name: "fail", Description: "fail", Command: "sh", Arguments: []string{"-c", "exit 42"}},
+	})
+	if err != nil {
+		t.Fatalf("NewRegistry: %v", err)
+	}
+
+	result, err := reg.Execute(context.Background(), "fail", "{}")
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	if result.ProcessState.ExitCode() != 42 {
+		t.Errorf("ExitCode = %d, want 42", result.ProcessState.ExitCode())
+	}
+}
+
+func TestRegistry_Execute_UnknownCommand(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{
+		{Name: "nope", Description: "nope", Command: "nonexistent_binary_xyz", Arguments: []string{}},
+	})
+	if err != nil {
+		t.Fatalf("NewRegistry: %v", err)
+	}
+
+	_, err = reg.Execute(context.Background(), "nope", "{}")
+	if err == nil {
+		t.Fatal("Execute: want error, got nil")
+	}
+}
+
+func TestRegistry_Execute_UnknownTool(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{})
+	if err != nil {
+		t.Fatalf("NewRegistry: %v", err)
+	}
+
+	_, err = reg.Execute(context.Background(), "missing", "{}")
+	if err == nil {
+		t.Fatal("Execute: want error, got nil")
+	}
+	if !strings.Contains(err.Error(), "unknown tool") {
+		t.Errorf("err = %v, want unknown tool error", err)
+	}
+}
+
+func TestRegistry_Execute_Timeout(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{
+		{Name: "sleep", Description: "sleep", Command: "sleep", Arguments: []string{"10"}, Timeout: 50 * time.Millisecond},
+	})
+	if err != nil {
+		t.Fatalf("NewRegistry: %v", err)
+	}
+
+	_, err = reg.Execute(context.Background(), "sleep", "{}")
+	if err == nil {
+		t.Fatal("Execute: want timeout error, got nil")
+	}
+	if !strings.Contains(err.Error(), "timed out") {
+		t.Errorf("err = %v, want timed out error", err)
+	}
+}
+
+func TestRegistry_Execute_SignalKilled(t *testing.T) {
+	reg, err := NewRegistry([]config.ToolConfig{
+		{Name: "kill", Description: "kill", Command: "sh", Arguments: []string{"-c", "kill -9 $$"}},
+	})
+	if err != nil {
+		t.Fatalf("NewRegistry: %v", err)
+	}
+
+	result, err := reg.Execute(context.Background(), "kill", "{}")
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	// The shell kills itself ($$ is its own PID) so that the direct
+	// child dies by signal and ExitCode reports -1.
+	if result.ProcessState.ExitCode() != -1 {
+		t.Errorf("ExitCode = %d, want -1 (killed by signal)", result.ProcessState.ExitCode())
+	}
+	if !strings.Contains(result.ProcessState.String(), "signal:") {
+		t.Errorf("ProcessState.String() = %q, want signal info", result.ProcessState.String())
+	}
+}