From 8ff5f8d5eef0c3865e56a0acabfb91fb9a3994f3 Mon Sep 17 00:00:00 2001 From: Ludovico Russo Date: Thu, 30 Apr 2026 10:10:05 +0200 Subject: [PATCH 1/5] feat(tools): grep tool + pkg/search foundation (#77) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First slice of PRD #76. Adds a pure-Go grep tool the agent can call instead of shelling out to ripgrep/grep, plus the deep-module package that owns the workspace walk semantics so future search tools (#80 glob) can share them. **pkg/search.** Self-contained, dependency-free of tool/agent layers. Content(ctx, root, opts) walks root, applies a default skip list (.git, node_modules, vendor, dist, build, target), sniffs the leading 512 bytes for a NUL byte to skip binaries, scans surviving files line by line with a compiled regex, and returns matches sorted by (path, line) for deterministic ordering. Path/glob filters and a head limit are honored at the package layer; an absolute or "../" path that escapes root is rejected with an explicit error so the tool layer inherits the boundary check for free. **grep tool.** Thin adapter at pkg/agent/tools/search.go. Three output modes: content (default, returns {path, line, text}), files_with_matches (deduped path list), and count (per-file totals). When matches exceed head_limit the response carries Truncated=true plus an Indicator ("showing first N of M …") so the model knows it's seeing a window. Wired into both Prepare (chat) and PrepareCode (code) registries; the existing subagent default-inheritance picks it up automatically. **Config plumbing.** YAML gains a tools.grep block with max_results and max_file_size_bytes; both are optional and zero-valued defaults fall through to the constants in pkg/search. Threaded daemon.Config → agent.Config → tools.SearchConfig. **Tests.** Fixture-workspace coverage in pkg/search for match counts, binary skip, default skip dirs, head-limit truncation, case insensitivity, path scoping, boundary rejection, glob filtering, deterministic ordering, max-file-size, and invalid-pattern. Tool-layer tests cover schema generation (pattern is required), each output mode, invalid output_mode, head-limit + indicator, path-escape rejection, case-insensitive flag, and config-driven MaxResults. Config tests cover the new tools block parsing both present and absent. CODE_AGENT.md gains a Search section pointing at grep. --- cmd/start.go | 13 ++ pkg/agent/CODE_AGENT.md | 10 ++ pkg/agent/agent.go | 3 + pkg/agent/tools/config.go | 7 + pkg/agent/tools/search.go | 194 +++++++++++++++++++++++++ pkg/agent/tools/search_test.go | 204 ++++++++++++++++++++++++++ pkg/daemon/daemon.go | 2 + pkg/search/search.go | 257 +++++++++++++++++++++++++++++++++ pkg/search/search_test.go | 241 +++++++++++++++++++++++++++++++ x/config/load.go | 13 ++ x/config/load_test.go | 52 +++++++ 11 files changed, 996 insertions(+) create mode 100644 pkg/agent/tools/config.go create mode 100644 pkg/agent/tools/search.go create mode 100644 pkg/agent/tools/search_test.go create mode 100644 pkg/search/search.go create mode 100644 pkg/search/search_test.go diff --git a/cmd/start.go b/cmd/start.go index e9e2b3b..450a5a8 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" + "github.com/ludusrusso/wildgecu/pkg/agent/tools" "github.com/ludusrusso/wildgecu/pkg/daemon" "github.com/ludusrusso/wildgecu/x/config" "github.com/ludusrusso/wildgecu/x/setup" @@ -13,6 +14,17 @@ import ( "github.com/spf13/cobra" ) +// buildToolsConfig translates the YAML tools section into the runtime +// tools.Config consumed by the agent. +func buildToolsConfig(in config.ToolsConfig) tools.Config { + return tools.Config{ + Search: tools.SearchConfig{ + MaxResults: in.Grep.MaxResults, + MaxFileSizeBytes: in.Grep.MaxFileSizeBytes, + }, + } +} + func init() { cmd := startCmd() rootCmd.AddCommand(cmd) @@ -85,5 +97,6 @@ func runDaemon() error { Container: newContainer(), ProviderNames: providerNames, ModelAliases: appConfig.Models, + Tools: buildToolsConfig(appConfig.Tools), }) } diff --git a/pkg/agent/CODE_AGENT.md b/pkg/agent/CODE_AGENT.md index 3ccf2bf..fcf55e6 100644 --- a/pkg/agent/CODE_AGENT.md +++ b/pkg/agent/CODE_AGENT.md @@ -13,6 +13,16 @@ You have dedicated file tools. **Always prefer them over bash for file I/O:** **Do NOT use bash for**: `cat`, `head`, `tail`, `ls`, `find`, `echo >`, or any file read/write operation. +## Search + +You also have a dedicated content-search tool. **Prefer it over shelling out to `grep` or `rg`:** + +- **`grep`** — Search file contents by regex across the workspace. Supports `path` to scope to a subtree, `glob` for filename filters (e.g. `*.go`), `case_insensitive`, `head_limit`, and three output modes: + - `content` (default) returns `{path, line, text}` entries. + - `files_with_matches` returns just the matching paths. + - `count` returns per-file match counts. + Skips `.git`, `node_modules`, `vendor`, `dist`, `build`, `target`, and binary files automatically. + ## Bash Use `bash` only for running commands: build, test, git, install, compile, lint — anything that is not file I/O. Bash runs in the working directory `{CWD}`. diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index b7b1a20..048f674 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -24,6 +24,7 @@ type Config struct { ResolveProvider tools.ProviderResolver // nil when model override is not supported MemoryModel string // optional alias/ref for the memory agent; empty = use Provider ModelsInfo *tools.ModelInfo // nil when model info is not available + Tools tools.Config // tool configuration; zero values pick sensible defaults Debug bool } @@ -56,6 +57,7 @@ func Prepare(ctx context.Context, cfg Config) (*session.Config, *debug.Logger, e registry := tool.NewRegistry() registry.Add(tools.GeneralTools()) registry.Add(tools.ExecTools(cfg.Home.Dir())) + registry.Add(tools.SearchTools(cfg.Home.Dir(), cfg.Tools.Search)) registry.Add(tools.SkillTools(skillsDir)) registry.Add(tools.InformTools()) registry.Add(tools.TelegramTools(cfg.TelegramAuth)) @@ -141,6 +143,7 @@ func PrepareCode(ctx context.Context, cfg Config, workDir string) (*session.Conf registry.Add(tools.GeneralTools()) registry.Add(tools.ExecTools(workDir)) registry.Add(tools.FileTools(workDir)) + registry.Add(tools.SearchTools(workDir, cfg.Tools.Search)) registry.Add(tools.SkillTools(skillsDir)) registry.Add(tools.InformTools()) registry.Add(tools.TelegramTools(cfg.TelegramAuth)) diff --git a/pkg/agent/tools/config.go b/pkg/agent/tools/config.go new file mode 100644 index 0000000..0332c45 --- /dev/null +++ b/pkg/agent/tools/config.go @@ -0,0 +1,7 @@ +package tools + +// Config aggregates per-tool configuration. Zero values pick sensible defaults. +type Config struct { + // Search configures grep (and future search tools). + Search SearchConfig +} diff --git a/pkg/agent/tools/search.go b/pkg/agent/tools/search.go new file mode 100644 index 0000000..c846c2b --- /dev/null +++ b/pkg/agent/tools/search.go @@ -0,0 +1,194 @@ +package tools + +import ( + "context" + "fmt" + "sort" + + "github.com/ludusrusso/wildgecu/pkg/provider/tool" + "github.com/ludusrusso/wildgecu/pkg/search" +) + +// SearchConfig configures the search tools (currently grep). Zero values fall +// back to the search package defaults. +type SearchConfig struct { + // MaxResults caps the number of returned items per call (lines for + // content mode, files for files_with_matches/count). Zero = use + // search.DefaultHeadLimit. + MaxResults int + // MaxFileSizeBytes skips files larger than this. Zero = use + // search.DefaultMaxFileSize. + MaxFileSizeBytes int64 +} + +const ( + grepModeContent = "content" + grepModeFilesWithMatches = "files_with_matches" + grepModeCount = "count" +) + +// SearchTools returns search tools (grep) bound to workDir. +func SearchTools(workDir string, cfg SearchConfig) []tool.Tool { + return []tool.Tool{newGrepTool(workDir, cfg)} +} + +type grepInput struct { + Pattern string `json:"pattern" description:"Regex pattern to search for in file contents"` + Path string `json:"path,omitempty" description:"Subdirectory under the workspace to scope the search (defaults to workspace root)"` + Glob string `json:"glob,omitempty" description:"Filename glob filter (e.g. *.go). Matched against file basename."` + CaseInsensitive bool `json:"case_insensitive,omitempty" description:"Case-insensitive match"` + OutputMode string `json:"output_mode,omitempty" description:"content (default, returns path:line:text) | files_with_matches | count"` + HeadLimit int `json:"head_limit,omitempty" description:"Cap on returned items. Default 200."` +} + +type grepMatchOut struct { + Path string `json:"path"` + Line int `json:"line"` + Text string `json:"text"` +} + +type grepFileCount struct { + Path string `json:"path"` + Count int `json:"count"` +} + +type grepOutput struct { + Mode string `json:"mode"` + Matches []grepMatchOut `json:"matches,omitempty"` + Files []string `json:"files,omitempty"` + Counts []grepFileCount `json:"counts,omitempty"` + Total int `json:"total"` + Returned int `json:"returned"` + Truncated bool `json:"truncated,omitempty"` + Indicator string `json:"indicator,omitempty"` +} + +func newGrepTool(workDir string, cfg SearchConfig) tool.Tool { + return tool.NewTool("grep", + "Search file contents by regex across the workspace. Prefer this over bash grep/rg. "+ + "Supports content (path:line:text), files_with_matches, and count output modes.", + func(ctx context.Context, in grepInput) (grepOutput, error) { + mode := in.OutputMode + if mode == "" { + mode = grepModeContent + } + if mode != grepModeContent && mode != grepModeFilesWithMatches && mode != grepModeCount { + return grepOutput{}, fmt.Errorf("invalid output_mode %q (want content|files_with_matches|count)", mode) + } + + defaultHead := cfg.MaxResults + if defaultHead <= 0 { + defaultHead = search.DefaultHeadLimit + } + head := in.HeadLimit + if head <= 0 { + head = defaultHead + } + + // For non-content modes we need every match to aggregate + // correctly; the post-aggregation cap is applied by the + // tool wrapper. + searchHead := head + if mode != grepModeContent { + searchHead = -1 + } + + res, err := search.Content(ctx, workDir, search.Options{ + Pattern: in.Pattern, + Path: in.Path, + Glob: in.Glob, + CaseInsensitive: in.CaseInsensitive, + HeadLimit: searchHead, + MaxFileSize: cfg.MaxFileSizeBytes, + }) + if err != nil { + return grepOutput{}, err + } + + switch mode { + case grepModeContent: + return buildContentOutput(res), nil + case grepModeFilesWithMatches: + return buildFilesOutput(res, head), nil + case grepModeCount: + return buildCountOutput(res, head), nil + } + return grepOutput{}, fmt.Errorf("unreachable") + }, + ) +} + +func buildContentOutput(res search.Result) grepOutput { + out := grepOutput{Mode: grepModeContent, Total: res.Total} + out.Matches = make([]grepMatchOut, 0, len(res.Matches)) + for _, m := range res.Matches { + out.Matches = append(out.Matches, grepMatchOut{Path: m.Path, Line: m.Line, Text: m.Text}) + } + out.Returned = len(out.Matches) + if res.Truncated { + out.Truncated = true + out.Indicator = fmt.Sprintf("showing first %d of %d matches", out.Returned, res.Total) + } + return out +} + +func buildFilesOutput(res search.Result, head int) grepOutput { + seen := map[string]struct{}{} + files := make([]string, 0) + for _, m := range res.Matches { + if _, ok := seen[m.Path]; ok { + continue + } + seen[m.Path] = struct{}{} + files = append(files, m.Path) + } + sort.Strings(files) + totalFiles := len(files) + truncated := false + if head > 0 && totalFiles > head { + files = files[:head] + truncated = true + } + out := grepOutput{ + Mode: grepModeFilesWithMatches, + Files: files, + Total: totalFiles, + Returned: len(files), + } + if truncated { + out.Truncated = true + out.Indicator = fmt.Sprintf("showing first %d of %d files", out.Returned, totalFiles) + } + return out +} + +func buildCountOutput(res search.Result, head int) grepOutput { + counts := map[string]int{} + for _, m := range res.Matches { + counts[m.Path]++ + } + rows := make([]grepFileCount, 0, len(counts)) + for p, c := range counts { + rows = append(rows, grepFileCount{Path: p, Count: c}) + } + sort.Slice(rows, func(i, j int) bool { + return rows[i].Path < rows[j].Path + }) + totalFiles := len(rows) + truncated := false + if head > 0 && totalFiles > head { + rows = rows[:head] + truncated = true + } + out := grepOutput{ + Mode: grepModeCount, + Counts: rows, + Total: totalFiles, + Returned: len(rows), + } + if truncated { + out.Truncated = true + out.Indicator = fmt.Sprintf("showing first %d of %d files", out.Returned, totalFiles) + } + return out +} diff --git a/pkg/agent/tools/search_test.go b/pkg/agent/tools/search_test.go new file mode 100644 index 0000000..0292175 --- /dev/null +++ b/pkg/agent/tools/search_test.go @@ -0,0 +1,204 @@ +package tools + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +// grepFixture lays out a small workspace used by the grep tool tests. +// +// root/ +// ├── a.go "package a\nfunc Foo() {}\n// TODO: refactor\n" +// ├── b.go "package b\nfunc Bar() {}\n" +// ├── notes.md "Read me\nTODO: write more\n" +// └── pkg/ +// └── deep.go "func Deep() {} // TODO\n" +func grepFixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + files := map[string]string{ + "a.go": "package a\nfunc Foo() {}\n// TODO: refactor\n", + "b.go": "package b\nfunc Bar() {}\n", + "notes.md": "Read me\nTODO: write more\n", + "pkg/deep.go": "func Deep() {} // TODO\n", + } + for rel, body := range files { + full := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + return root +} + +func TestSearchTools(t *testing.T) { + tls := SearchTools("/tmp", SearchConfig{}) + if len(tls) != 1 { + t.Fatalf("expected 1 search tool, got %d", len(tls)) + } + if tls[0].Definition().Name != "grep" { + t.Fatalf("tool name = %q, want grep", tls[0].Definition().Name) + } +} + +func TestGrepTool(t *testing.T) { + t.Run("schema lists pattern as required", func(t *testing.T) { + tl := newGrepTool("/tmp", SearchConfig{}) + def := tl.Definition() + if def.Name != "grep" { + t.Fatalf("name = %q", def.Name) + } + params, _ := def.Parameters["properties"].(map[string]any) + if _, ok := params["pattern"]; !ok { + t.Fatal("schema missing pattern property") + } + req, _ := def.Parameters["required"].([]any) + found := false + for _, name := range req { + if name == "pattern" { + found = true + } + } + if !found { + t.Fatal("pattern should be required in schema") + } + }) + + t.Run("content mode default", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + var out grepOutput + execTool(t, tl, map[string]any{"pattern": "TODO"}, &out) + + if out.Mode != "content" { + t.Fatalf("mode = %q, want content", out.Mode) + } + if out.Total != 3 { + t.Fatalf("total = %d, want 3", out.Total) + } + if len(out.Matches) != 3 { + t.Fatalf("matches len = %d, want 3", len(out.Matches)) + } + }) + + t.Run("files_with_matches mode", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + var out grepOutput + execTool(t, tl, map[string]any{"pattern": "TODO", "output_mode": "files_with_matches"}, &out) + + if out.Mode != "files_with_matches" { + t.Fatalf("mode = %q", out.Mode) + } + if len(out.Files) != 3 { + t.Fatalf("files len = %d, want 3", len(out.Files)) + } + // Lex sorted. + want := []string{"a.go", "notes.md", "pkg/deep.go"} + for i, p := range out.Files { + if p != want[i] { + t.Errorf("file[%d] = %q, want %q", i, p, want[i]) + } + } + }) + + t.Run("count mode", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + var out grepOutput + execTool(t, tl, map[string]any{"pattern": "func", "output_mode": "count"}, &out) + + if out.Mode != "count" { + t.Fatalf("mode = %q", out.Mode) + } + if len(out.Counts) == 0 { + t.Fatal("expected count rows") + } + // All file paths should be set; counts > 0. + for _, row := range out.Counts { + if row.Path == "" || row.Count <= 0 { + t.Errorf("bad row: %+v", row) + } + } + }) + + t.Run("invalid output mode", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + _, err := tl.Execute(context.Background(), map[string]any{ + "pattern": "TODO", + "output_mode": "bogus", + }) + if err == nil { + t.Fatal("expected error for invalid output_mode") + } + }) + + t.Run("head limit truncates with indicator", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + var out grepOutput + execTool(t, tl, map[string]any{ + "pattern": "TODO", + "head_limit": 1, + }, &out) + + if out.Total != 3 { + t.Fatalf("total = %d, want 3", out.Total) + } + if !out.Truncated { + t.Fatal("truncated should be true") + } + if out.Indicator == "" { + t.Fatal("indicator should be populated when truncated") + } + if len(out.Matches) != 1 { + t.Fatalf("matches len = %d, want 1", len(out.Matches)) + } + }) + + t.Run("path scope outside root rejected", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + _, err := tl.Execute(context.Background(), map[string]any{ + "pattern": "TODO", + "path": "../etc", + }) + if err == nil { + t.Fatal("expected error for path escaping root") + } + }) + + t.Run("case insensitive flag", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{}) + var out grepOutput + execTool(t, tl, map[string]any{ + "pattern": "todo", + "case_insensitive": true, + }, &out) + + if out.Total != 3 { + t.Fatalf("total = %d, want 3 (case-insensitive)", out.Total) + } + }) + + t.Run("config max results respected", func(t *testing.T) { + root := grepFixture(t) + tl := newGrepTool(root, SearchConfig{MaxResults: 1}) + var out grepOutput + execTool(t, tl, map[string]any{"pattern": "TODO"}, &out) + + if !out.Truncated { + t.Fatal("expected truncation due to MaxResults=1") + } + if len(out.Matches) != 1 { + t.Fatalf("matches len = %d, want 1", len(out.Matches)) + } + }) +} diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index ae52bad..1b7b141 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -35,6 +35,7 @@ type Config struct { Container *container.Container ProviderNames []string // names of configured providers ModelAliases map[string]string // alias → "provider/model" + Tools tools.Config // tool-specific configuration; zero values pick sensible defaults } // Run is the main daemon loop. It manages the PID file, socket server, watchdog, @@ -338,6 +339,7 @@ func initSessionManager(ctx context.Context, cfg Config, h *home.Home, tgAuth *a Providers: cfg.ProviderNames, Models: cfg.ModelAliases, }, + Tools: cfg.Tools, } return NewSessionManager(ctx, agentCfg, cfg.Container) diff --git a/pkg/search/search.go b/pkg/search/search.go new file mode 100644 index 0000000..cc04c98 --- /dev/null +++ b/pkg/search/search.go @@ -0,0 +1,257 @@ +// Package search provides workspace content search primitives for the agent's +// tooling layer. The package is dependency-free of tool/agent layers so that +// it can be tested directly against on-disk fixtures. +// +// Content searches files under a root directory for a regex pattern, applying +// a workspace-boundary check, a default skip list, and binary-file filtering. +package search + +import ( + "bufio" + "bytes" + "context" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "regexp" + "sort" + "strings" +) + +// DefaultSkipDirs is the standard list of directory base names that the search +// package skips when walking a workspace tree. +var DefaultSkipDirs = []string{".git", "node_modules", "vendor", "dist", "build", "target"} + +// DefaultMaxFileSize is the default per-file size cap (1 MiB). Files larger +// than this are skipped silently. +const DefaultMaxFileSize int64 = 1 << 20 + +// DefaultHeadLimit is the default cap on Match entries returned by Content +// when Options.HeadLimit is zero. +const DefaultHeadLimit = 200 + +// binarySniffSize is the number of leading bytes inspected to classify a file +// as text or binary. +const binarySniffSize = 512 + +// Options configures a Content search. +type Options struct { + // Pattern is the regular expression to match against each line. Required. + Pattern string + // Path optionally scopes the walk to a sub-path under root. Empty means + // walk root directly. Both relative (joined to root) and absolute paths + // are accepted, but the resolved path must remain under root. + Path string + // Glob optionally filters files by basename glob (filepath.Match syntax, + // e.g. "*.go"). Empty means no filename filter. + Glob string + // CaseInsensitive enables case-insensitive matching. + CaseInsensitive bool + // HeadLimit caps the number of returned Match entries. Zero means use + // DefaultHeadLimit. A negative value disables capping entirely. + HeadLimit int + // MaxFileSize skips files larger than this. Zero means use DefaultMaxFileSize. + MaxFileSize int64 + // SkipDirs overrides the default skip-dir list. Nil means use DefaultSkipDirs. + SkipDirs []string +} + +// Match is a single content match: the file (workspace-relative, slash-separated), +// the 1-based line number, and the matched line's text. +type Match struct { + Path string + Line int + Text string +} + +// Result carries the matches plus truncation metadata. +// +// Total is the total number of matches found across the scan; len(Matches) is +// at most HeadLimit. Truncated is true when Total > len(Matches). +type Result struct { + Matches []Match + Total int + Truncated bool +} + +// Content walks root, opens text files matching the optional path/glob filters, +// and returns lines matching opts.Pattern. Results are sorted by (path, line) +// for deterministic ordering across calls. +func Content(ctx context.Context, root string, opts Options) (Result, error) { + if opts.Pattern == "" { + return Result{}, errors.New("search: pattern is required") + } + + root = filepath.Clean(root) + walkRoot := root + if opts.Path != "" { + sub, err := resolveUnderRoot(root, opts.Path) + if err != nil { + return Result{}, err + } + walkRoot = sub + } + + re, err := compilePattern(opts.Pattern, opts.CaseInsensitive) + if err != nil { + return Result{}, err + } + + skipSet := buildSkipSet(opts.SkipDirs) + + maxSize := opts.MaxFileSize + if maxSize == 0 { + maxSize = DefaultMaxFileSize + } + + var matches []Match + walkErr := filepath.WalkDir(walkRoot, func(path string, d fs.DirEntry, werr error) error { + if cerr := ctx.Err(); cerr != nil { + return cerr + } + if werr != nil { + if d != nil && d.IsDir() { + return fs.SkipDir + } + return nil + } + if d.IsDir() { + if path != walkRoot { + if _, skip := skipSet[d.Name()]; skip { + return fs.SkipDir + } + } + return nil + } + if !d.Type().IsRegular() { + return nil + } + + info, ierr := d.Info() + if ierr != nil || info.Size() > maxSize { + return nil + } + + if opts.Glob != "" { + ok, _ := filepath.Match(opts.Glob, d.Name()) + if !ok { + return nil + } + } + + rel, rerr := filepath.Rel(root, path) + if rerr != nil { + return nil + } + relSlash := filepath.ToSlash(rel) + + fileMatches, ferr := scanFile(path, re) + if ferr != nil { + return nil + } + for _, m := range fileMatches { + m.Path = relSlash + matches = append(matches, m) + } + return nil + }) + if walkErr != nil { + return Result{}, walkErr + } + + sort.SliceStable(matches, func(i, j int) bool { + if matches[i].Path != matches[j].Path { + return matches[i].Path < matches[j].Path + } + return matches[i].Line < matches[j].Line + }) + + total := len(matches) + head := opts.HeadLimit + if head == 0 { + head = DefaultHeadLimit + } + truncated := false + if head > 0 && total > head { + matches = matches[:head] + truncated = true + } + return Result{Matches: matches, Total: total, Truncated: truncated}, nil +} + +// resolveUnderRoot normalizes p (relative to root if not absolute) and refuses +// any result that escapes the root tree. +func resolveUnderRoot(root, p string) (string, error) { + abs := p + if !filepath.IsAbs(abs) { + abs = filepath.Join(root, abs) + } + abs = filepath.Clean(abs) + rel, err := filepath.Rel(root, abs) + if err != nil { + return "", fmt.Errorf("search: path %q: %w", p, err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("search: path %q is outside workspace root", p) + } + return abs, nil +} + +func compilePattern(pat string, caseInsensitive bool) (*regexp.Regexp, error) { + if caseInsensitive { + pat = "(?i)" + pat + } + re, err := regexp.Compile(pat) + if err != nil { + return nil, fmt.Errorf("search: invalid pattern: %w", err) + } + return re, nil +} + +func buildSkipSet(override []string) map[string]struct{} { + src := override + if src == nil { + src = DefaultSkipDirs + } + out := make(map[string]struct{}, len(src)) + for _, d := range src { + out[d] = struct{}{} + } + return out +} + +// scanFile sniffs the leading bytes for a NUL byte (binary heuristic), and +// otherwise streams the file line-by-line applying re. Returns matches with +// 1-based line numbers and Path left empty (caller fills in). +func scanFile(path string, re *regexp.Regexp) ([]Match, error) { + f, err := os.Open(path) // #nosec G304 -- path comes from a bounded WalkDir under workspace root + if err != nil { + return nil, err + } + defer f.Close() + + head := make([]byte, binarySniffSize) + n, _ := io.ReadFull(f, head) + if n > 0 && bytes.IndexByte(head[:n], 0) >= 0 { + return nil, nil + } + if _, err := f.Seek(0, io.SeekStart); err != nil { + return nil, err + } + + var matches []Match + sc := bufio.NewScanner(f) + sc.Buffer(make([]byte, 0, 64*1024), 1024*1024) + line := 0 + for sc.Scan() { + line++ + text := sc.Text() + if re.MatchString(text) { + matches = append(matches, Match{Line: line, Text: text}) + } + } + return matches, nil +} diff --git a/pkg/search/search_test.go b/pkg/search/search_test.go new file mode 100644 index 0000000..d9b6a7c --- /dev/null +++ b/pkg/search/search_test.go @@ -0,0 +1,241 @@ +package search + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +// fixture builds a small workspace under t.TempDir(). +// +// root/ +// ├── a.go "package a\nfunc Foo() {}\n// TODO: refactor\n" +// ├── b.go "package b\nfunc Bar() {}\n" +// ├── notes.md "Read me\nTODO: write more\n" +// ├── bin.dat (binary, contains NUL) +// ├── .git/HEAD "ref: refs/heads/main" (must be skipped) +// ├── node_modules/x.js "var TODO = 1" (must be skipped) +// └── pkg/ +// ├── nested/ +// │ └── deep.go "func Deep() {} // TODO\n" +// └── other.go "func Other() {}\n" +func fixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + + files := map[string]string{ + "a.go": "package a\nfunc Foo() {}\n// TODO: refactor\n", + "b.go": "package b\nfunc Bar() {}\n", + "notes.md": "Read me\nTODO: write more\n", + ".git/HEAD": "ref: refs/heads/main", + "node_modules/x.js": "var TODO = 1", + "pkg/nested/deep.go": "func Deep() {} // TODO\n", + "pkg/other.go": "func Other() {}\n", + } + for rel, content := range files { + full := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + // Binary file with embedded NUL. + bin := []byte{0x89, 'P', 'N', 'G', 0x00, 'T', 'O', 'D', 'O'} + if err := os.WriteFile(filepath.Join(root, "bin.dat"), bin, 0o644); err != nil { + t.Fatal(err) + } + return root +} + +func TestContent(t *testing.T) { + t.Run("basic match counts and order", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "TODO"}) + if err != nil { + t.Fatal(err) + } + want := []Match{ + {Path: "a.go", Line: 3, Text: "// TODO: refactor"}, + {Path: "notes.md", Line: 2, Text: "TODO: write more"}, + {Path: "pkg/nested/deep.go", Line: 1, Text: "func Deep() {} // TODO"}, + } + if got.Total != len(want) { + t.Fatalf("total = %d, want %d (matches=%v)", got.Total, len(want), got.Matches) + } + if len(got.Matches) != len(want) { + t.Fatalf("matches len = %d, want %d", len(got.Matches), len(want)) + } + for i, m := range got.Matches { + if m != want[i] { + t.Errorf("match[%d] = %+v, want %+v", i, m, want[i]) + } + } + }) + + t.Run("binary file skipped", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "TODO"}) + if err != nil { + t.Fatal(err) + } + for _, m := range got.Matches { + if m.Path == "bin.dat" { + t.Fatalf("binary file should have been skipped, got match: %+v", m) + } + } + }) + + t.Run("default skip dirs are skipped", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "TODO"}) + if err != nil { + t.Fatal(err) + } + for _, m := range got.Matches { + if strings.HasPrefix(m.Path, ".git/") || strings.HasPrefix(m.Path, "node_modules/") { + t.Fatalf("skip dir leaked into results: %+v", m) + } + } + }) + + t.Run("head limit truncates with total preserved", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "TODO", HeadLimit: 2}) + if err != nil { + t.Fatal(err) + } + if got.Total != 3 { + t.Fatalf("total = %d, want 3", got.Total) + } + if len(got.Matches) != 2 { + t.Fatalf("matches len = %d, want 2", len(got.Matches)) + } + if !got.Truncated { + t.Fatal("truncated should be true") + } + }) + + t.Run("case insensitive", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "todo", CaseInsensitive: true}) + if err != nil { + t.Fatal(err) + } + if got.Total < 3 { + t.Fatalf("expected >=3 matches case-insensitive, got %d", got.Total) + } + }) + + t.Run("path scope under root", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "func", Path: "pkg"}) + if err != nil { + t.Fatal(err) + } + for _, m := range got.Matches { + if !strings.HasPrefix(m.Path, "pkg/") { + t.Errorf("unexpected match outside pkg/: %+v", m) + } + } + if got.Total < 2 { + t.Fatalf("expected >=2 matches under pkg/, got %d", got.Total) + } + }) + + t.Run("path outside root rejected", func(t *testing.T) { + root := fixture(t) + _, err := Content(context.Background(), root, Options{Pattern: "x", Path: "../etc"}) + if err == nil { + t.Fatal("expected error for path escaping root") + } + }) + + t.Run("absolute path outside root rejected", func(t *testing.T) { + root := fixture(t) + _, err := Content(context.Background(), root, Options{Pattern: "x", Path: "/tmp"}) + if err == nil { + t.Fatal("expected error for absolute path outside root") + } + }) + + t.Run("glob filename filter", func(t *testing.T) { + root := fixture(t) + got, err := Content(context.Background(), root, Options{Pattern: "TODO", Glob: "*.md"}) + if err != nil { + t.Fatal(err) + } + if got.Total != 1 { + t.Fatalf("total = %d, want 1 (matches=%v)", got.Total, got.Matches) + } + if got.Matches[0].Path != "notes.md" { + t.Fatalf("path = %q, want notes.md", got.Matches[0].Path) + } + }) + + t.Run("deterministic ordering across calls", func(t *testing.T) { + root := fixture(t) + first, err := Content(context.Background(), root, Options{Pattern: "func"}) + if err != nil { + t.Fatal(err) + } + second, err := Content(context.Background(), root, Options{Pattern: "func"}) + if err != nil { + t.Fatal(err) + } + if len(first.Matches) != len(second.Matches) { + t.Fatalf("len differs: %d vs %d", len(first.Matches), len(second.Matches)) + } + for i := range first.Matches { + if first.Matches[i] != second.Matches[i] { + t.Errorf("match[%d] differs: %+v vs %+v", i, first.Matches[i], second.Matches[i]) + } + } + }) + + t.Run("max file size skips large files", func(t *testing.T) { + root := t.TempDir() + large := strings.Repeat("needle\n", 1000) + small := "needle\n" + if err := os.WriteFile(filepath.Join(root, "large.txt"), []byte(large), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "small.txt"), []byte(small), 0o644); err != nil { + t.Fatal(err) + } + got, err := Content(context.Background(), root, Options{ + Pattern: "needle", + MaxFileSize: 32, // small.txt fits, large.txt doesn't + }) + if err != nil { + t.Fatal(err) + } + for _, m := range got.Matches { + if m.Path == "large.txt" { + t.Fatalf("large.txt should have been skipped, got match: %+v", m) + } + } + if got.Total == 0 { + t.Fatal("expected small.txt match, got none") + } + }) + + t.Run("invalid regex returns error", func(t *testing.T) { + root := t.TempDir() + _, err := Content(context.Background(), root, Options{Pattern: "([unclosed"}) + if err == nil { + t.Fatal("expected error for invalid regex") + } + }) + + t.Run("empty pattern returns error", func(t *testing.T) { + root := t.TempDir() + _, err := Content(context.Background(), root, Options{}) + if err == nil { + t.Fatal("expected error for empty pattern") + } + }) +} diff --git a/x/config/load.go b/x/config/load.go index d6d4e74..c09948c 100644 --- a/x/config/load.go +++ b/x/config/load.go @@ -19,6 +19,18 @@ type ProviderConfig struct { GoogleSearch bool `yaml:"google_search"` } +// GrepConfig configures the grep tool. Zero values fall through to the +// defaults baked into pkg/search. +type GrepConfig struct { + MaxResults int `yaml:"max_results"` + MaxFileSizeBytes int64 `yaml:"max_file_size_bytes"` +} + +// ToolsConfig groups per-tool configuration loaded from the YAML "tools" block. +type ToolsConfig struct { + Grep GrepConfig `yaml:"grep"` +} + // Config is the top-level application configuration. type Config struct { Providers map[string]ProviderConfig `yaml:"providers"` @@ -26,6 +38,7 @@ type Config struct { DefaultModel string `yaml:"default_model"` MemoryModel string `yaml:"memory_model"` TelegramToken string `yaml:"telegram_token"` + Tools ToolsConfig `yaml:"tools"` } // Load reads and validates a YAML config file from the given path. diff --git a/x/config/load_test.go b/x/config/load_test.go index 6913030..0fa0ddc 100644 --- a/x/config/load_test.go +++ b/x/config/load_test.go @@ -655,4 +655,56 @@ extra_top_level: ignored t.Errorf("DefaultModel = %q, want %q", cfg.DefaultModel, "gemini/gemini-3-flash-preview") } }) + + t.Run("ParsesToolsBlock", func(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "wildgecu.yaml") + data := []byte(`providers: + gemini: + type: gemini + api_key: key +default_model: gemini/gemini-3-flash-preview +tools: + grep: + max_results: 50 + max_file_size_bytes: 524288 +`) + if err := os.WriteFile(cfgPath, data, 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Load(cfgPath) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Tools.Grep.MaxResults != 50 { + t.Errorf("Tools.Grep.MaxResults = %d, want 50", cfg.Tools.Grep.MaxResults) + } + if cfg.Tools.Grep.MaxFileSizeBytes != 524288 { + t.Errorf("Tools.Grep.MaxFileSizeBytes = %d, want 524288", cfg.Tools.Grep.MaxFileSizeBytes) + } + }) + + t.Run("ToolsBlockOptional", func(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "wildgecu.yaml") + data := []byte(`providers: + gemini: + type: gemini + api_key: key +default_model: gemini/gemini-3-flash-preview +`) + if err := os.WriteFile(cfgPath, data, 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Load(cfgPath) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + // Zero values are fine; defaults are applied at the tool layer. + if cfg.Tools.Grep.MaxResults != 0 { + t.Errorf("Tools.Grep.MaxResults default = %d, want 0", cfg.Tools.Grep.MaxResults) + } + }) } From a25c772f4f0071d4e49a8fa10c0b53156f6530fc Mon Sep 17 00:00:00 2001 From: Ludovico Russo Date: Thu, 30 Apr 2026 10:17:33 +0200 Subject: [PATCH 2/5] feat(tools): glob tool + pkg/search.Paths (#80) Second slice of PRD #76. Adds the glob tool the agent can call to find files by doublestar pattern (e.g. **/*_test.go, pkg/**/agent.go), extending pkg/search with a Paths() function that shares the workspace walk semantics introduced for grep in #77. **pkg/search.** Paths(ctx, root, pattern, opts) walks root (or an optional sub-path scope), tests each regular file's workspace-relative slash path against the doublestar pattern, and returns matches with truncation metadata. Reuses Content's default skip-list (.git, node_modules, vendor, dist, build, target) and the same resolveUnderRoot boundary check, so escaping the root is rejected at the package layer. Default sort is mtime-desc (Claude Code convention) with a stable path tiebreaker; SortLex is opt-in. DefaultPathsMaxResults = 1000. **glob tool.** Thin adapter at pkg/agent/tools/search.go. Schema: pattern (required), path (scope), sort (mtime_desc | lex), max_results. Returns {paths, total, returned, truncated, indicator} where indicator ("showing first N of M paths") fires when results are capped. Wired into SearchTools alongside grep so chat, code, and subagent registries pick it up automatically. **Tests.** pkg/search covers doublestar correctness (**/*.go vs pkg/*.go, **/*_test.go), mtime-desc default vs lex, max_results truncation, default skip dirs, path scope, absolute/relative boundary rejection, invalid sort, empty/invalid pattern. Tool-layer covers schema (pattern required), default sort, lex sort, truncation + indicator, path-escape rejection, and invalid sort. CODE_AGENT.md gains a glob bullet under Search and notes preferring it over recursive list_files. --- go.mod | 1 + go.sum | 2 + pkg/agent/CODE_AGENT.md | 6 +- pkg/agent/tools/search.go | 50 +++++++- pkg/agent/tools/search_test.go | 143 +++++++++++++++++++++- pkg/search/search.go | 154 +++++++++++++++++++++++ pkg/search/search_test.go | 216 +++++++++++++++++++++++++++++++++ 7 files changed, 564 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index fea49ad..52ae312 100644 --- a/go.mod +++ b/go.mod @@ -28,6 +28,7 @@ require ( github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/aymerick/douceur v0.2.0 // indirect + github.com/bmatcuk/doublestar/v4 v4.10.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/colorprofile v0.4.3 // indirect github.com/charmbracelet/x/ansi v0.11.6 // indirect diff --git a/go.sum b/go.sum index dc6aa09..18e1066 100644 --- a/go.sum +++ b/go.sum @@ -23,6 +23,8 @@ github.com/aymanbagabas/go-udiff v0.3.1 h1:LV+qyBQ2pqe0u42ZsUEtPiCaUoqgA9gYRDs3v github.com/aymanbagabas/go-udiff v0.3.1/go.mod h1:G0fsKmG+P6ylD0r6N/KgQD/nWzgfnl8ZBcNLgcbrw8E= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= +github.com/bmatcuk/doublestar/v4 v4.10.0 h1:zU9WiOla1YA122oLM6i4EXvGW62DvKZVxIe6TYWexEs= +github.com/bmatcuk/doublestar/v4 v4.10.0/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/charmbracelet/bubbles v1.0.0 h1:12J8/ak/uCZEMQ6KU7pcfwceyjLlWsDLAxB5fXonfvc= diff --git a/pkg/agent/CODE_AGENT.md b/pkg/agent/CODE_AGENT.md index fcf55e6..3b59348 100644 --- a/pkg/agent/CODE_AGENT.md +++ b/pkg/agent/CODE_AGENT.md @@ -15,13 +15,15 @@ You have dedicated file tools. **Always prefer them over bash for file I/O:** ## Search -You also have a dedicated content-search tool. **Prefer it over shelling out to `grep` or `rg`:** +You have dedicated search tools. **Prefer them over shelling out to `grep`/`rg`/`find`, and over recursing through `list_files` calls:** - **`grep`** — Search file contents by regex across the workspace. Supports `path` to scope to a subtree, `glob` for filename filters (e.g. `*.go`), `case_insensitive`, `head_limit`, and three output modes: - `content` (default) returns `{path, line, text}` entries. - `files_with_matches` returns just the matching paths. - `count` returns per-file match counts. - Skips `.git`, `node_modules`, `vendor`, `dist`, `build`, `target`, and binary files automatically. +- **`glob`** — Find files by doublestar path pattern (e.g. `**/*_test.go`, `pkg/**/agent.go`). Supports `path` to scope to a subtree, `sort` (`mtime_desc` default — most recently modified first; `lex` for lexicographic), and `max_results` (default 1000). Prefer this over recursive `list_files`. + +Both tools skip `.git`, `node_modules`, `vendor`, `dist`, `build`, `target`, and (for `grep`) binary files automatically. ## Bash diff --git a/pkg/agent/tools/search.go b/pkg/agent/tools/search.go index c846c2b..7bee09c 100644 --- a/pkg/agent/tools/search.go +++ b/pkg/agent/tools/search.go @@ -27,9 +27,12 @@ const ( grepModeCount = "count" ) -// SearchTools returns search tools (grep) bound to workDir. +// SearchTools returns search tools (grep, glob) bound to workDir. func SearchTools(workDir string, cfg SearchConfig) []tool.Tool { - return []tool.Tool{newGrepTool(workDir, cfg)} + return []tool.Tool{ + newGrepTool(workDir, cfg), + newGlobTool(workDir), + } } type grepInput struct { @@ -162,6 +165,49 @@ func buildFilesOutput(res search.Result, head int) grepOutput { return out } +type globInput struct { + Pattern string `json:"pattern" description:"Doublestar path pattern matched against workspace-relative paths (e.g. **/*.go, pkg/**/agent.go)"` + Path string `json:"path,omitempty" description:"Subdirectory under the workspace to scope the search (defaults to workspace root)"` + Sort string `json:"sort,omitempty" description:"Result ordering: mtime_desc (default, most recently modified first) | lex"` + MaxResults int `json:"max_results,omitempty" description:"Cap on returned paths. Default 1000."` +} + +type globOutput struct { + Paths []string `json:"paths"` + Total int `json:"total"` + Returned int `json:"returned"` + Truncated bool `json:"truncated,omitempty"` + Indicator string `json:"indicator,omitempty"` +} + +func newGlobTool(workDir string) tool.Tool { + return tool.NewTool("glob", + "Find files by doublestar path pattern (e.g. **/*_test.go, pkg/**/agent.go). "+ + "Prefer this over recursive list_files. Defaults to mtime-descending order; "+ + "pass sort=lex for lexicographic. Skips .git, node_modules, vendor, dist, build, target.", + func(ctx context.Context, in globInput) (globOutput, error) { + res, err := search.Paths(ctx, workDir, in.Pattern, search.PathOptions{ + Path: in.Path, + Sort: in.Sort, + MaxResults: in.MaxResults, + }) + if err != nil { + return globOutput{}, err + } + out := globOutput{ + Paths: res.Paths, + Total: res.Total, + Returned: len(res.Paths), + } + if res.Truncated { + out.Truncated = true + out.Indicator = fmt.Sprintf("showing first %d of %d paths", out.Returned, res.Total) + } + return out, nil + }, + ) +} + func buildCountOutput(res search.Result, head int) grepOutput { counts := map[string]int{} for _, m := range res.Matches { diff --git a/pkg/agent/tools/search_test.go b/pkg/agent/tools/search_test.go index 0292175..98550b1 100644 --- a/pkg/agent/tools/search_test.go +++ b/pkg/agent/tools/search_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "testing" + "time" ) // grepFixture lays out a small workspace used by the grep tool tests. @@ -38,11 +39,14 @@ func grepFixture(t *testing.T) string { func TestSearchTools(t *testing.T) { tls := SearchTools("/tmp", SearchConfig{}) - if len(tls) != 1 { - t.Fatalf("expected 1 search tool, got %d", len(tls)) + want := []string{"grep", "glob"} + if len(tls) != len(want) { + t.Fatalf("expected %d search tools, got %d", len(want), len(tls)) } - if tls[0].Definition().Name != "grep" { - t.Fatalf("tool name = %q, want grep", tls[0].Definition().Name) + for i, name := range want { + if tls[i].Definition().Name != name { + t.Fatalf("tool[%d] name = %q, want %q", i, tls[i].Definition().Name, name) + } } } @@ -202,3 +206,134 @@ func TestGrepTool(t *testing.T) { } }) } + +// globFixture lays out a small workspace whose Go files have a known mtime +// ladder so default mtime-desc ordering is testable. +func globFixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + files := []string{ + "a.go", + "pkg/agent.go", + "pkg/nested/deep.go", + "README.md", + } + for _, rel := range files { + full := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("// "+rel+"\n"), 0o644); err != nil { + t.Fatal(err) + } + } + // Apply mtimes oldest-first so reverse order is the mtime-desc result. + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for i, rel := range files { + full := filepath.Join(root, filepath.FromSlash(rel)) + mt := base.Add(time.Duration(i) * time.Hour) + if err := os.Chtimes(full, mt, mt); err != nil { + t.Fatal(err) + } + } + return root +} + +func TestGlobTool(t *testing.T) { + t.Run("schema lists pattern as required", func(t *testing.T) { + tl := newGlobTool("/tmp") + def := tl.Definition() + if def.Name != "glob" { + t.Fatalf("name = %q", def.Name) + } + params, _ := def.Parameters["properties"].(map[string]any) + if _, ok := params["pattern"]; !ok { + t.Fatal("schema missing pattern property") + } + req, _ := def.Parameters["required"].([]any) + found := false + for _, name := range req { + if name == "pattern" { + found = true + } + } + if !found { + t.Fatal("pattern should be required in schema") + } + }) + + t.Run("default sort is mtime descending", func(t *testing.T) { + root := globFixture(t) + tl := newGlobTool(root) + var out globOutput + execTool(t, tl, map[string]any{"pattern": "**/*.go"}, &out) + + want := []string{"pkg/nested/deep.go", "pkg/agent.go", "a.go"} + if len(out.Paths) != len(want) { + t.Fatalf("paths len = %d, want %d (paths=%v)", len(out.Paths), len(want), out.Paths) + } + for i, p := range out.Paths { + if p != want[i] { + t.Errorf("paths[%d] = %q, want %q", i, p, want[i]) + } + } + }) + + t.Run("lex sort", func(t *testing.T) { + root := globFixture(t) + tl := newGlobTool(root) + var out globOutput + execTool(t, tl, map[string]any{"pattern": "**/*.go", "sort": "lex"}, &out) + + want := []string{"a.go", "pkg/agent.go", "pkg/nested/deep.go"} + for i, p := range out.Paths { + if p != want[i] { + t.Errorf("paths[%d] = %q, want %q", i, p, want[i]) + } + } + }) + + t.Run("max results truncates with indicator", func(t *testing.T) { + root := globFixture(t) + tl := newGlobTool(root) + var out globOutput + execTool(t, tl, map[string]any{"pattern": "**/*.go", "sort": "lex", "max_results": 1}, &out) + + if out.Total != 3 { + t.Fatalf("total = %d, want 3", out.Total) + } + if !out.Truncated { + t.Fatal("truncated should be true") + } + if out.Indicator == "" { + t.Fatal("indicator should be populated when truncated") + } + if len(out.Paths) != 1 { + t.Fatalf("paths len = %d, want 1", len(out.Paths)) + } + }) + + t.Run("path scope outside root rejected", func(t *testing.T) { + root := globFixture(t) + tl := newGlobTool(root) + _, err := tl.Execute(context.Background(), map[string]any{ + "pattern": "**/*.go", + "path": "../etc", + }) + if err == nil { + t.Fatal("expected error for path escaping root") + } + }) + + t.Run("invalid sort rejected", func(t *testing.T) { + root := globFixture(t) + tl := newGlobTool(root) + _, err := tl.Execute(context.Background(), map[string]any{ + "pattern": "**/*.go", + "sort": "weird", + }) + if err == nil { + t.Fatal("expected error for invalid sort") + } + }) +} diff --git a/pkg/search/search.go b/pkg/search/search.go index cc04c98..0c0ba9e 100644 --- a/pkg/search/search.go +++ b/pkg/search/search.go @@ -19,6 +19,9 @@ import ( "regexp" "sort" "strings" + "time" + + "github.com/bmatcuk/doublestar/v4" ) // DefaultSkipDirs is the standard list of directory base names that the search @@ -33,6 +36,17 @@ const DefaultMaxFileSize int64 = 1 << 20 // when Options.HeadLimit is zero. const DefaultHeadLimit = 200 +// DefaultPathsMaxResults is the default cap on paths returned by Paths when +// PathOptions.MaxResults is zero. +const DefaultPathsMaxResults = 1000 + +// SortMtimeDesc sorts paths by modification time, most recent first +// (Claude Code convention). Stable tiebreaker on path. +const SortMtimeDesc = "mtime_desc" + +// SortLex sorts paths lexicographically, ascending. +const SortLex = "lex" + // binarySniffSize is the number of leading bytes inspected to classify a file // as text or binary. const binarySniffSize = 512 @@ -255,3 +269,143 @@ func scanFile(path string, re *regexp.Regexp) ([]Match, error) { } return matches, nil } + +// PathOptions configures a Paths search. +type PathOptions struct { + // Path optionally scopes the walk to a sub-path under root. Empty means + // walk root directly. Both relative (joined to root) and absolute paths + // are accepted, but the resolved path must remain under root. + Path string + // Sort selects the result ordering. Empty or SortMtimeDesc orders by + // modification time descending (most recently modified first); SortLex + // orders lexicographically ascending. + Sort string + // MaxResults caps the number of returned paths. Zero means use + // DefaultPathsMaxResults. A negative value disables capping entirely. + MaxResults int + // SkipDirs overrides the default skip-dir list. Nil means use DefaultSkipDirs. + SkipDirs []string +} + +// PathsResult carries the matched paths plus truncation metadata. +// +// Total is the total number of paths found across the scan; len(Paths) is at +// most MaxResults. Truncated is true when Total > len(Paths). +type PathsResult struct { + Paths []string + Total int + Truncated bool +} + +// Paths walks root and returns regular-file paths whose workspace-relative, +// slash-separated path matches the given doublestar pattern (e.g. "**/*.go", +// "pkg/**/agent.go"). Default skip-dirs and the workspace boundary check are +// shared with Content. +func Paths(ctx context.Context, root, pattern string, opts PathOptions) (PathsResult, error) { + if pattern == "" { + return PathsResult{}, errors.New("search: pattern is required") + } + if !doublestar.ValidatePattern(pattern) { + return PathsResult{}, fmt.Errorf("search: invalid pattern %q", pattern) + } + + root = filepath.Clean(root) + walkRoot := root + if opts.Path != "" { + sub, err := resolveUnderRoot(root, opts.Path) + if err != nil { + return PathsResult{}, err + } + walkRoot = sub + } + + sortMode := opts.Sort + if sortMode == "" { + sortMode = SortMtimeDesc + } + if sortMode != SortMtimeDesc && sortMode != SortLex { + return PathsResult{}, fmt.Errorf("search: invalid sort %q (want %q or %q)", sortMode, SortMtimeDesc, SortLex) + } + + skipSet := buildSkipSet(opts.SkipDirs) + + type entry struct { + path string + mtime time.Time + } + var entries []entry + walkErr := filepath.WalkDir(walkRoot, func(path string, d fs.DirEntry, werr error) error { + if cerr := ctx.Err(); cerr != nil { + return cerr + } + if werr != nil { + if d != nil && d.IsDir() { + return fs.SkipDir + } + return nil + } + if d.IsDir() { + if path != walkRoot { + if _, skip := skipSet[d.Name()]; skip { + return fs.SkipDir + } + } + return nil + } + if !d.Type().IsRegular() { + return nil + } + + rel, rerr := filepath.Rel(root, path) + if rerr != nil { + return nil + } + relSlash := filepath.ToSlash(rel) + + ok, mErr := doublestar.Match(pattern, relSlash) + if mErr != nil || !ok { + return nil + } + + info, ierr := d.Info() + if ierr != nil { + return nil + } + entries = append(entries, entry{path: relSlash, mtime: info.ModTime()}) + return nil + }) + if walkErr != nil { + return PathsResult{}, walkErr + } + + switch sortMode { + case SortLex: + sort.Slice(entries, func(i, j int) bool { + return entries[i].path < entries[j].path + }) + default: + sort.SliceStable(entries, func(i, j int) bool { + if !entries[i].mtime.Equal(entries[j].mtime) { + return entries[i].mtime.After(entries[j].mtime) + } + return entries[i].path < entries[j].path + }) + } + + total := len(entries) + limit := opts.MaxResults + if limit == 0 { + limit = DefaultPathsMaxResults + } + truncated := false + if limit > 0 && total > limit { + entries = entries[:limit] + truncated = true + } + + paths := make([]string, len(entries)) + for i, e := range entries { + paths[i] = e.path + } + return PathsResult{Paths: paths, Total: total, Truncated: truncated}, nil +} diff --git a/pkg/search/search_test.go b/pkg/search/search_test.go index d9b6a7c..4aec69e 100644 --- a/pkg/search/search_test.go +++ b/pkg/search/search_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) // fixture builds a small workspace under t.TempDir(). @@ -239,3 +240,218 @@ func TestContent(t *testing.T) { } }) } + +// pathsFixture builds a workspace with files at known relative paths and +// applies a deterministic mtime ladder so mtime-desc sort is testable. +// +// root/ +// ├── README.md +// ├── a.go +// ├── pkg/agent.go +// ├── pkg/nested/deep.go +// ├── pkg/nested/agent_test.go +// ├── .git/HEAD (skipped) +// └── node_modules/x.js (skipped) +// +// Files are touched in the order listed below so that touchOrder[0] has the +// oldest mtime and touchOrder[len-1] is the newest. +func pathsFixture(t *testing.T) (root string, touchOrder []string) { + t.Helper() + root = t.TempDir() + + files := []string{ + "README.md", + "a.go", + "pkg/agent.go", + "pkg/nested/deep.go", + "pkg/nested/agent_test.go", + ".git/HEAD", + "node_modules/x.js", + } + for _, rel := range files { + full := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte("// "+rel+"\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + touchOrder = []string{ + "a.go", + "pkg/agent.go", + "README.md", + "pkg/nested/deep.go", + "pkg/nested/agent_test.go", + } + base := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for i, rel := range touchOrder { + full := filepath.Join(root, filepath.FromSlash(rel)) + mt := base.Add(time.Duration(i) * time.Hour) + if err := os.Chtimes(full, mt, mt); err != nil { + t.Fatal(err) + } + } + return root, touchOrder +} + +func TestPaths(t *testing.T) { + t.Run("matches all go files via doublestar", func(t *testing.T) { + root, _ := pathsFixture(t) + got, err := Paths(context.Background(), root, "**/*.go", PathOptions{Sort: SortLex}) + if err != nil { + t.Fatal(err) + } + want := []string{ + "a.go", + "pkg/agent.go", + "pkg/nested/agent_test.go", + "pkg/nested/deep.go", + } + if got.Total != len(want) { + t.Fatalf("total = %d, want %d (paths=%v)", got.Total, len(want), got.Paths) + } + for i, p := range got.Paths { + if p != want[i] { + t.Errorf("paths[%d] = %q, want %q", i, p, want[i]) + } + } + }) + + t.Run("single-star scoped to package directory", func(t *testing.T) { + root, _ := pathsFixture(t) + got, err := Paths(context.Background(), root, "pkg/*.go", PathOptions{Sort: SortLex}) + if err != nil { + t.Fatal(err) + } + want := []string{"pkg/agent.go"} + if len(got.Paths) != len(want) { + t.Fatalf("paths len = %d, want %d (paths=%v)", len(got.Paths), len(want), got.Paths) + } + if got.Paths[0] != want[0] { + t.Errorf("paths[0] = %q, want %q", got.Paths[0], want[0]) + } + }) + + t.Run("test-file pattern", func(t *testing.T) { + root, _ := pathsFixture(t) + got, err := Paths(context.Background(), root, "**/*_test.go", PathOptions{Sort: SortLex}) + if err != nil { + t.Fatal(err) + } + if len(got.Paths) != 1 || got.Paths[0] != "pkg/nested/agent_test.go" { + t.Fatalf("paths = %v, want [pkg/nested/agent_test.go]", got.Paths) + } + }) + + t.Run("default sort is mtime descending", func(t *testing.T) { + root, touchOrder := pathsFixture(t) + got, err := Paths(context.Background(), root, "**/*.go", PathOptions{}) + if err != nil { + t.Fatal(err) + } + // touchOrder lists oldest-first; the .go subset reversed should equal + // the mtime-desc ordering. + var goOrder []string + for i := len(touchOrder) - 1; i >= 0; i-- { + if strings.HasSuffix(touchOrder[i], ".go") { + goOrder = append(goOrder, touchOrder[i]) + } + } + if len(got.Paths) != len(goOrder) { + t.Fatalf("paths len = %d, want %d (paths=%v)", len(got.Paths), len(goOrder), got.Paths) + } + for i, p := range got.Paths { + if p != goOrder[i] { + t.Errorf("paths[%d] = %q, want %q (full=%v)", i, p, goOrder[i], got.Paths) + } + } + }) + + t.Run("max results truncates with total preserved", func(t *testing.T) { + root, _ := pathsFixture(t) + got, err := Paths(context.Background(), root, "**/*.go", PathOptions{Sort: SortLex, MaxResults: 2}) + if err != nil { + t.Fatal(err) + } + if got.Total != 4 { + t.Fatalf("total = %d, want 4", got.Total) + } + if len(got.Paths) != 2 { + t.Fatalf("paths len = %d, want 2", len(got.Paths)) + } + if !got.Truncated { + t.Fatal("truncated should be true") + } + }) + + t.Run("default skip dirs are skipped", func(t *testing.T) { + root, _ := pathsFixture(t) + got, err := Paths(context.Background(), root, "**/*", PathOptions{Sort: SortLex}) + if err != nil { + t.Fatal(err) + } + for _, p := range got.Paths { + if strings.HasPrefix(p, ".git/") || strings.HasPrefix(p, "node_modules/") { + t.Fatalf("skip dir leaked into results: %q", p) + } + } + }) + + t.Run("path scope under root", func(t *testing.T) { + root, _ := pathsFixture(t) + got, err := Paths(context.Background(), root, "**/*.go", PathOptions{Path: "pkg", Sort: SortLex}) + if err != nil { + t.Fatal(err) + } + for _, p := range got.Paths { + if !strings.HasPrefix(p, "pkg/") { + t.Errorf("path %q escaped pkg/ scope", p) + } + } + if len(got.Paths) == 0 { + t.Fatal("expected matches under pkg/") + } + }) + + t.Run("path outside root rejected", func(t *testing.T) { + root, _ := pathsFixture(t) + _, err := Paths(context.Background(), root, "**/*.go", PathOptions{Path: "../etc"}) + if err == nil { + t.Fatal("expected error for path escaping root") + } + }) + + t.Run("absolute path outside root rejected", func(t *testing.T) { + root, _ := pathsFixture(t) + _, err := Paths(context.Background(), root, "**/*.go", PathOptions{Path: "/tmp"}) + if err == nil { + t.Fatal("expected error for absolute path outside root") + } + }) + + t.Run("invalid sort returns error", func(t *testing.T) { + root, _ := pathsFixture(t) + _, err := Paths(context.Background(), root, "**/*.go", PathOptions{Sort: "weird"}) + if err == nil { + t.Fatal("expected error for invalid sort") + } + }) + + t.Run("empty pattern returns error", func(t *testing.T) { + root, _ := pathsFixture(t) + _, err := Paths(context.Background(), root, "", PathOptions{}) + if err == nil { + t.Fatal("expected error for empty pattern") + } + }) + + t.Run("invalid pattern returns error", func(t *testing.T) { + root, _ := pathsFixture(t) + _, err := Paths(context.Background(), root, "[unclosed", PathOptions{}) + if err == nil { + t.Fatal("expected error for invalid pattern") + } + }) +} From 91b65e5b4725b1fab699a65b6c152922600f1dfa Mon Sep 17 00:00:00 2001 From: Ludovico Russo Date: Thu, 30 Apr 2026 10:23:47 +0200 Subject: [PATCH 3/5] feat(tools): multi_edit tool (atomic batched string-replace) (#78) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third slice of PRD #76. Adds multi_edit so the agent can apply an ordered batch of `{old_string, new_string, replace_all?}` edits to a single file in one round-trip, instead of N sequential update_file hops. **multi_edit.** Inlined in pkg/agent/tools/files.go next to update_file (which is left unchanged). Validates the batch up-front (non-empty, each old != new), reads the file once, applies edits sequentially against the in-progress buffer so a later edit can target text produced by an earlier one, and writes the final buffer in a single fs write — atomic by construction. Per-edit replace_all overrides the default unique-old_string requirement. Errors identify the failing edit by index ("edit 1: ...") so the agent can fix one edit and retry without rebuilding the whole batch. **Wiring.** Code mode registers multi_edit via FileTools (now 5 tools). Chat mode registers a thin MultiEditTools(workDir) constructor that returns just the multi_edit tool — chat mode doesn't have read_file etc. so we expose only the edit primitive, not the full file surface. Subagents inherit it automatically through the existing default-inheritance filter. **Tests.** Cover the schema (path + edits required), sequential application, later-targets-earlier-output, per-edit replace_all, atomic-failure-leaves-file-unchanged with per-index error message, non-unique without replace_all, in-progress-buffer uniqueness (first edit creates a duplicate, second edit must fail), empty edits, identical old/new, and missing file. CODE_AGENT.md gains a multi_edit bullet under File operations and a workflow note preferring it over multiple update_file calls when editing the same file two-or-more times. --- pkg/agent/CODE_AGENT.md | 3 +- pkg/agent/agent.go | 1 + pkg/agent/tools/files.go | 76 +++++++++++ pkg/agent/tools/files_test.go | 236 +++++++++++++++++++++++++++++++++- 4 files changed, 312 insertions(+), 4 deletions(-) diff --git a/pkg/agent/CODE_AGENT.md b/pkg/agent/CODE_AGENT.md index 3b59348..0a3039c 100644 --- a/pkg/agent/CODE_AGENT.md +++ b/pkg/agent/CODE_AGENT.md @@ -10,6 +10,7 @@ You have dedicated file tools. **Always prefer them over bash for file I/O:** - **`read_file`** — Read file content with line numbers. Always read a file before modifying it. - **`write_file`** — Write full file content. Use for new files or complete rewrites. - **`update_file`** — Replace an exact string in a file. Preferred for targeted edits — the old_string must be unique in the file. Always `read_file` first. +- **`multi_edit`** — Apply an ordered, atomic batch of `{old_string, new_string, replace_all?}` edits to a single file in one call. Either every edit applies or nothing changes on disk. Edits run sequentially so a later edit can target text produced by an earlier one. Prefer this over multiple `update_file` calls when editing the same file two or more times. **Do NOT use bash for**: `cat`, `head`, `tail`, `ls`, `find`, `echo >`, or any file read/write operation. @@ -33,7 +34,7 @@ Use `bash` only for running commands: build, test, git, install, compile, lint 1. Use `list_files` to understand the project structure before making changes. 2. Use `read_file` to understand existing code before editing. -3. Use `update_file` for targeted edits, or `write_file` for new files / complete rewrites. +3. Use `update_file` for a single targeted edit, `multi_edit` for two-or-more edits to the same file, or `write_file` for new files / complete rewrites. 4. Use `bash` to build, test, or run commands to verify your changes. ## Inform User diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 048f674..1395ac4 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -58,6 +58,7 @@ func Prepare(ctx context.Context, cfg Config) (*session.Config, *debug.Logger, e registry.Add(tools.GeneralTools()) registry.Add(tools.ExecTools(cfg.Home.Dir())) registry.Add(tools.SearchTools(cfg.Home.Dir(), cfg.Tools.Search)) + registry.Add(tools.MultiEditTools(cfg.Home.Dir())) registry.Add(tools.SkillTools(skillsDir)) registry.Add(tools.InformTools()) registry.Add(tools.TelegramTools(cfg.TelegramAuth)) diff --git a/pkg/agent/tools/files.go b/pkg/agent/tools/files.go index a1e5c43..b8f81c6 100644 --- a/pkg/agent/tools/files.go +++ b/pkg/agent/tools/files.go @@ -18,9 +18,18 @@ func FileTools(workDir string) []tool.Tool { newReadFileTool(workDir), newWriteFileTool(workDir), newUpdateFileTool(workDir), + newMultiEditTool(workDir), } } +// MultiEditTools returns just the multi_edit tool bound to workDir. +// Chat mode registers this without the rest of FileTools so the agent can +// apply atomic batched edits during investigation without exposing the full +// read/write surface. +func MultiEditTools(workDir string) []tool.Tool { + return []tool.Tool{newMultiEditTool(workDir)} +} + // resolvePath resolves inputPath relative to workDir. func resolvePath(workDir, inputPath string) string { if filepath.IsAbs(inputPath) { @@ -228,3 +237,70 @@ func newUpdateFileTool(workDir string) tool.Tool { }, ) } + +// --- multi_edit --- + +type multiEditSpec struct { + OldString string `json:"old_string" description:"Exact string to find. Must be unique in the in-progress buffer unless replace_all is true."` + NewString string `json:"new_string" description:"Replacement string. Must differ from old_string."` + ReplaceAll bool `json:"replace_all,omitempty" description:"Replace every occurrence of old_string instead of requiring uniqueness."` +} + +type multiEditInput struct { + Path string `json:"path" description:"File path to modify"` + Edits []multiEditSpec `json:"edits" description:"Ordered list of edits applied sequentially against the in-progress buffer; either every edit applies or nothing is written."` +} + +type multiEditOutput struct { + Path string `json:"path"` + EditsApplied int `json:"edits_applied"` +} + +func newMultiEditTool(workDir string) tool.Tool { + return tool.NewTool("multi_edit", + "Apply an ordered, atomic batch of string replacements to a single file. "+ + "Each edit's old_string must be unique in the in-progress buffer unless that edit's replace_all is true. "+ + "Edits run sequentially so a later edit can target text produced by an earlier edit. "+ + "Either every edit applies and the file is written once, or nothing changes on disk. "+ + "Prefer this over multiple update_file calls when editing the same file two or more times.", + func(ctx context.Context, in multiEditInput) (multiEditOutput, error) { + if len(in.Edits) == 0 { + return multiEditOutput{}, fmt.Errorf("edits list is empty") + } + for i, e := range in.Edits { + if e.OldString == e.NewString { + return multiEditOutput{}, fmt.Errorf("edit %d: old_string and new_string are identical", i) + } + } + + p := resolvePath(workDir, in.Path) + data, err := os.ReadFile(p) + if err != nil { + return multiEditOutput{}, fmt.Errorf("reading %s: %w", p, err) + } + + content := string(data) + for i, e := range in.Edits { + count := strings.Count(content, e.OldString) + if count == 0 { + return multiEditOutput{}, fmt.Errorf("edit %d: old_string not found in %s", i, p) + } + if !e.ReplaceAll && count > 1 { + return multiEditOutput{}, fmt.Errorf("edit %d: old_string appears %d times in %s — must be unique (set replace_all to override)", i, count, p) + } + if e.ReplaceAll { + content = strings.ReplaceAll(content, e.OldString, e.NewString) + } else { + content = strings.Replace(content, e.OldString, e.NewString, 1) + } + } + + // #nosec G703 - path is resolved through resolvePath from user input + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + return multiEditOutput{}, fmt.Errorf("writing %s: %w", p, err) + } + + return multiEditOutput{Path: p, EditsApplied: len(in.Edits)}, nil + }, + ) +} diff --git a/pkg/agent/tools/files_test.go b/pkg/agent/tools/files_test.go index 45efbd3..37d73e0 100644 --- a/pkg/agent/tools/files_test.go +++ b/pkg/agent/tools/files_test.go @@ -1,10 +1,12 @@ package tools import ( + "bytes" "context" "encoding/json" "os" "path/filepath" + "strings" "testing" "github.com/ludusrusso/wildgecu/pkg/provider/tool" @@ -24,20 +26,30 @@ func execTool(t *testing.T, tl tool.Tool, args map[string]any, dst any) { func TestFileTools(t *testing.T) { tools := FileTools("/tmp") - if len(tools) != 4 { - t.Fatalf("expected 4 file tools, got %d", len(tools)) + if len(tools) != 5 { + t.Fatalf("expected 5 file tools, got %d", len(tools)) } names := map[string]bool{} for _, tl := range tools { names[tl.Definition().Name] = true } - for _, want := range []string{"list_files", "read_file", "write_file", "update_file"} { + for _, want := range []string{"list_files", "read_file", "write_file", "update_file", "multi_edit"} { if !names[want] { t.Errorf("missing tool %q", want) } } } +func TestMultiEditTools(t *testing.T) { + tools := MultiEditTools("/tmp") + if len(tools) != 1 { + t.Fatalf("expected 1 tool, got %d", len(tools)) + } + if got := tools[0].Definition().Name; got != "multi_edit" { + t.Errorf("expected multi_edit, got %q", got) + } +} + func TestListFiles(t *testing.T) { t.Run("default dir", func(t *testing.T) { dir := t.TempDir() @@ -263,6 +275,224 @@ func TestUpdateFile(t *testing.T) { }) } +func TestMultiEdit(t *testing.T) { + t.Run("schema marks path and edits required", func(t *testing.T) { + tl := newMultiEditTool("/tmp") + params := tl.Definition().Parameters + props, _ := params["properties"].(map[string]any) + if _, ok := props["path"]; !ok { + t.Error("expected path in properties") + } + if _, ok := props["edits"]; !ok { + t.Error("expected edits in properties") + } + required, _ := params["required"].([]any) + got := map[string]bool{} + for _, r := range required { + got[r.(string)] = true + } + if !got["path"] || !got["edits"] { + t.Errorf("expected required={path, edits}, got %v", required) + } + }) + + t.Run("applies sequential edits", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + os.WriteFile(path, []byte("hello world"), 0o644) + + tl := newMultiEditTool(dir) + var out multiEditOutput + execTool(t, tl, map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "hello", "new_string": "hi"}, + map[string]any{"old_string": "world", "new_string": "gopher"}, + }, + }, &out) + + if out.EditsApplied != 2 { + t.Errorf("edits_applied = %d, want 2", out.EditsApplied) + } + data, _ := os.ReadFile(path) + if string(data) != "hi gopher" { + t.Errorf("file content = %q, want %q", string(data), "hi gopher") + } + }) + + t.Run("later edit targets earlier edit's output", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + os.WriteFile(path, []byte("foo"), 0o644) + + tl := newMultiEditTool(dir) + var out multiEditOutput + execTool(t, tl, map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "foo", "new_string": "bar"}, + map[string]any{"old_string": "bar", "new_string": "baz"}, + }, + }, &out) + + data, _ := os.ReadFile(path) + if string(data) != "baz" { + t.Errorf("file content = %q, want %q", string(data), "baz") + } + }) + + t.Run("replace_all per edit", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + os.WriteFile(path, []byte("aa bb aa cc aa"), 0o644) + + tl := newMultiEditTool(dir) + var out multiEditOutput + execTool(t, tl, map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "aa", "new_string": "X", "replace_all": true}, + map[string]any{"old_string": "cc", "new_string": "Y"}, + }, + }, &out) + + data, _ := os.ReadFile(path) + if string(data) != "X bb X Y X" { + t.Errorf("file content = %q, want %q", string(data), "X bb X Y X") + } + }) + + t.Run("atomic failure leaves file unchanged", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + original := []byte("alpha beta gamma") + os.WriteFile(path, original, 0o644) + + tl := newMultiEditTool(dir) + _, err := tl.Execute(context.Background(), map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "alpha", "new_string": "A"}, + map[string]any{"old_string": "missing", "new_string": "Z"}, + }, + }) + if err == nil { + t.Fatal("expected error for missing old_string") + } + if !strings.Contains(err.Error(), "edit 1") { + t.Errorf("error should reference edit 1, got %v", err) + } + + data, _ := os.ReadFile(path) + if !bytes.Equal(data, original) { + t.Errorf("file should be unchanged, got %q", string(data)) + } + }) + + t.Run("non-unique old_string without replace_all fails", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + original := []byte("aa bb aa") + os.WriteFile(path, original, 0o644) + + tl := newMultiEditTool(dir) + _, err := tl.Execute(context.Background(), map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "aa", "new_string": "X"}, + }, + }) + if err == nil { + t.Fatal("expected error for duplicate old_string") + } + if !strings.Contains(err.Error(), "edit 0") { + t.Errorf("error should reference edit 0, got %v", err) + } + + data, _ := os.ReadFile(path) + if !bytes.Equal(data, original) { + t.Errorf("file should be unchanged, got %q", string(data)) + } + }) + + t.Run("uniqueness checked against in-progress buffer", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + // Initially "x" appears once. After first edit replacing "y" with + // "x", the buffer has two "x" — the second edit (without replace_all) + // must fail because uniqueness is computed against the current buffer. + os.WriteFile(path, []byte("x y"), 0o644) + + tl := newMultiEditTool(dir) + _, err := tl.Execute(context.Background(), map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "y", "new_string": "x"}, + map[string]any{"old_string": "x", "new_string": "Z"}, + }, + }) + if err == nil { + t.Fatal("expected error: second edit should see two occurrences of x") + } + + data, _ := os.ReadFile(path) + if string(data) != "x y" { + t.Errorf("file should be unchanged, got %q", string(data)) + } + }) + + t.Run("empty edits rejected", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + os.WriteFile(path, []byte("hello"), 0o644) + + tl := newMultiEditTool(dir) + _, err := tl.Execute(context.Background(), map[string]any{ + "path": "f.txt", + "edits": []any{}, + }) + if err == nil { + t.Fatal("expected error for empty edits") + } + }) + + t.Run("identical old_string and new_string rejected", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "f.txt") + original := []byte("hello") + os.WriteFile(path, original, 0o644) + + tl := newMultiEditTool(dir) + _, err := tl.Execute(context.Background(), map[string]any{ + "path": "f.txt", + "edits": []any{ + map[string]any{"old_string": "hello", "new_string": "hello"}, + }, + }) + if err == nil { + t.Fatal("expected error for identical old/new") + } + + data, _ := os.ReadFile(path) + if !bytes.Equal(data, original) { + t.Errorf("file should be unchanged, got %q", string(data)) + } + }) + + t.Run("missing file returns error", func(t *testing.T) { + tl := newMultiEditTool(t.TempDir()) + _, err := tl.Execute(context.Background(), map[string]any{ + "path": "nope.txt", + "edits": []any{ + map[string]any{"old_string": "x", "new_string": "y"}, + }, + }) + if err == nil { + t.Fatal("expected error for missing file") + } + }) +} + func TestResolvePath(t *testing.T) { t.Run("relative", func(t *testing.T) { got := resolvePath("/work", "sub/file.txt") From 0c7b7788dc9078511d147c1befe814ead4bc4b8a Mon Sep 17 00:00:00 2001 From: Ludovico Russo Date: Thu, 30 Apr 2026 10:32:32 +0200 Subject: [PATCH 4/5] feat(tools): bounded-exec helper + bash timeout & output truncation (#79) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth slice of PRD #76. Closes the bash-hardening half of the PRD: the agent can now give long-running commands an explicit timeout budget, and a runaway `find /` or verbose build can no longer dump 100MB into the LLM context. **pkg/exec/bounded.** New deep module wrapping os/exec. `Run(ctx, name, args, opts) (Result, error)` enforces a per-call timeout via context.WithTimeout, and captures stdout/stderr through head+tail buffers — a fixed-size head, a ring-buffered tail, and a total-bytes counter that keeps incrementing past the cap. When total exceeds head+tail the rendered output is line-aligned (head trimmed back to the last newline; tail advanced past the first) with a `[... truncated N bytes from middle ...]` marker between them, so the final lines of a build (which usually contain the actual error) survive. Result carries `Stdout`, `Stderr`, `ExitCode`, `TimedOut`, `StdoutTotalBytes`, `StderrTotalBytes`. Timeout > MaxTimeout returns a clear "exceeds maximum" error rather than being silently clamped; context-deadline kills surface as `TimedOut: true` + `ExitCode: -1`, distinct from a normal failure. **bash tool.** Wired through bounded.Run. New `timeout_seconds` arg (default 30, hard-cap 600 unless cfg overrides). Output JSON gains `timed_out`, `stdout_total_bytes`, `stderr_total_bytes`. Existing calls without the arg keep working unchanged. node tool intentionally left on the legacy exec path — that's slice #81's job — but its constructor signature widens to take ExecConfig so the conversion is a one-file change. **Config plumbing.** YAML gains a `tools.bash` block with `max_timeout_seconds`, `head_bytes`, `tail_bytes`. All optional; zero-valued defaults fall through to the constants in pkg/exec/bounded. Threaded daemon.Config → agent.Config → tools.ExecConfig. **Tests.** pkg/exec/bounded covers small output (no marker), large output (cap + marker + total bytes), timeout (TimedOut + fast return), nonzero exit code propagation, head+tail line alignment around the marker, exact-boundary no-truncation, and independent stderr capping. Also unit-tests the headTailBuf directly. Tool-layer tests cover default-timeout behavior, `timeout_seconds` firing, hard-cap rejection, and large-stdout truncation. TestBash now skips cleanly when bash is absent, matching the existing TestNode pattern. CODE_AGENT.md gains a note on `timeout_seconds`, the truncation marker, and the `stdout_total_bytes` field. --- cmd/start.go | 5 + pkg/agent/CODE_AGENT.md | 4 + pkg/agent/agent.go | 4 +- pkg/agent/tools/config.go | 2 + pkg/agent/tools/exec.go | 91 ++++++---- pkg/agent/tools/exec_test.go | 85 ++++++++- pkg/exec/bounded/bounded.go | 215 ++++++++++++++++++++++ pkg/exec/bounded/bounded_test.go | 296 +++++++++++++++++++++++++++++++ x/config/load.go | 9 + x/config/load_test.go | 13 ++ 10 files changed, 687 insertions(+), 37 deletions(-) create mode 100644 pkg/exec/bounded/bounded.go create mode 100644 pkg/exec/bounded/bounded_test.go diff --git a/cmd/start.go b/cmd/start.go index 450a5a8..455b3e8 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -22,6 +22,11 @@ func buildToolsConfig(in config.ToolsConfig) tools.Config { MaxResults: in.Grep.MaxResults, MaxFileSizeBytes: in.Grep.MaxFileSizeBytes, }, + Exec: tools.ExecConfig{ + MaxTimeoutSeconds: in.Bash.MaxTimeoutSeconds, + HeadBytes: in.Bash.HeadBytes, + TailBytes: in.Bash.TailBytes, + }, } } diff --git a/pkg/agent/CODE_AGENT.md b/pkg/agent/CODE_AGENT.md index 0a3039c..81dfff0 100644 --- a/pkg/agent/CODE_AGENT.md +++ b/pkg/agent/CODE_AGENT.md @@ -30,6 +30,10 @@ Both tools skip `.git`, `node_modules`, `vendor`, `dist`, `build`, `target`, and Use `bash` only for running commands: build, test, git, install, compile, lint — anything that is not file I/O. Bash runs in the working directory `{CWD}`. +Pass `timeout_seconds` to give long-running commands more time. Default is 30s; the hard cap is 600s (10 minutes). Asking for more than the cap returns an error rather than being silently clamped. When a command exceeds its budget the result has `timed_out: true` and `exit_code: -1`. + +Stdout and stderr are each captured through a head+tail window. When output overflows, the result includes a `[... truncated N bytes from middle ...]` marker between the head and the tail (so the final lines of a build — which usually contain the actual error — survive). The full byte counts are reported in `stdout_total_bytes` and `stderr_total_bytes`. + ## Workflow 1. Use `list_files` to understand the project structure before making changes. diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 1395ac4..ae01f8e 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -56,7 +56,7 @@ func Prepare(ctx context.Context, cfg Config) (*session.Config, *debug.Logger, e skillsDir := cfg.Home.SkillsDir() registry := tool.NewRegistry() registry.Add(tools.GeneralTools()) - registry.Add(tools.ExecTools(cfg.Home.Dir())) + registry.Add(tools.ExecTools(cfg.Home.Dir(), cfg.Tools.Exec)) registry.Add(tools.SearchTools(cfg.Home.Dir(), cfg.Tools.Search)) registry.Add(tools.MultiEditTools(cfg.Home.Dir())) registry.Add(tools.SkillTools(skillsDir)) @@ -142,7 +142,7 @@ func PrepareCode(ctx context.Context, cfg Config, workDir string) (*session.Conf skillsDir := cfg.Home.SkillsDir() registry := tool.NewRegistry() registry.Add(tools.GeneralTools()) - registry.Add(tools.ExecTools(workDir)) + registry.Add(tools.ExecTools(workDir, cfg.Tools.Exec)) registry.Add(tools.FileTools(workDir)) registry.Add(tools.SearchTools(workDir, cfg.Tools.Search)) registry.Add(tools.SkillTools(skillsDir)) diff --git a/pkg/agent/tools/config.go b/pkg/agent/tools/config.go index 0332c45..ced3c17 100644 --- a/pkg/agent/tools/config.go +++ b/pkg/agent/tools/config.go @@ -4,4 +4,6 @@ package tools type Config struct { // Search configures grep (and future search tools). Search SearchConfig + // Exec configures bash (and node, once #81 lands). + Exec ExecConfig } diff --git a/pkg/agent/tools/exec.go b/pkg/agent/tools/exec.go index a4a6f96..0d78388 100644 --- a/pkg/agent/tools/exec.go +++ b/pkg/agent/tools/exec.go @@ -7,61 +7,88 @@ import ( "os/exec" "time" + "github.com/ludusrusso/wildgecu/pkg/exec/bounded" "github.com/ludusrusso/wildgecu/pkg/provider/tool" ) +// ExecConfig configures the execution tools (bash, node). Zero values fall +// back to the bounded package's defaults. +type ExecConfig struct { + // MaxTimeoutSeconds is the hard ceiling on the per-call + // timeout_seconds argument. Zero = use bounded.DefaultMaxTimeout. + MaxTimeoutSeconds int + // HeadBytes is the size of the captured head window for + // stdout/stderr. Zero = use bounded.DefaultHeadBytes. + HeadBytes int + // TailBytes is the size of the captured tail window for + // stdout/stderr. Zero = use bounded.DefaultTailBytes. + TailBytes int +} + // ExecTools returns execution tools (bash, node) bound to workDir. -func ExecTools(workDir string) []tool.Tool { - return []tool.Tool{newBashTool(workDir), newNodeTool(workDir)} +func ExecTools(workDir string, cfg ExecConfig) []tool.Tool { + return []tool.Tool{newBashTool(workDir, cfg), newNodeTool(workDir, cfg)} +} + +// boundedOpts builds bounded.Opts from cfg and a per-call timeout_seconds. A +// zero or negative timeoutSeconds picks the bounded.DefaultTimeout. +func (cfg ExecConfig) boundedOpts(workDir string, timeoutSeconds int) bounded.Opts { + opts := bounded.Opts{ + HeadBytes: cfg.HeadBytes, + TailBytes: cfg.TailBytes, + WorkDir: workDir, + } + if cfg.MaxTimeoutSeconds > 0 { + opts.MaxTimeout = time.Duration(cfg.MaxTimeoutSeconds) * time.Second + } + if timeoutSeconds > 0 { + opts.Timeout = time.Duration(timeoutSeconds) * time.Second + } + return opts } // --- bash --- type bashInput struct { - Command string `json:"command" description:"The bash command to execute"` + Command string `json:"command" description:"The bash command to execute"` + TimeoutSeconds int `json:"timeout_seconds,omitempty" description:"Per-call timeout in seconds. Default 30, hard-capped at 600."` } type bashOutput struct { - Stdout string `json:"stdout"` - Stderr string `json:"stderr"` - ExitCode int `json:"exit_code"` + Stdout string `json:"stdout"` + Stderr string `json:"stderr"` + ExitCode int `json:"exit_code"` + TimedOut bool `json:"timed_out,omitempty"` + StdoutTotalBytes int `json:"stdout_total_bytes"` + StderrTotalBytes int `json:"stderr_total_bytes"` } -func newBashTool(workDir string) tool.Tool { - return tool.NewTool("bash", "Execute a bash command and return its output", +func newBashTool(workDir string, cfg ExecConfig) tool.Tool { + return tool.NewTool("bash", + "Execute a bash command and return its output. Pass timeout_seconds (default 30, hard-capped at 600) to give long-running commands more time.", func(ctx context.Context, in bashInput) (bashOutput, error) { - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - cmd := exec.CommandContext(ctx, "bash", "-c", in.Command) - cmd.Dir = workDir - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - err := cmd.Run() - - exitCode := 0 + res, err := bounded.Run(ctx, "bash", []string{"-c", in.Command}, cfg.boundedOpts(workDir, in.TimeoutSeconds)) if err != nil { - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - exitCode = exitErr.ExitCode() - } else { - return bashOutput{}, err - } + return bashOutput{}, err } - return bashOutput{ - Stdout: stdout.String(), - Stderr: stderr.String(), - ExitCode: exitCode, + Stdout: res.Stdout, + Stderr: res.Stderr, + ExitCode: res.ExitCode, + TimedOut: res.TimedOut, + StdoutTotalBytes: res.StdoutTotalBytes, + StderrTotalBytes: res.StderrTotalBytes, }, nil }, ) } // --- node --- +// +// Note: node still uses the legacy exec path. Slice 5 (#81) converts it to +// bounded.Run for parity with bash. Until then the cfg is ignored here so the +// existing behavior — 30s hardcoded timeout, no output truncation — is +// preserved. type nodeInput struct { Script string `json:"script" description:"The Node.js script to execute"` @@ -73,7 +100,7 @@ type nodeOutput struct { ExitCode int `json:"exit_code"` } -func newNodeTool(workDir string) tool.Tool { +func newNodeTool(workDir string, _ ExecConfig) tool.Tool { return tool.NewTool("node", "Execute a Node.js script and return its output", func(ctx context.Context, in nodeInput) (nodeOutput, error) { ctx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/pkg/agent/tools/exec_test.go b/pkg/agent/tools/exec_test.go index 898ad46..842b3ea 100644 --- a/pkg/agent/tools/exec_test.go +++ b/pkg/agent/tools/exec_test.go @@ -4,11 +4,13 @@ import ( "context" "encoding/json" "os/exec" + "strings" "testing" + "time" ) func TestExecTools(t *testing.T) { - tools := ExecTools("/tmp") + tools := ExecTools("/tmp", ExecConfig{}) if len(tools) != 2 { t.Fatalf("expected 2 exec tools, got %d", len(tools)) } @@ -24,8 +26,12 @@ func TestExecTools(t *testing.T) { } func TestBash(t *testing.T) { + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not available") + } + dir := t.TempDir() - tl := newBashTool(dir) + tl := newBashTool(dir, ExecConfig{}) t.Run("echo stdout", func(t *testing.T) { var out bashOutput @@ -103,6 +109,79 @@ func TestBash(t *testing.T) { t.Fatalf("exit_code = %d", out.ExitCode) } }) + + t.Run("default timeout still applies when timeout_seconds omitted", func(t *testing.T) { + // Sanity check: a quick command still works without a + // timeout_seconds arg. + var out bashOutput + result, err := tl.Execute(context.Background(), map[string]any{"command": "echo ok"}) + if err != nil { + t.Fatal(err) + } + json.Unmarshal([]byte(result), &out) + if out.Stdout != "ok\n" { + t.Fatalf("stdout = %q", out.Stdout) + } + }) + + t.Run("timeout_seconds arg fires", func(t *testing.T) { + var out bashOutput + start := time.Now() + result, err := tl.Execute(context.Background(), map[string]any{ + "command": "sleep 5", + "timeout_seconds": float64(1), + }) + elapsed := time.Since(start) + if err != nil { + t.Fatal(err) + } + json.Unmarshal([]byte(result), &out) + if !out.TimedOut { + t.Fatalf("expected timed_out=true, got %+v", out) + } + if out.ExitCode != -1 { + t.Fatalf("exit_code = %d, want -1", out.ExitCode) + } + if elapsed > 4*time.Second { + t.Fatalf("expected fast return, elapsed = %s", elapsed) + } + }) + + t.Run("timeout_seconds above hard cap returns boundary error", func(t *testing.T) { + capped := newBashTool(dir, ExecConfig{MaxTimeoutSeconds: 5}) + _, err := capped.Execute(context.Background(), map[string]any{ + "command": "true", + "timeout_seconds": float64(9999), + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "exceeds maximum") { + t.Fatalf("error = %q, want it to mention 'exceeds maximum'", err.Error()) + } + }) + + t.Run("large stdout is truncated with marker and total bytes", func(t *testing.T) { + small := newBashTool(dir, ExecConfig{HeadBytes: 100, TailBytes: 100}) + var out bashOutput + result, err := small.Execute(context.Background(), map[string]any{ + "command": `for i in $(seq 1 200); do printf 'line%03d-AAAAAAAAAAAA\n' $i; done`, + }) + if err != nil { + t.Fatal(err) + } + json.Unmarshal([]byte(result), &out) + if !strings.Contains(out.Stdout, "[... truncated ") { + t.Fatalf("expected truncation marker in stdout: %q", out.Stdout) + } + if out.StdoutTotalBytes <= 200 { + t.Fatalf("StdoutTotalBytes = %d, want > 200", out.StdoutTotalBytes) + } + // Last line must survive in tail. + if !strings.Contains(out.Stdout, "line200-") { + t.Fatalf("tail lost last line: %q", out.Stdout) + } + }) } func TestNode(t *testing.T) { @@ -111,7 +190,7 @@ func TestNode(t *testing.T) { } dir := t.TempDir() - tl := newNodeTool(dir) + tl := newNodeTool(dir, ExecConfig{}) t.Run("console.log", func(t *testing.T) { var out nodeOutput diff --git a/pkg/exec/bounded/bounded.go b/pkg/exec/bounded/bounded.go new file mode 100644 index 0000000..d319154 --- /dev/null +++ b/pkg/exec/bounded/bounded.go @@ -0,0 +1,215 @@ +// Package bounded wraps os/exec with two responsibilities: enforce a per-call +// timeout (returning a typed TimedOut bool rather than just an error), and +// capture stdout/stderr through line-aligned head+tail buffers with +// total-byte counters. +// +// The package is dependency-free of the tool/agent layers so it can be tested +// directly with synthetic processes. +package bounded + +import ( + "bytes" + "context" + "errors" + "fmt" + "os/exec" + "time" +) + +// Defaults are applied when the corresponding Opts field is zero. +const ( + DefaultTimeout = 30 * time.Second + DefaultMaxTimeout = 10 * time.Minute + DefaultHeadBytes = 24576 + DefaultTailBytes = 6144 +) + +// truncationMarkerFormat is the marker injected between the head and tail +// when output is truncated. The %d is the total number of bytes dropped from +// the middle (including bytes trimmed off head/tail to reach a newline). +const truncationMarkerFormat = "[... truncated %d bytes from middle ...]\n" + +// Opts configures a Run call. Zero values pick defaults from this package's +// Default* constants. +type Opts struct { + // Timeout is the per-call timeout. Defaults to DefaultTimeout. + Timeout time.Duration + // MaxTimeout is the hard ceiling enforced at the call boundary. A + // Timeout greater than MaxTimeout returns an error rather than being + // silently clamped. Defaults to DefaultMaxTimeout. + MaxTimeout time.Duration + // HeadBytes is the size of the captured head window for stdout/stderr. + HeadBytes int + // TailBytes is the size of the captured tail window for stdout/stderr. + TailBytes int + // WorkDir, if non-empty, is the working directory for the child + // process. + WorkDir string +} + +// Result carries the captured output and exit status of a Run call. +type Result struct { + Stdout string + Stderr string + ExitCode int + TimedOut bool + StdoutTotalBytes int + StderrTotalBytes int +} + +// Run executes name with args, enforcing the configured timeout and capturing +// stdout/stderr through head+tail buffers. The returned error is non-nil only +// for unexpected failures (e.g. command not found). A non-zero exit code, a +// timeout, or large output do NOT produce an error: they surface through +// Result fields so the caller can react distinctly. +func Run(ctx context.Context, name string, args []string, opts Opts) (Result, error) { + maxT := opts.MaxTimeout + if maxT <= 0 { + maxT = DefaultMaxTimeout + } + timeout := opts.Timeout + if timeout <= 0 { + timeout = DefaultTimeout + } + if timeout > maxT { + return Result{}, fmt.Errorf("timeout %s exceeds maximum %s", timeout, maxT) + } + + headBytes := opts.HeadBytes + if headBytes <= 0 { + headBytes = DefaultHeadBytes + } + tailBytes := opts.TailBytes + if tailBytes <= 0 { + tailBytes = DefaultTailBytes + } + + cmdCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + cmd := exec.CommandContext(cmdCtx, name, args...) + if opts.WorkDir != "" { + cmd.Dir = opts.WorkDir + } + + stdout := newHeadTailBuf(headBytes, tailBytes) + stderr := newHeadTailBuf(headBytes, tailBytes) + cmd.Stdout = stdout + cmd.Stderr = stderr + + runErr := cmd.Run() + timedOut := errors.Is(cmdCtx.Err(), context.DeadlineExceeded) + + res := Result{ + Stdout: stdout.String(), + Stderr: stderr.String(), + StdoutTotalBytes: stdout.Total(), + StderrTotalBytes: stderr.Total(), + TimedOut: timedOut, + } + + switch { + case timedOut: + res.ExitCode = -1 + return res, nil + case runErr != nil: + var exitErr *exec.ExitError + if errors.As(runErr, &exitErr) { + res.ExitCode = exitErr.ExitCode() + return res, nil + } + return res, runErr + default: + res.ExitCode = 0 + return res, nil + } +} + +// headTailBuf is an io.Writer that retains the first HeadBytes and the last +// TailBytes of all bytes written to it. When the total written exceeds +// head+tail, String() returns head, a truncation marker, then tail — +// line-aligned (head trimmed back to the last newline; tail advanced past +// the first newline). +type headTailBuf struct { + head []byte + headCap int + tail []byte // ring buffer of size tailCap + tailCap int + tailStart int // index in tail of the oldest retained byte + tailLen int // current count of bytes retained in tail (≤ tailCap) + total int +} + +func newHeadTailBuf(headCap, tailCap int) *headTailBuf { + return &headTailBuf{ + headCap: headCap, + tailCap: tailCap, + head: make([]byte, 0, headCap), + tail: make([]byte, tailCap), + } +} + +func (b *headTailBuf) Write(p []byte) (int, error) { + n := len(p) + b.total += n + if len(b.head) < b.headCap { + need := b.headCap - len(b.head) + if need >= len(p) { + b.head = append(b.head, p...) + return n, nil + } + b.head = append(b.head, p[:need]...) + p = p[need:] + } + for _, c := range p { + if b.tailLen < b.tailCap { + b.tail[(b.tailStart+b.tailLen)%b.tailCap] = c + b.tailLen++ + continue + } + b.tail[b.tailStart] = c + b.tailStart = (b.tailStart + 1) % b.tailCap + } + return n, nil +} + +// Total returns the total number of bytes written, including any that were +// dropped from the middle when the buffer overflowed. +func (b *headTailBuf) Total() int { return b.total } + +// String renders the captured head+tail. When no truncation occurred, it +// returns simply head followed by tail (concatenated). When truncation +// occurred (total > head+tail), the result is line-aligned and includes a +// "[... truncated N bytes from middle ...]" marker. +func (b *headTailBuf) String() string { + if b.tailLen == 0 { + return string(b.head) + } + + tail := b.linearTail() + bytesDropped := b.total - len(b.head) - b.tailLen + headFull := len(b.head) == b.headCap + if !headFull || bytesDropped <= 0 { + return string(b.head) + string(tail) + } + + headPart := b.head + if i := bytes.LastIndexByte(headPart, '\n'); i >= 0 { + headPart = headPart[:i+1] + } + tailPart := tail + if i := bytes.IndexByte(tailPart, '\n'); i >= 0 { + tailPart = tailPart[i+1:] + } + totalDropped := bytesDropped + (len(b.head) - len(headPart)) + (len(tail) - len(tailPart)) + marker := fmt.Sprintf(truncationMarkerFormat, totalDropped) + return string(headPart) + marker + string(tailPart) +} + +func (b *headTailBuf) linearTail() []byte { + out := make([]byte, b.tailLen) + for i := 0; i < b.tailLen; i++ { + out[i] = b.tail[(b.tailStart+i)%b.tailCap] + } + return out +} diff --git a/pkg/exec/bounded/bounded_test.go b/pkg/exec/bounded/bounded_test.go new file mode 100644 index 0000000..3cfc69c --- /dev/null +++ b/pkg/exec/bounded/bounded_test.go @@ -0,0 +1,296 @@ +package bounded + +import ( + "context" + "os/exec" + "strings" + "testing" + "time" +) + +func skipIfNoBash(t *testing.T) { + t.Helper() + if _, err := exec.LookPath("bash"); err != nil { + t.Skip("bash not available") + } +} + +func TestRun(t *testing.T) { + skipIfNoBash(t) + + t.Run("small output passes through unchanged", func(t *testing.T) { + res, err := Run(context.Background(), "bash", []string{"-c", "echo hello"}, Opts{}) + if err != nil { + t.Fatal(err) + } + if res.Stdout != "hello\n" { + t.Fatalf("stdout = %q, want %q", res.Stdout, "hello\n") + } + if res.ExitCode != 0 { + t.Fatalf("exit_code = %d, want 0", res.ExitCode) + } + if res.TimedOut { + t.Fatal("expected TimedOut=false") + } + if res.StdoutTotalBytes != len("hello\n") { + t.Fatalf("StdoutTotalBytes = %d, want %d", res.StdoutTotalBytes, len("hello\n")) + } + }) + + t.Run("nonzero exit code propagates without error", func(t *testing.T) { + res, err := Run(context.Background(), "bash", []string{"-c", "exit 42"}, Opts{}) + if err != nil { + t.Fatal(err) + } + if res.ExitCode != 42 { + t.Fatalf("exit_code = %d, want 42", res.ExitCode) + } + if res.TimedOut { + t.Fatal("expected TimedOut=false") + } + }) + + t.Run("stdout and stderr are captured independently", func(t *testing.T) { + res, err := Run(context.Background(), "bash", + []string{"-c", "echo out && echo err >&2"}, Opts{}) + if err != nil { + t.Fatal(err) + } + if res.Stdout != "out\n" { + t.Fatalf("stdout = %q", res.Stdout) + } + if res.Stderr != "err\n" { + t.Fatalf("stderr = %q", res.Stderr) + } + }) + + t.Run("workdir is honored", func(t *testing.T) { + dir := t.TempDir() + res, err := Run(context.Background(), "bash", []string{"-c", "pwd"}, Opts{WorkDir: dir}) + if err != nil { + t.Fatal(err) + } + // macOS may resolve /tmp through /private/tmp; just assert non-empty. + if strings.TrimSpace(res.Stdout) == "" { + t.Fatal("expected non-empty pwd output") + } + }) + + t.Run("timeout fires with TimedOut=true and exit_code=-1", func(t *testing.T) { + start := time.Now() + res, err := Run(context.Background(), "bash", []string{"-c", "sleep 5"}, Opts{ + Timeout: 200 * time.Millisecond, + }) + elapsed := time.Since(start) + if err != nil { + t.Fatal(err) + } + if !res.TimedOut { + t.Fatal("expected TimedOut=true") + } + if res.ExitCode != -1 { + t.Fatalf("ExitCode = %d, want -1", res.ExitCode) + } + if elapsed > 3*time.Second { + t.Fatalf("expected fast return, elapsed = %s", elapsed) + } + }) + + t.Run("timeout above max returns boundary error", func(t *testing.T) { + _, err := Run(context.Background(), "bash", []string{"-c", "true"}, Opts{ + Timeout: 20 * time.Minute, + MaxTimeout: 10 * time.Minute, + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "exceeds maximum") { + t.Fatalf("error = %q, want it to mention 'exceeds maximum'", err.Error()) + } + }) + + t.Run("large output is capped with marker and total bytes", func(t *testing.T) { + // Generate a known number of newline-separated lines so we can + // verify line alignment of the head+tail window. + head := 100 + tail := 100 + // 50 lines of "AAAA...\n" each ~30 bytes → ~1500 bytes total. + script := `for i in $(seq 1 50); do printf 'line%02d-AAAAAAAAAAAAAAAAAAA\n' $i; done` + res, err := Run(context.Background(), "bash", []string{"-c", script}, Opts{ + HeadBytes: head, + TailBytes: tail, + }) + if err != nil { + t.Fatal(err) + } + if res.ExitCode != 0 { + t.Fatalf("exit_code = %d", res.ExitCode) + } + if !strings.Contains(res.Stdout, "[... truncated ") { + t.Fatalf("stdout = %q, want truncation marker", res.Stdout) + } + if res.StdoutTotalBytes < head+tail { + t.Fatalf("StdoutTotalBytes = %d, want >= %d", res.StdoutTotalBytes, head+tail) + } + // Last line of the original output should survive in the tail. + if !strings.Contains(res.Stdout, "line50-") { + t.Fatalf("stdout = %q, expected last line to survive", res.Stdout) + } + // Head should start at line01 (no truncation before head). + if !strings.HasPrefix(res.Stdout, "line01-") { + t.Fatalf("stdout = %q, expected head to start at line01", res.Stdout) + } + }) + + t.Run("head+tail capture is line-aligned around the marker", func(t *testing.T) { + // Force a marker by exceeding head+tail. Lines are exactly 10 + // bytes each ("ABCDEFGHI\n") so we can reason about alignment. + head := 25 // mid-line of line 3 (bytes 21-25 are "ABCDE" of line 3) + tail := 25 // mid-line of last line; round-up moves to next newline + // 100 lines: 1000 bytes total → forces truncation. + script := `for i in $(seq 1 100); do printf 'ABCDEFGHI\n'; done` + res, err := Run(context.Background(), "bash", []string{"-c", script}, Opts{ + HeadBytes: head, + TailBytes: tail, + }) + if err != nil { + t.Fatal(err) + } + // Find the marker. + idx := strings.Index(res.Stdout, "[... truncated") + if idx < 0 { + t.Fatalf("missing marker in output: %q", res.Stdout) + } + // Head portion ends at the byte immediately before the marker. + // Line alignment: that byte must be a newline. + if idx == 0 { + t.Fatal("marker landed at start; expected at least one full line of head") + } + if res.Stdout[idx-1] != '\n' { + t.Fatalf("byte before marker = %q, want newline", res.Stdout[idx-1]) + } + // Tail portion begins after the marker line. Marker is followed + // by '\n' itself (per format), and the next portion must begin + // with a fresh line (or be empty). + afterMarker := res.Stdout[idx:] + nl := strings.IndexByte(afterMarker, '\n') + if nl < 0 { + t.Fatalf("marker has no trailing newline: %q", afterMarker) + } + tailPortion := afterMarker[nl+1:] + // tailPortion (if non-empty) should be made of full "ABCDEFGHI\n" + // lines — i.e. start with a complete line. + if tailPortion != "" && !strings.HasPrefix(tailPortion, "ABCDEFGHI\n") { + t.Fatalf("tail portion not line-aligned: %q", tailPortion) + } + }) + + t.Run("output exactly head+tail is not truncated", func(t *testing.T) { + // Produce a deterministic stream just barely fitting head+tail. + head := 50 + tail := 50 + // 10 lines of "abcdefghi\n" (10 bytes) = 100 bytes = head+tail + script := `for i in $(seq 1 10); do printf 'abcdefghi\n'; done` + res, err := Run(context.Background(), "bash", []string{"-c", script}, Opts{ + HeadBytes: head, + TailBytes: tail, + }) + if err != nil { + t.Fatal(err) + } + if strings.Contains(res.Stdout, "[... truncated ") { + t.Fatalf("unexpected marker in stdout: %q", res.Stdout) + } + if res.StdoutTotalBytes != 100 { + t.Fatalf("StdoutTotalBytes = %d, want 100", res.StdoutTotalBytes) + } + if res.Stdout != strings.Repeat("abcdefghi\n", 10) { + t.Fatalf("stdout mismatch: %q", res.Stdout) + } + }) + + t.Run("stderr truncates independently of stdout", func(t *testing.T) { + head := 50 + tail := 50 + // Big stderr, tiny stdout. + script := `echo small; for i in $(seq 1 100); do printf 'errline%02d\n' $i >&2; done` + res, err := Run(context.Background(), "bash", []string{"-c", script}, Opts{ + HeadBytes: head, + TailBytes: tail, + }) + if err != nil { + t.Fatal(err) + } + if res.Stdout != "small\n" { + t.Fatalf("stdout = %q", res.Stdout) + } + if !strings.Contains(res.Stderr, "[... truncated ") { + t.Fatalf("stderr should be truncated: %q", res.Stderr) + } + if !strings.Contains(res.Stderr, "errline100") { + t.Fatalf("stderr lost the last line: %q", res.Stderr) + } + }) +} + +func TestHeadTailBuf(t *testing.T) { + t.Run("under capacity returns full buffer", func(t *testing.T) { + b := newHeadTailBuf(100, 100) + _, _ = b.Write([]byte("hello")) + if b.String() != "hello" { + t.Fatalf("got %q", b.String()) + } + if b.Total() != 5 { + t.Fatalf("total = %d", b.Total()) + } + }) + + t.Run("exact head+tail boundary returns concatenated", func(t *testing.T) { + b := newHeadTailBuf(5, 5) + _, _ = b.Write([]byte("aaaaa")) // fills head + _, _ = b.Write([]byte("bbbbb")) // fills tail + if b.String() != "aaaaabbbbb" { + t.Fatalf("got %q", b.String()) + } + if b.Total() != 10 { + t.Fatalf("total = %d", b.Total()) + } + }) + + t.Run("overflow drops middle bytes and reports them", func(t *testing.T) { + // head=10 captures "AAAAA\nBBBB"; tail=10 captures the last 10 + // bytes "CCC\nDDDDD\n". Line-alignment trims head down to + // "AAAAA\n" and advances tail past its first newline to + // "DDDDD\n", with the marker between them. + b := newHeadTailBuf(10, 10) + _, _ = b.Write([]byte("AAAAA\nBBBBB\nCCCCC\nDDDDD\n")) + out := b.String() + if !strings.Contains(out, "[... truncated ") { + t.Fatalf("missing marker: %q", out) + } + if !strings.HasPrefix(out, "AAAAA\n") { + t.Fatalf("head not aligned to newline: %q", out) + } + if !strings.HasSuffix(out, "DDDDD\n") { + t.Fatalf("tail not aligned to newline: %q", out) + } + if b.Total() != 24 { + t.Fatalf("total = %d, want 24", b.Total()) + } + }) + + t.Run("multi-write across head boundary", func(t *testing.T) { + b := newHeadTailBuf(5, 100) + _, _ = b.Write([]byte("aa")) + _, _ = b.Write([]byte("bbcc")) + _, _ = b.Write([]byte("dd")) + // total 8 bytes; head takes first 5 ("aabbc"), tail keeps "cdd" + if b.Total() != 8 { + t.Fatalf("total = %d", b.Total()) + } + // no truncation since 8 ≤ 5+100 + if b.String() != "aabbccdd" { + t.Fatalf("got %q", b.String()) + } + }) +} diff --git a/x/config/load.go b/x/config/load.go index c09948c..73a84c0 100644 --- a/x/config/load.go +++ b/x/config/load.go @@ -26,9 +26,18 @@ type GrepConfig struct { MaxFileSizeBytes int64 `yaml:"max_file_size_bytes"` } +// BashConfig configures the bash tool. Zero values fall through to the +// defaults baked into pkg/exec/bounded. +type BashConfig struct { + MaxTimeoutSeconds int `yaml:"max_timeout_seconds"` + HeadBytes int `yaml:"head_bytes"` + TailBytes int `yaml:"tail_bytes"` +} + // ToolsConfig groups per-tool configuration loaded from the YAML "tools" block. type ToolsConfig struct { Grep GrepConfig `yaml:"grep"` + Bash BashConfig `yaml:"bash"` } // Config is the top-level application configuration. diff --git a/x/config/load_test.go b/x/config/load_test.go index 0fa0ddc..27a5fbe 100644 --- a/x/config/load_test.go +++ b/x/config/load_test.go @@ -668,6 +668,10 @@ tools: grep: max_results: 50 max_file_size_bytes: 524288 + bash: + max_timeout_seconds: 300 + head_bytes: 16384 + tail_bytes: 4096 `) if err := os.WriteFile(cfgPath, data, 0o644); err != nil { t.Fatal(err) @@ -683,6 +687,15 @@ tools: if cfg.Tools.Grep.MaxFileSizeBytes != 524288 { t.Errorf("Tools.Grep.MaxFileSizeBytes = %d, want 524288", cfg.Tools.Grep.MaxFileSizeBytes) } + if cfg.Tools.Bash.MaxTimeoutSeconds != 300 { + t.Errorf("Tools.Bash.MaxTimeoutSeconds = %d, want 300", cfg.Tools.Bash.MaxTimeoutSeconds) + } + if cfg.Tools.Bash.HeadBytes != 16384 { + t.Errorf("Tools.Bash.HeadBytes = %d, want 16384", cfg.Tools.Bash.HeadBytes) + } + if cfg.Tools.Bash.TailBytes != 4096 { + t.Errorf("Tools.Bash.TailBytes = %d, want 4096", cfg.Tools.Bash.TailBytes) + } }) t.Run("ToolsBlockOptional", func(t *testing.T) { From 00ebf5839e0342e964c3d71d0796b39ca56df7f4 Mon Sep 17 00:00:00 2001 From: Ludovico Russo Date: Thu, 30 Apr 2026 10:35:15 +0200 Subject: [PATCH 5/5] feat(tools): apply bounded-exec to node tool (#81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifth and final slice of PRD #76. Brings node to bash parity: same timeout_seconds arg (default 30, hard-cap 600), same head+tail output truncation with line-aligned [... truncated N bytes from middle ...] marker, same {timed_out, stdout_total_bytes, stderr_total_bytes} fields in the result. node piggybacks on the existing tools.bash.* config knobs rather than introducing a parallel block — they're the same execution surface, two thin command wrappers over bounded.Run. The legacy exec path (manual context.WithTimeout + bytes.Buffer + exec.ExitError unwrap) is gone; newNodeTool is now the same shape as newBashTool. Existing node calls without timeout_seconds keep working unchanged. CODE_AGENT.md's "Bash" section becomes "Bash and node", since the timeout/truncation contract is identical. Tests mirror bash: default-timeout sanity, timeout_seconds firing (setTimeout 5s killed at 1s, fast return + TimedOut true + exit -1), hard-cap rejection, and large-stdout truncation (200 lines through HeadBytes=100/TailBytes=100, marker present, last line survives in tail). All under t.Run subtests; the existing TestNode skip-when-node- absent guard covers the new cases too. --- pkg/agent/CODE_AGENT.md | 6 +-- pkg/agent/tools/exec.go | 56 ++++++++++------------------ pkg/agent/tools/exec_test.go | 72 ++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 40 deletions(-) diff --git a/pkg/agent/CODE_AGENT.md b/pkg/agent/CODE_AGENT.md index 81dfff0..0510c51 100644 --- a/pkg/agent/CODE_AGENT.md +++ b/pkg/agent/CODE_AGENT.md @@ -26,11 +26,11 @@ You have dedicated search tools. **Prefer them over shelling out to `grep`/`rg`/ Both tools skip `.git`, `node_modules`, `vendor`, `dist`, `build`, `target`, and (for `grep`) binary files automatically. -## Bash +## Bash and node -Use `bash` only for running commands: build, test, git, install, compile, lint — anything that is not file I/O. Bash runs in the working directory `{CWD}`. +Use `bash` only for running commands: build, test, git, install, compile, lint — anything that is not file I/O. Bash runs in the working directory `{CWD}`. Use `node` to execute Node.js scripts in the same working directory. -Pass `timeout_seconds` to give long-running commands more time. Default is 30s; the hard cap is 600s (10 minutes). Asking for more than the cap returns an error rather than being silently clamped. When a command exceeds its budget the result has `timed_out: true` and `exit_code: -1`. +Both tools accept a `timeout_seconds` arg to give long-running commands more time. Default is 30s; the hard cap is 600s (10 minutes). Asking for more than the cap returns an error rather than being silently clamped. When a command exceeds its budget the result has `timed_out: true` and `exit_code: -1`. Stdout and stderr are each captured through a head+tail window. When output overflows, the result includes a `[... truncated N bytes from middle ...]` marker between the head and the tail (so the final lines of a build — which usually contain the actual error — survive). The full byte counts are reported in `stdout_total_bytes` and `stderr_total_bytes`. diff --git a/pkg/agent/tools/exec.go b/pkg/agent/tools/exec.go index 0d78388..4386189 100644 --- a/pkg/agent/tools/exec.go +++ b/pkg/agent/tools/exec.go @@ -1,10 +1,7 @@ package tools import ( - "bytes" "context" - "errors" - "os/exec" "time" "github.com/ludusrusso/wildgecu/pkg/exec/bounded" @@ -84,51 +81,36 @@ func newBashTool(workDir string, cfg ExecConfig) tool.Tool { } // --- node --- -// -// Note: node still uses the legacy exec path. Slice 5 (#81) converts it to -// bounded.Run for parity with bash. Until then the cfg is ignored here so the -// existing behavior — 30s hardcoded timeout, no output truncation — is -// preserved. type nodeInput struct { - Script string `json:"script" description:"The Node.js script to execute"` + Script string `json:"script" description:"The Node.js script to execute"` + TimeoutSeconds int `json:"timeout_seconds,omitempty" description:"Per-call timeout in seconds. Default 30, hard-capped at 600."` } type nodeOutput struct { - Stdout string `json:"stdout"` - Stderr string `json:"stderr"` - ExitCode int `json:"exit_code"` + Stdout string `json:"stdout"` + Stderr string `json:"stderr"` + ExitCode int `json:"exit_code"` + TimedOut bool `json:"timed_out,omitempty"` + StdoutTotalBytes int `json:"stdout_total_bytes"` + StderrTotalBytes int `json:"stderr_total_bytes"` } -func newNodeTool(workDir string, _ ExecConfig) tool.Tool { - return tool.NewTool("node", "Execute a Node.js script and return its output", +func newNodeTool(workDir string, cfg ExecConfig) tool.Tool { + return tool.NewTool("node", + "Execute a Node.js script and return its output. Pass timeout_seconds (default 30, hard-capped at 600) to give long-running scripts more time.", func(ctx context.Context, in nodeInput) (nodeOutput, error) { - ctx, cancel := context.WithTimeout(ctx, 30*time.Second) - defer cancel() - - cmd := exec.CommandContext(ctx, "node", "-e", in.Script) - cmd.Dir = workDir - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - err := cmd.Run() - - exitCode := 0 + res, err := bounded.Run(ctx, "node", []string{"-e", in.Script}, cfg.boundedOpts(workDir, in.TimeoutSeconds)) if err != nil { - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - exitCode = exitErr.ExitCode() - } else { - return nodeOutput{}, err - } + return nodeOutput{}, err } - return nodeOutput{ - Stdout: stdout.String(), - Stderr: stderr.String(), - ExitCode: exitCode, + Stdout: res.Stdout, + Stderr: res.Stderr, + ExitCode: res.ExitCode, + TimedOut: res.TimedOut, + StdoutTotalBytes: res.StdoutTotalBytes, + StderrTotalBytes: res.StderrTotalBytes, }, nil }, ) diff --git a/pkg/agent/tools/exec_test.go b/pkg/agent/tools/exec_test.go index 842b3ea..eb5ead2 100644 --- a/pkg/agent/tools/exec_test.go +++ b/pkg/agent/tools/exec_test.go @@ -254,4 +254,76 @@ func TestNode(t *testing.T) { t.Fatalf("stdout = %q", out.Stdout) } }) + + t.Run("default timeout still applies when timeout_seconds omitted", func(t *testing.T) { + var out nodeOutput + result, err := tl.Execute(context.Background(), map[string]any{ + "script": `console.log("ok")`, + }) + if err != nil { + t.Fatal(err) + } + json.Unmarshal([]byte(result), &out) + if out.Stdout != "ok\n" { + t.Fatalf("stdout = %q", out.Stdout) + } + }) + + t.Run("timeout_seconds arg fires", func(t *testing.T) { + var out nodeOutput + start := time.Now() + result, err := tl.Execute(context.Background(), map[string]any{ + "script": `setTimeout(() => {}, 5000)`, + "timeout_seconds": float64(1), + }) + elapsed := time.Since(start) + if err != nil { + t.Fatal(err) + } + json.Unmarshal([]byte(result), &out) + if !out.TimedOut { + t.Fatalf("expected timed_out=true, got %+v", out) + } + if out.ExitCode != -1 { + t.Fatalf("exit_code = %d, want -1", out.ExitCode) + } + if elapsed > 4*time.Second { + t.Fatalf("expected fast return, elapsed = %s", elapsed) + } + }) + + t.Run("timeout_seconds above hard cap returns boundary error", func(t *testing.T) { + capped := newNodeTool(dir, ExecConfig{MaxTimeoutSeconds: 5}) + _, err := capped.Execute(context.Background(), map[string]any{ + "script": `console.log("hi")`, + "timeout_seconds": float64(9999), + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "exceeds maximum") { + t.Fatalf("error = %q, want it to mention 'exceeds maximum'", err.Error()) + } + }) + + t.Run("large stdout is truncated with marker and total bytes", func(t *testing.T) { + small := newNodeTool(dir, ExecConfig{HeadBytes: 100, TailBytes: 100}) + var out nodeOutput + result, err := small.Execute(context.Background(), map[string]any{ + "script": `for (let i = 1; i <= 200; i++) console.log("line" + String(i).padStart(3, "0") + "-AAAAAAAAAAAA");`, + }) + if err != nil { + t.Fatal(err) + } + json.Unmarshal([]byte(result), &out) + if !strings.Contains(out.Stdout, "[... truncated ") { + t.Fatalf("expected truncation marker in stdout: %q", out.Stdout) + } + if out.StdoutTotalBytes <= 200 { + t.Fatalf("StdoutTotalBytes = %d, want > 200", out.StdoutTotalBytes) + } + if !strings.Contains(out.Stdout, "line200-") { + t.Fatalf("tail lost last line: %q", out.Stdout) + } + }) }