diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6466d6e..07779e9 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -12,8 +12,9 @@ pkg/ backend/ — Backend interface + implementations config/ — Agent config struct and loader mcp/ — MCP client (concrete) - tools/ — Executor interface + MCPExecutor - tools/execute/ — Built-in sandbox tools (execute_code, execute_tool, execute_pipe) + tools/ — Server/Executor interfaces, MCPExecutor, BuiltinServer, tool definitions + tools/execute/ — Sandbox runner (execute_code, execute_tool, execute_pipe) + tools/file/ — File operations (file_read, file_write) internal/ sandbox/ — landrun sandbox config and command wrapper @@ -47,11 +48,13 @@ Frontend ├── backend.ChatStream() — LLM call └── tools.Executor.Execute(server, tool, args) — tool dispatch (interface) └── tools.MCPExecutor — routes by server name - ├── "builtin" → tools/execute.Executor — sandbox tools + ├── "builtin" → tools.BuiltinServer — all built-in tools + │ ├── execute_* → execute.Executor.Dispatch() + │ └── file_* → file.Read / file.Write └── "" → tools.mcpServer — MCP protocol tools ``` -All tool calls go through one `tools.Executor.Execute` path. Built-in tools implement `tools.Server` directly (no subprocess); MCP tools are wrapped in `mcpServer` which speaks the MCP protocol over stdio. +All tool calls go through one `tools.Executor.Execute` path. `BuiltinServer` in `pkg/tools/` is the single built-in server; `execute/` and `file/` are its implementation subpackages and do not implement `tools.Server` themselves. MCP tools are wrapped in `mcpServer`. ## Built-in Tools @@ -65,7 +68,7 @@ Five tools are registered under the `"builtin"` server by default: | `file_read` | Reads a file with line numbers (required before `file_write`) | | `file_write` | Writes or patches a file by line range | -`execute.Executor` implements `tools.Server` — built-in tools speak the same interface as MCP tools without running a subprocess. All dispatch logic lives in `pkg/tools/execute/dispatch.go`. +`tools.BuiltinServer` in `pkg/tools/` is the single `tools.Server` for all built-in tools. Tool definitions (`ExecuteDefs`, `FileDefs`) and file dispatch live in `pkg/tools/builtin.go`; execute dispatch lives in `pkg/tools/execute/dispatch.go`. `OLLIE_TOOLS_PATH` defaults to `~/.local/share/ollie/tools`. The directory can be a symlink or a mountpoint — `execute_tool` treats it as an ordinary filesystem path. diff --git a/pkg/agent/core.go b/pkg/agent/core.go index 7077729..d1604e3 100644 --- a/pkg/agent/core.go +++ b/pkg/agent/core.go @@ -20,7 +20,6 @@ import ( "ollie/pkg/backend" "ollie/pkg/config" execute "ollie/pkg/tools/execute" - "ollie/pkg/tools/file" "ollie/pkg/mcp" "ollie/pkg/tools" ) @@ -181,17 +180,13 @@ func BuildAgentEnv(cfg *config.Config, builtinExec *execute.Executor) AgentEnv { // Wire confirm through pointer so it can be updated after construction. var cfn confirmFn confirmPtr := &cfn - confirmClosure := func(prompt string) bool { + builtinExec.Confirm = func(prompt string) bool { if *confirmPtr == nil { return true } return (*confirmPtr)(prompt) } - builtinExec.Confirm = confirmClosure - mcpExec.AddServer("builtin", builtinExec) - - fileServer := &file.Server{Confirm: confirmClosure} - mcpExec.AddServer("file", fileServer) + mcpExec.AddServer("builtin", &tools.BuiltinServer{Exec: builtinExec}) allToolInfos, listErr := mcpExec.ListTools() if listErr != nil { diff --git a/pkg/tools/builtin.go b/pkg/tools/builtin.go new file mode 100644 index 0000000..1beb3a5 --- /dev/null +++ b/pkg/tools/builtin.go @@ -0,0 +1,187 @@ +// Package tools defines the Executor and Server interfaces, the MCPExecutor +// implementation, and the BuiltinServer that provides the built-in file and +// execute tools. +package tools + +import ( + "context" + "encoding/json" + "fmt" + + "ollie/pkg/tools/execute" + "ollie/pkg/tools/file" +) + +// BuiltinServer implements Server for the built-in execute and file tools. +// Confirm is read from Exec.Confirm for all operations. +type BuiltinServer struct { + Exec *execute.Executor +} + +// ListTools returns definitions for all five built-in tools. +func (b *BuiltinServer) ListTools() ([]ToolInfo, error) { + return append(ExecuteDefs(execute.ToolsPath()), FileDefs()...), nil +} + +// CallTool dispatches a built-in tool call and wraps the result in MCP format. +func (b *BuiltinServer) CallTool(ctx context.Context, tool string, args json.RawMessage) (json.RawMessage, error) { + result, err := b.dispatch(ctx, tool, args) + if err != nil { + return json.Marshal(map[string]string{"error": err.Error()}) + } + return json.Marshal(map[string]any{ + "content": []map[string]string{{"type": "text", "text": result}}, + }) +} + +// Close is a no-op. +func (b *BuiltinServer) Close() {} + +func (b *BuiltinServer) dispatch(ctx context.Context, name string, args json.RawMessage) (string, error) { + switch name { + case "execute_code", "execute_tool", "execute_pipe": + return b.Exec.Dispatch(ctx, name, args) + case "file_read": + return b.dispatchFileRead(args) + case "file_write": + return b.dispatchFileWrite(args) + default: + return "", fmt.Errorf("unknown built-in tool: %s", name) + } +} + +func (b *BuiltinServer) confirm(prompt string) bool { + if b.Exec.Confirm == nil { + return true + } + return b.Exec.Confirm(prompt) +} + +func (b *BuiltinServer) dispatchFileRead(args json.RawMessage) (string, error) { + var a struct { + Path string `json:"path"` + } + if err := json.Unmarshal(args, &a); err != nil { + return "", fmt.Errorf("file_read: bad args: %w", err) + } + if a.Path == "" { + return "", fmt.Errorf("file_read: 'path' is required") + } + if !b.confirm("read " + a.Path) { + return "", fmt.Errorf("file_read: denied by user") + } + return file.Read(a.Path) +} + +func (b *BuiltinServer) dispatchFileWrite(args json.RawMessage) (string, error) { + var a struct { + Path string `json:"path"` + Content string `json:"content"` + StartLine int `json:"start_line"` + EndLine int `json:"end_line"` + } + if err := json.Unmarshal(args, &a); err != nil { + return "", fmt.Errorf("file_write: bad args: %w", err) + } + if a.Path == "" { + return "", fmt.Errorf("file_write: 'path' is required") + } + prompt := "write " + a.Path + if a.StartLine > 0 { + prompt = fmt.Sprintf("write %s lines %d-%d", a.Path, a.StartLine, a.EndLine) + } + if !b.confirm(prompt) { + return "", fmt.Errorf("file_write: denied by user") + } + return file.Write(a.Path, a.Content, a.StartLine, a.EndLine) +} + +// ExecuteDefs returns the ToolInfo definitions for the execute_* built-in tools. +// toolsPath is embedded in the descriptions. +func ExecuteDefs(toolsPath string) []ToolInfo { + return []ToolInfo{ + { + Name: "execute_code", + Description: "Run inline code in a sandboxed environment.", + InputSchema: json.RawMessage(`{ + "type": "object", + "required": ["code"], + "properties": { + "code": {"type": "string", "description": "Code to execute."}, + "language": {"type": "string", "description": "Language interpreter (default: bash)."}, + "timeout": {"type": "integer", "description": "Timeout in seconds (default: 30)."}, + "sandbox": {"type": "string", "description": "Sandbox name (default: default)."} + } + }`), + }, + { + Name: "execute_tool", + Description: "Run a named tool script from " + toolsPath + " in a sandboxed environment. Use this only for named scripts, not for inline shell commands.", + InputSchema: json.RawMessage(`{ + "type": "object", + "required": ["tool"], + "properties": { + "tool": {"type": "string", "description": "Name of the tool script (e.g. discover_skill.sh)."}, + "args": {"type": "array", "items": {"type": "string"}, "description": "Arguments for the tool script."}, + "timeout": {"type": "integer", "description": "Timeout in seconds (default: 30)."}, + "sandbox": {"type": "string", "description": "Sandbox name (default: default)."} + } + }`), + }, + { + Name: "execute_pipe", + Description: "Run a pipeline of steps, piping stdout of each into stdin of the next. Use {code: \"cmd --flags\"} for shell commands; use {tool, args} only for named scripts in " + toolsPath + ".", + InputSchema: json.RawMessage(`{ + "type": "object", + "required": ["pipe"], + "properties": { + "pipe": { + "type": "array", + "items": { + "type": "object", + "properties": { + "tool": {"type": "string"}, + "args": {"type": "array", "items": {"type": "string"}}, + "code": {"type": "string"} + } + } + }, + "timeout": {"type": "integer", "description": "Timeout in seconds (default: 30)."}, + "sandbox": {"type": "string", "description": "Sandbox name (default: default)."} + } + }`), + }, + } +} + +// FileDefs returns the ToolInfo definitions for the file_* built-in tools. +func FileDefs() []ToolInfo { + return []ToolInfo{ + { + Name: "file_read", + Description: "Read a file in full. Output includes line numbers. Use grep/execute_code to search before reading. Prefer file_read only when you need to write — use grep or execute_code for exploration.", + InputSchema: json.RawMessage(`{ + "type": "object", + "required": ["path"], + "properties": { + "path": {"type": "string", "description": "Path to the file."} + } + }`), + }, + { + Name: "file_write", + Description: "Write content to a file. For existing files, start_line and end_line are required — whole-file overwrites are not permitted. For new files (not yet on disk), omit start_line/end_line to write the full content. Always use file_read or grep -n to identify the exact line range before writing. Never guess line numbers. Preserve original formatting and indentation.", + InputSchema: json.RawMessage(`{ + "type": "object", + "required": ["path", "content"], + "properties": { + "path": {"type": "string", "description": "Path to the file."}, + "content": {"type": "string", "description": "Content to write."}, + "start_line": {"type": "integer", "description": "First line of range to replace, 1-based."}, + "end_line": {"type": "integer", "description": "Last line of range to replace, inclusive."} + } + }`), + }, + } +} + diff --git a/pkg/tools/execute/dispatch.go b/pkg/tools/execute/dispatch.go index 167389b..7aeff94 100644 --- a/pkg/tools/execute/dispatch.go +++ b/pkg/tools/execute/dispatch.go @@ -5,97 +5,19 @@ import ( "encoding/json" "fmt" "strings" - - "ollie/pkg/tools" ) -// builtinToolDefs returns the tool definitions for all built-in tools. -func builtinToolDefs() []tools.ToolInfo { - tp := ToolsPath() - return []tools.ToolInfo{ - { - Name: "execute_code", - Description: "Run inline code in a sandboxed environment.", - InputSchema: json.RawMessage(`{ - "type": "object", - "required": ["code"], - "properties": { - "code": {"type": "string", "description": "Code to execute."}, - "language": {"type": "string", "description": "Language interpreter (default: bash)."}, - "timeout": {"type": "integer", "description": "Timeout in seconds (default: 30)."}, - "sandbox": {"type": "string", "description": "Sandbox name (default: default)."} - } - }`), - }, - { - Name: "execute_tool", - Description: "Run a named tool script from " + tp + " in a sandboxed environment. Use this only for named scripts, not for inline shell commands.", - InputSchema: json.RawMessage(`{ - "type": "object", - "required": ["tool"], - "properties": { - "tool": {"type": "string", "description": "Name of the tool script (e.g. discover_skill.sh)."}, - "args": {"type": "array", "items": {"type": "string"}, "description": "Arguments for the tool script."}, - "timeout": {"type": "integer", "description": "Timeout in seconds (default: 30)."}, - "sandbox": {"type": "string", "description": "Sandbox name (default: default)."} - } - }`), - }, - { - Name: "execute_pipe", - Description: "Run a pipeline of steps, piping stdout of each into stdin of the next. Use {code: \"cmd --flags\"} for shell commands; use {tool, args} only for named scripts in " + tp + ".", - InputSchema: json.RawMessage(`{ - "type": "object", - "required": ["pipe"], - "properties": { - "pipe": { - "type": "array", - "items": { - "type": "object", - "properties": { - "tool": {"type": "string"}, - "args": {"type": "array", "items": {"type": "string"}}, - "code": {"type": "string"} - } - } - }, - "timeout": {"type": "integer", "description": "Timeout in seconds (default: 30)."}, - "sandbox": {"type": "string", "description": "Sandbox name (default: default)."} - } - }`), - }, - } -} - -// ListTools implements tools.Server. -func (e *Executor) ListTools() ([]tools.ToolInfo, error) { - return builtinToolDefs(), nil -} - -// CallTool implements tools.Server. -func (e *Executor) CallTool(ctx context.Context, tool string, args json.RawMessage) (json.RawMessage, error) { - result, err := e.dispatch(ctx, tool, args) - if err != nil { - return json.Marshal(map[string]string{"error": err.Error()}) - } - return json.Marshal(map[string]any{ - "content": []map[string]string{{"type": "text", "text": result}}, - }) -} - -// Close implements tools.Server (no-op). -func (e *Executor) Close() {} - -func (e *Executor) dispatch(ctx context.Context, name string, args json.RawMessage) (string, error) { +// Dispatch routes a named execute tool call. Called by tools.BuiltinServer. +func (e *Executor) Dispatch(ctx context.Context, name string, args json.RawMessage) (string, error) { switch name { - case "execute_pipe": - return dispatchExecutePipe(ctx, e, args) - case "execute_tool": - return dispatchExecuteTool(ctx, e, args) case "execute_code": return dispatchExecuteCode(ctx, e, args) + case "execute_tool": + return dispatchExecuteTool(ctx, e, args) + case "execute_pipe": + return dispatchExecutePipe(ctx, e, args) default: - return "", fmt.Errorf("unknown built-in tool: %s", name) + return "", fmt.Errorf("unknown execute tool: %s", name) } } @@ -141,11 +63,10 @@ func dispatchExecuteCode(ctx context.Context, e *Executor, args json.RawMessage) func dispatchExecuteTool(ctx context.Context, e *Executor, args json.RawMessage) (string, error) { var a struct { - Tool string `json:"tool"` - Args []string `json:"args"` - Language string `json:"language"` - Timeout int `json:"timeout"` - Sandbox string `json:"sandbox"` + Tool string `json:"tool"` + Args []string `json:"args"` + Timeout int `json:"timeout"` + Sandbox string `json:"sandbox"` } if err := json.Unmarshal(args, &a); err != nil { return "", fmt.Errorf("execute_tool: bad args: %w", err) @@ -168,10 +89,6 @@ func dispatchExecuteTool(ctx context.Context, e *Executor, args json.RawMessage) } code = fmt.Sprintf("set -- %s\n%s", strings.Join(escaped, " "), code) } - language := a.Language - if language == "" { - language = "bash" - } timeout := a.Timeout if timeout <= 0 { timeout = 30 @@ -180,7 +97,7 @@ func dispatchExecuteTool(ctx context.Context, e *Executor, args json.RawMessage) if sandbox == "" { sandbox = "default" } - return e.Execute(ctx, code, language, timeout, sandbox, true) + return e.Execute(ctx, code, "bash", timeout, sandbox, true) } func dispatchExecutePipe(ctx context.Context, e *Executor, args json.RawMessage) (string, error) { diff --git a/pkg/tools/file/read.go b/pkg/tools/file/read.go index 57b27e6..8763e0d 100644 --- a/pkg/tools/file/read.go +++ b/pkg/tools/file/read.go @@ -1,26 +1,14 @@ package file import ( - "encoding/json" "fmt" "os" "strings" ) -func read(confirm func(string) bool, args json.RawMessage) (string, error) { - var a struct { - Path string `json:"path"` - } - if err := json.Unmarshal(args, &a); err != nil { - return "", fmt.Errorf("file_read: bad args: %w", err) - } - if a.Path == "" { - return "", fmt.Errorf("file_read: 'path' is required") - } - if confirm != nil && !confirm(fmt.Sprintf("read %s", a.Path)) { - return "", fmt.Errorf("file_read: denied by user") - } - data, err := os.ReadFile(a.Path) +// Read reads path and returns its contents with 1-based line numbers. +func Read(path string) (string, error) { + data, err := os.ReadFile(path) if err != nil { return "", fmt.Errorf("file_read: %w", err) } diff --git a/pkg/tools/file/server.go b/pkg/tools/file/server.go deleted file mode 100644 index f99db42..0000000 --- a/pkg/tools/file/server.go +++ /dev/null @@ -1,67 +0,0 @@ -// Package file provides the file_read and file_write built-in tools. -package file - -import ( - "context" - "encoding/json" - "fmt" - - "ollie/pkg/tools" -) - -// Server implements tools.Server for file_read and file_write. -type Server struct { - // Confirm is called before each operation. Return false to deny. - Confirm func(string) bool -} - -func (s *Server) ListTools() ([]tools.ToolInfo, error) { - return []tools.ToolInfo{ - { - Name: "file_read", - Description: "Read a file in full. Output includes line numbers. Use grep/execute_code to search before reading. Prefer file_read only when you need to write — use grep or execute_code for exploration.", - InputSchema: json.RawMessage(`{ - "type": "object", - "required": ["path"], - "properties": { - "path": {"type": "string", "description": "Path to the file."} - } - }`), - }, - { - Name: "file_write", - Description: "Write content to a file. For existing files, start_line and end_line are required — whole-file overwrites are not permitted. For new files (not yet on disk), omit start_line/end_line to write the full content. Always use file_read or grep -n to identify the exact line range before writing. Never guess line numbers. Preserve original formatting and indentation.", - InputSchema: json.RawMessage(`{ - "type": "object", - "required": ["path", "content"], - "properties": { - "path": {"type": "string", "description": "Path to the file."}, - "content": {"type": "string", "description": "Content to write."}, - "start_line": {"type": "integer", "description": "First line of range to replace, 1-based."}, - "end_line": {"type": "integer", "description": "Last line of range to replace, inclusive."} - } - }`), - }, - }, nil -} - -func (s *Server) CallTool(_ context.Context, tool string, args json.RawMessage) (json.RawMessage, error) { - var result string - var err error - switch tool { - case "file_read": - result, err = read(s.Confirm, args) - case "file_write": - result, err = write(s.Confirm, args) - default: - return nil, fmt.Errorf("unknown tool: %s", tool) - } - if err != nil { - return json.Marshal(map[string]string{"error": err.Error()}) - } - return json.Marshal(map[string]any{ - "content": []map[string]string{{"type": "text", "text": result}}, - }) -} - -func (s *Server) Close() {} diff --git a/pkg/tools/file/write.go b/pkg/tools/file/write.go index 774c1b1..c135506 100644 --- a/pkg/tools/file/write.go +++ b/pkg/tools/file/write.go @@ -1,44 +1,26 @@ package file import ( - "encoding/json" "fmt" "os" "strings" ) -func write(confirm func(string) bool, args json.RawMessage) (string, error) { - var a struct { - Path string `json:"path"` - Content string `json:"content"` - StartLine int `json:"start_line"` - EndLine int `json:"end_line"` - } - if err := json.Unmarshal(args, &a); err != nil { - return "", fmt.Errorf("file_write: bad args: %w", err) - } - if a.Path == "" { - return "", fmt.Errorf("file_write: 'path' is required") - } - prompt := fmt.Sprintf("write %s", a.Path) - if a.StartLine > 0 { - prompt = fmt.Sprintf("write %s lines %d-%d", a.Path, a.StartLine, a.EndLine) - } - if confirm != nil && !confirm(prompt) { - return "", fmt.Errorf("file_write: denied by user") - } - if a.StartLine == 0 && a.EndLine == 0 { - if err := os.WriteFile(a.Path, []byte(a.Content), 0644); err != nil { +// Write writes content to path. If startLine and endLine are both zero the +// file is written in full; otherwise the given line range is replaced. +func Write(path, content string, startLine, endLine int) (string, error) { + if startLine == 0 && endLine == 0 { + if err := os.WriteFile(path, []byte(content), 0644); err != nil { return "", fmt.Errorf("file_write: %w", err) } - return fmt.Sprintf("wrote %d bytes to %s", len(a.Content), a.Path), nil + return fmt.Sprintf("wrote %d bytes to %s", len(content), path), nil } - data, err := os.ReadFile(a.Path) + data, err := os.ReadFile(path) if err != nil { return "", fmt.Errorf("file_write: %w", err) } lines := strings.Split(string(data), "\n") - start, end := a.StartLine, a.EndLine + start, end := startLine, endLine if start < 1 { start = 1 } @@ -48,10 +30,10 @@ func write(confirm func(string) bool, args json.RawMessage) (string, error) { if start > end { return "", fmt.Errorf("file_write: start_line %d > end_line %d", start, end) } - newLines := strings.Split(a.Content, "\n") + newLines := strings.Split(content, "\n") result := append(lines[:start-1], append(newLines, lines[end:]...)...) - if err := os.WriteFile(a.Path, []byte(strings.Join(result, "\n")), 0644); err != nil { + if err := os.WriteFile(path, []byte(strings.Join(result, "\n")), 0644); err != nil { return "", fmt.Errorf("file_write: %w", err) } - return fmt.Sprintf("replaced lines %d-%d in %s", start, end, a.Path), nil + return fmt.Sprintf("replaced lines %d-%d in %s", start, end, path), nil }