tools: remove file_read and file_write in favor of shell tools
The line-range patching interface was fragile (models drift on line numbers), and both tools were redundant with execute_code + standard shell utilities (cat, grep, sed, ed, ssam). Drop the file server entirely; update agent configs, system prompt, and documentation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e2d973fe3f
commit
4b9b03053f
20
README.md
20
README.md
|
|
@ -9,13 +9,13 @@ The reference frontend is [ollie-tui](https://github.com/lneely/ollie-tui), a te
|
|||
## Packages
|
||||
|
||||
```
|
||||
pkg/agent/ — Core interface, agent loop, session management
|
||||
pkg/backend/ — Backend interface + implementations (Ollama, OpenAI, Anthropic, Copilot, Kiro)
|
||||
pkg/config/ — Config struct and loader
|
||||
pkg/mcp/ — MCP client
|
||||
pkg/tools/ — Server and Dispatcher interfaces; tool definitions
|
||||
pkg/tools/execute/ — execute.Server: execute_code, execute_tool, execute_pipe
|
||||
pkg/tools/file/ — file.Server: file_read, file_write
|
||||
pkg/agent/ — Core interface, agent loop, session management
|
||||
pkg/backend/ — Backend interface + implementations (Ollama, OpenAI, Anthropic, Copilot, Kiro)
|
||||
pkg/config/ — Config struct and loader
|
||||
pkg/mcp/ — MCP client
|
||||
pkg/tools/ — Server and Dispatcher interfaces; tool definitions
|
||||
pkg/tools/execute/ — execute.Server: execute_code, execute_tool, execute_pipe
|
||||
pkg/tools/reasoning/ — reasoning.Server: reasoning_think, reasoning_plan
|
||||
```
|
||||
|
||||
## Install
|
||||
|
|
@ -78,8 +78,10 @@ Five built-in tools across two servers:
|
|||
- `execute_code` — run inline shell code in a sandbox
|
||||
- `execute_tool` — run a named tool script from `OLLIE_TOOLS_PATH` (default: `~/.local/share/ollie/tools`)
|
||||
- `execute_pipe` — chain steps as a pipeline
|
||||
- `file_read` — read a file with line numbers (required before `file_write`)
|
||||
- `file_write` — write or patch a file by line range
|
||||
- `reasoning_think` — externalize intermediate reasoning
|
||||
- `reasoning_plan` — decompose a goal into ordered steps; persists to task backend if available
|
||||
|
||||
File operations go through `execute_code` using standard shell tools (`cat`, `grep`, `sed`, `ed`, `ssam` if plan9port is available, etc.).
|
||||
|
||||
MCP server tools are discovered at startup and available alongside the built-ins.
|
||||
|
||||
|
|
|
|||
|
|
@ -22,8 +22,6 @@
|
|||
"execute_code",
|
||||
"execute_tool",
|
||||
"execute_pipe",
|
||||
"file_read",
|
||||
"file_write",
|
||||
"task_create",
|
||||
"task_list",
|
||||
"task_read",
|
||||
|
|
|
|||
|
|
@ -15,8 +15,6 @@
|
|||
"execute_code",
|
||||
"execute_tool",
|
||||
"execute_pipe",
|
||||
"file_read",
|
||||
"file_write",
|
||||
"task_create",
|
||||
"task_list",
|
||||
"task_read",
|
||||
|
|
|
|||
|
|
@ -14,7 +14,6 @@ pkg/
|
|||
mcp/ — MCP client (concrete)
|
||||
tools/ — Server and Dispatcher interfaces, tool definitions (builtin.go)
|
||||
tools/execute/ — execute.Server: execute_code, execute_tool, execute_pipe
|
||||
tools/file/ — file.Server: file_read, file_write
|
||||
tools/reasoning/ — reasoning.Server: reasoning_think, reasoning_plan
|
||||
|
||||
internal/
|
||||
|
|
@ -61,8 +60,6 @@ Frontend
|
|||
└── tools.Dispatcher.Dispatch(...) — tool dispatch
|
||||
├── "execute" → tools.Server
|
||||
│ execute_code / execute_tool / execute_pipe
|
||||
├── "file" → tools.Server
|
||||
│ file_read / file_write
|
||||
├── "reasoning" → tools.Server
|
||||
│ reasoning_think
|
||||
│ reasoning_plan
|
||||
|
|
@ -75,18 +72,18 @@ All tool calls go through one `tools.Dispatcher.Dispatch` path. Every registered
|
|||
|
||||
## Built-in Tools
|
||||
|
||||
Seven tools are registered across three servers:
|
||||
Five tools are registered across two servers:
|
||||
|
||||
| Server | Tool | What it does |
|
||||
|---|---|---|
|
||||
| `execute` | `execute_code` | Runs inline bash in a landrun sandbox |
|
||||
| `execute` | `execute_tool` | Reads a named script from `OLLIE_TOOLS_PATH` and runs it sandboxed |
|
||||
| `execute` | `execute_pipe` | Chains steps, piping stdout of each into stdin of the next |
|
||||
| `file` | `file_read` | Reads a file with line numbers (required before `file_write`) |
|
||||
| `file` | `file_write` | Writes or patches a file by line range |
|
||||
| `reasoning` | `reasoning_think` | Externalizes intermediate reasoning (no-op, recorded in history) |
|
||||
| `reasoning` | `reasoning_plan` | Decomposes a goal into ordered steps; persists to task backend if available |
|
||||
|
||||
File operations go through `execute_code` using standard shell tools (`cat`, `grep`, `sed`, `ed`, `ssam` if plan9port is available, etc.).
|
||||
|
||||
Tool definitions live in `pkg/tools/builtin.go` to avoid import cycles — the subpackages import `pkg/tools`, so `pkg/tools` cannot import them back.
|
||||
|
||||
`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.
|
||||
|
|
@ -121,8 +118,8 @@ See [doc/PLANNING.md](doc/PLANNING.md) for the full design rationale.
|
|||
|
||||
```go
|
||||
newDispatcher := tools.NewDispatcherFunc(map[string]func() tools.Server{
|
||||
"execute": execute.Decl(workdir), // "" falls back to os.Getwd()
|
||||
"file": file.Decl,
|
||||
"execute": execute.Decl(workdir), // "" falls back to os.Getwd()
|
||||
"reasoning": reasoning.Decl(),
|
||||
})
|
||||
|
||||
env := agent.BuildAgentEnv(cfg, newDispatcher(), workdir) // also connects MCP servers from cfg
|
||||
|
|
|
|||
|
|
@ -60,37 +60,6 @@ func ExecuteDefs(toolsPath string) []ToolInfo {
|
|||
}
|
||||
}
|
||||
|
||||
// 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."}
|
||||
}
|
||||
}`),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// ReasoningDefs returns the ToolInfo definitions for the reasoning_* built-in tools.
|
||||
func ReasoningDefs() []ToolInfo {
|
||||
return []ToolInfo{
|
||||
|
|
|
|||
|
|
@ -1,21 +0,0 @@
|
|||
package file
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// 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)
|
||||
}
|
||||
lines := strings.Split(string(data), "\n")
|
||||
var out strings.Builder
|
||||
for i, line := range lines {
|
||||
fmt.Fprintf(&out, "%d\t%s\n", i+1, line)
|
||||
}
|
||||
return strings.TrimRight(out.String(), "\n"), nil
|
||||
}
|
||||
|
|
@ -1,92 +0,0 @@
|
|||
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
|
||||
}
|
||||
|
||||
// New returns a file Server.
|
||||
func New() *Server { return &Server{} }
|
||||
|
||||
// Decl is the default factory for a file Server. Reassign to override.
|
||||
var Decl func() tools.Server = func() tools.Server { return New() }
|
||||
|
||||
// ListTools implements tools.Server.
|
||||
func (s *Server) ListTools() ([]tools.ToolInfo, error) {
|
||||
return tools.FileDefs(), nil
|
||||
}
|
||||
|
||||
// CallTool implements tools.Server.
|
||||
func (s *Server) CallTool(_ context.Context, tool string, args json.RawMessage) (json.RawMessage, error) {
|
||||
result, err := s.dispatch(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 (s *Server) Close() {}
|
||||
|
||||
func (s *Server) dispatch(name string, args json.RawMessage) (string, error) {
|
||||
switch name {
|
||||
case "file_read":
|
||||
return s.dispatchRead(args)
|
||||
case "file_write":
|
||||
return s.dispatchWrite(args)
|
||||
default:
|
||||
return "", fmt.Errorf("unknown file tool: %s", name)
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Server) dispatchRead(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 s.Confirm != nil && !s.Confirm("read "+a.Path) {
|
||||
return "", fmt.Errorf("file_read: denied by user")
|
||||
}
|
||||
return Read(a.Path)
|
||||
}
|
||||
|
||||
func (s *Server) dispatchWrite(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 s.Confirm != nil && !s.Confirm(prompt) {
|
||||
return "", fmt.Errorf("file_write: denied by user")
|
||||
}
|
||||
return Write(a.Path, a.Content, a.StartLine, a.EndLine)
|
||||
}
|
||||
|
||||
var _ tools.Server = (*Server)(nil) // compile-time interface check
|
||||
|
|
@ -1,39 +0,0 @@
|
|||
package file
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// 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(content), path), nil
|
||||
}
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("file_write: %w", err)
|
||||
}
|
||||
lines := strings.Split(string(data), "\n")
|
||||
start, end := startLine, endLine
|
||||
if start < 1 {
|
||||
start = 1
|
||||
}
|
||||
if end > len(lines) {
|
||||
end = len(lines)
|
||||
}
|
||||
if start > end {
|
||||
return "", fmt.Errorf("file_write: start_line %d > end_line %d", start, end)
|
||||
}
|
||||
newLines := strings.Split(content, "\n")
|
||||
result := append(lines[:start-1], append(newLines, lines[end:]...)...)
|
||||
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, path), nil
|
||||
}
|
||||
|
|
@ -19,7 +19,7 @@ Be truthful. If the user is wrong, say so — do not soften, hedge, or dance aro
|
|||
|
||||
# Tool preferences
|
||||
|
||||
- Prefer `grep`/`execute_code` for searching and exploring. Use `file_read` when you need the full file.
|
||||
- Use `execute_code` to run shell commands: grep, cat, sed, ed, and other standard tools for reading and editing files.
|
||||
- Make independent tool calls in parallel when there are no dependencies between them.
|
||||
|
||||
# Environment
|
||||
|
|
|
|||
Reference in New Issue