diff --git a/cmd/odek/batch_patch_early_stop_test.go b/cmd/odek/batch_patch_early_stop_test.go new file mode 100644 index 0000000..0d8f9ed --- /dev/null +++ b/cmd/odek/batch_patch_early_stop_test.go @@ -0,0 +1,67 @@ +package main + +// Regression test for the batch_patch early-stop contract: the tool +// description has promised "at the first failing edit the remaining edits +// are skipped (early-stop)" since before v1.41.1. The loop previously +// continued past failures — this pins the documented behavior. + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBatchPatchEarlyStopOnFirstFailure(t *testing.T) { + dir := t.TempDir() + pathA := filepath.Join(dir, "a.txt") + pathC := filepath.Join(dir, "c.txt") + if err := os.WriteFile(pathA, []byte("alpha\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(pathC, []byte("gamma\n"), 0o644); err != nil { + t.Fatal(err) + } + + tool := &batchPatchTool{restrictToCWD: false} + args, _ := json.Marshal(map[string]any{ + "patches": []map[string]any{ + {"path": pathA, "old_string": "alpha", "new_string": "ALPHA"}, + // Fails: old_string not present in a.txt after edit 1. + {"path": pathA, "old_string": "nope", "new_string": "x"}, + {"path": pathC, "old_string": "gamma", "new_string": "GAMMA"}, + }, + }) + out, err := tool.Call(string(args)) + if err != nil { + t.Fatalf("Call error: %v", err) + } + + var resp struct { + Results []struct { + Path string `json:"path"` + Success bool `json:"success"` + Error string `json:"error,omitempty"` + } `json:"results"` + } + if err := json.Unmarshal([]byte(out), &resp); err != nil { + t.Fatalf("decode: %v\nraw: %s", err, out) + } + if len(resp.Results) != 3 { + t.Fatalf("results = %d entries, want 3", len(resp.Results)) + } + if !resp.Results[0].Success { + t.Errorf("patch 1 should succeed: %+v", resp.Results[0]) + } + if resp.Results[1].Success || resp.Results[1].Error == "" { + t.Errorf("patch 2 should fail: %+v", resp.Results[1]) + } + if !strings.Contains(resp.Results[2].Error, "skipped") { + t.Errorf("patch 3 must be skipped (early-stop), got: %+v", resp.Results[2]) + } + got, _ := os.ReadFile(pathC) + if string(got) != "gamma\n" { + t.Errorf("c.txt was modified despite early-stop: %q", got) + } +} diff --git a/cmd/odek/browser_tool.go b/cmd/odek/browser_tool.go index 997f6ee..ed8fbca 100644 --- a/cmd/odek/browser_tool.go +++ b/cmd/odek/browser_tool.go @@ -279,7 +279,7 @@ func (t *browserTool) doSnaPshot() (string, error) { defer t.state.mu.Unlock() if t.state.current == nil { - return jsonError("no page loaded — call browser_navigate(url) first") + return jsonError("no page loaded — call browser with action \"navigate\" and url first") } return jsonResult(browserResult{ @@ -300,7 +300,7 @@ func (t *browserTool) doClick(ref string) (string, error) { t.state.mu.Unlock() if current == nil { - return jsonError("no page loaded — call browser_navigate(url) first") + return jsonError("no page loaded — call browser with action \"navigate\" and url first") } // Find the element by ref @@ -313,7 +313,7 @@ func (t *browserTool) doClick(ref string) (string, error) { } if target == nil { - return jsonError(fmt.Sprintf("element %q not found on current page. Use browser_snapshot() to see available refs.", ref)) + return jsonError(fmt.Sprintf("element %q not found on current page. Use the snapshot action to list available refs.", ref)) } if target.Type == "link" { diff --git a/cmd/odek/file_tool.go b/cmd/odek/file_tool.go index 5c70cfa..e7d886d 100644 --- a/cmd/odek/file_tool.go +++ b/cmd/odek/file_tool.go @@ -218,6 +218,7 @@ func (t *readFileTool) Name() string { return "read_file" } func (t *readFileTool) Description() string { return `Read a text file with line numbers and pagination. +Replaces shell cat / sed -n 'X,Yp' / head — zero forks, line-numbered, size-capped output. Returns file content prefixed with line numbers (LINE_NUM|CONTENT). Use offset and limit to read specific sections of large files. Cannot read binary files — use base64 or checksum for binary content.` @@ -567,7 +568,7 @@ func (t *searchFilesTool) Schema() any { }, "limit": map[string]any{ "type": "integer", - "description": "Maximum results to return (default: 50).", + "description": "Maximum results (default: 50, max: 500). The walk stops silently at the cap — raise to 200+ when completeness matters.", }, }, "required": []string{"pattern"}, @@ -1554,7 +1555,7 @@ Examples: glob(pattern="**/*.py") — all Python files recursively glob(pattern="test_*") — files starting with test_ -Returns an array of {path, size, is_dir} for each match.` +Returns an array of {path, size, is_dir} for each match, wrapped as {"matches":[…]}; an empty result is {"matches":null} — that means zero matches, not an error.` } type globArgs struct { @@ -1588,7 +1589,7 @@ func (t *globTool) Schema() any { }, "limit": map[string]any{ "type": "integer", - "description": "Maximum results to return (default: 50).", + "description": "Maximum results (default: 50, max: 1000). Silently truncated, newest first — raise before concluding a file doesn't exist.", }, }, "required": []string{"pattern"}, @@ -1680,6 +1681,7 @@ func (t *fileInfoTool) Name() string { return "file_info" } func (t *fileInfoTool) Description() string { return `Get file or directory metadata without reading content. +Replaces shell stat / du / ls -l metadata checks — exact bytes + mode, zero forks. Returns size, modification time, file mode, and type flags. Uses Lstat — does NOT follow symlinks. Zero-fork — pure Go file stat with no subprocess.` diff --git a/cmd/odek/perf_tools.go b/cmd/odek/perf_tools.go index 1ef919c..b2e14f0 100644 --- a/cmd/odek/perf_tools.go +++ b/cmd/odek/perf_tools.go @@ -163,6 +163,14 @@ func (t *batchPatchTool) Call(argsJSON string) (result string, err error) { for idx, p := range args.Patches { entry := batchPatchEntry{Path: p.Path} + // Early-stop contract (AGENTS.md, tool description): the first + // failing edit stops the batch — later edits are skipped, not + // attempted, so the model never re-applies against a stale plan. + if idx > 0 && results[idx-1].Error != "" { + entry.Error = "skipped: an earlier patch failed (early-stop)" + results[idx] = entry + continue + } if p.OldString == "" { entry.Error = "old_string is required" results[idx] = entry @@ -948,7 +956,7 @@ type diffTool struct { func (t *diffTool) Name() string { return "diff" } func (t *diffTool) Description() string { - return `Compare two files and return structured hunks. Each hunk has a type (equal/added/removed) and line-by-line content. Use path_a+path_b for file-vs-file, or path+content for file-vs-string. Zero-fork LCS-based diff — no subprocess spawned.` + return `Compare two files and return structured hunks. Replaces shell diff / cmp — and uniquely compares a file against inline content (path + content) without temp files or process substitution. Each hunk has a type (equal/added/removed) and line-by-line content. Zero-fork LCS-based diff — no subprocess spawned.` } type diffArgs struct { @@ -1154,7 +1162,7 @@ type countLinesTool struct { func (t *countLinesTool) Name() string { return "count_lines" } func (t *countLinesTool) Description() string { - return `Count lines, bytes, and characters in one or more files. Streaming scanner — zero-alloc on content, zero subprocess forks. Per-file and aggregate totals.` + return `Count lines, bytes, and characters in one or more files. Replaces shell wc -l / wc -c — exact per-file and aggregate totals, zero subprocess forks. Streaming scanner, zero-alloc on content.` } type countFileArg struct { @@ -1327,7 +1335,7 @@ type multiGrepTool struct { func (t *multiGrepTool) Name() string { return "multi_grep" } func (t *multiGrepTool) Description() string { - return `Search for multiple regex patterns in parallel across files. Each pattern runs its own directory walk with bounded concurrency. Returns structured {pattern, path, line, content} results. Replaces N serial search_files calls.` + return `Search for multiple regex patterns in parallel across files. Each pattern runs its own directory walk with bounded concurrency. Returns structured {pattern, path, line, content} results. Replaces shell grep -rn / rg as well as N serial search_files calls — structured, capped output, zero subprocess forks.` } type grepMatch struct { @@ -1368,7 +1376,7 @@ func (t *multiGrepTool) Schema() any { }, "path": map[string]any{"type": "string", "description": "Root directory (default: '.')."}, "file_glob": map[string]any{"type": "string", "description": "Filter files by glob (e.g. '*.go')."}, - "limit": map[string]any{"type": "integer", "description": "Max matches per pattern (default: 50)."}, + "limit": map[string]any{"type": "integer", "description": "Max matches per pattern (default: 50). The walk stops silently at the cap — raise to 200+ when completeness matters."}, }, "required": []string{"patterns"}, } @@ -1540,7 +1548,7 @@ type jsonQueryTool struct { func (t *jsonQueryTool) Name() string { return "json_query" } func (t *jsonQueryTool) Description() string { - return `Parse a JSON file and extract a value using a dot-path query. Supports array indexing with [N]. Empty query returns the entire parsed JSON. Zero-fork — pure Go JSON traversal.` + return `Parse a JSON file and extract a value using a dot-path query. Supports array indexing with [N]. Empty query returns the entire parsed JSON. Zero-fork — pure Go JSON traversal. Works inside the sandbox where shell jq may be absent.` } type jsonQueryArgs struct { @@ -1712,7 +1720,7 @@ type treeTool struct { func (t *treeTool) Name() string { return "tree" } func (t *treeTool) Description() string { - return `List the directory tree with file counts, sizes, and nesting. Returns a structured tree: each entry shows path, is_dir, file_count, total_size, children, depth. Zero-fork — pure Go directory walk.` + return `List the directory tree with file counts, sizes, and nesting. Returns a structured tree: each entry shows path, is_dir, file_count, total_size, children, depth. Zero-fork — pure Go directory walk. Entries deeper than max_depth (default 3, max 10) are silently cut — pass 10 before concluding a file is absent.` } type treeArgs struct { @@ -1901,7 +1909,7 @@ type checksumTool struct { func (t *checksumTool) Name() string { return "checksum" } func (t *checksumTool) Description() string { - return `Compute cryptographic hashes of files using SHA-256 (default), SHA-1, or MD5. Uses Go crypto stdlib — zero subprocess fork, pure Go implementation.` + return `Compute cryptographic hashes of files using SHA-256 (default), SHA-1, or MD5. Uses Go crypto stdlib — zero subprocess fork, pure Go implementation; works inside the sandbox where shell hash tools may be absent.` } type checksumFileArg struct { @@ -2033,7 +2041,7 @@ type sortTool struct { func (t *sortTool) Name() string { return "sort" } func (t *sortTool) Description() string { - return `Sort lines in one or more files. Supports ascending (default), descending, unique (dedup), numeric, case-insensitive, and reverse. For multiple files, results are merged. Zero-fork — pure Go sort with no subprocess.` + return `Sort lines in one or more files. Supports ascending (default), descending, unique (dedup), numeric, case-insensitive, and reverse. For multiple files, results are merged. Returns the sorted text in the result — source files are never modified; persist with write_file. Zero-fork — pure Go sort with no subprocess.` } type sortArgs struct { @@ -2235,7 +2243,7 @@ type headTailTool struct { func (t *headTailTool) Name() string { return "head_tail" } func (t *headTailTool) Description() string { - return `Read the first or last N lines of one or more files. Reports the file's exact total line count (the head path scans the whole file, bounded by a 1 MiB line buffer). Supports multiple files in parallel. Zero-fork — pure Go scanner.` + return `Read the first or last N lines of one or more files. Reports the file's exact total line count (the head path scans the whole file, bounded by a 1 MiB line buffer). Supports multiple files in parallel. Zero-fork — pure Go scanner. Default 10 lines, max 100 — this is a peek, not the file; for whole content use read_file, and compare the returned count to total_lines before trusting it.` } type headTailFileArg struct { @@ -2429,7 +2437,7 @@ type base64Tool struct { func (t *base64Tool) Name() string { return "base64" } func (t *base64Tool) Description() string { - return `Encode or decode base64. Supports file input (path) or inline string (content). Encode: file or string → base64. Decode: base64 string → decoded string. Zero-fork — pure Go encoding. Use path for file, content for inline string, decode=true to decode.` + return `Encode or decode base64. Supports file input (path) or inline string (content). Encode: file or string → base64. Decode: base64 string → decoded string. Zero-fork — pure Go encoding; works inside the sandbox where shell xxd/openssl may be absent. Use path for file, content for inline string, decode=true to decode.` } type base64Args struct { @@ -2523,7 +2531,7 @@ type trTool struct { func (t *trTool) Name() string { return "tr" } func (t *trTool) Description() string { - return `Transform text: case conversion, character replacement, string substitution, character deletion. Operates on a file or inline content. Zero-fork — pure Go strings transformations.` + return `Transform text: case conversion, character replacement, string substitution, character deletion. READ-ONLY: operates on a file or inline content and returns the transformed text in the result — the source file is NEVER modified; persist changes with write_file or patch. Zero-fork — pure Go string transformations.` } type trTransform struct { @@ -2676,7 +2684,7 @@ type wordCountTool struct { func (t *wordCountTool) Name() string { return "word_count" } func (t *wordCountTool) Description() string { - return `Count words, lines, and characters in one or more files. Streaming scanner — no full-content load. Returns per-file and aggregate totals. Zero-fork — pure Go scanner.` + return `Count words, lines, and characters in one or more files. Replaces shell wc — exact per-file and aggregate totals, zero subprocess forks. Streaming scanner, no full-content load.` } type wordCountFileArg struct { diff --git a/cmd/odek/perf_tools_test.go b/cmd/odek/perf_tools_test.go index 71ea40b..0f8d469 100644 --- a/cmd/odek/perf_tools_test.go +++ b/cmd/odek/perf_tools_test.go @@ -122,7 +122,11 @@ func TestBatchPatch_MultipleFiles(t *testing.T) { } } -func TestBatchPatch_ContinueOnError(t *testing.T) { +// TestBatchPatch_EarlyStop pins the documented early-stop contract +// (tool Description(), promised since pre-v1.41.1): the first failing edit +// stops the batch; later edits are reported skipped and never touch disk, +// while edits applied before the failure are kept. +func TestBatchPatch_EarlyStop(t *testing.T) { dir := t.TempDir() path1 := filepath.Join(dir, "a.txt") os.WriteFile(path1, []byte("hello"), 0644) @@ -152,13 +156,14 @@ func TestBatchPatch_ContinueOnError(t *testing.T) { if r.Results[1].Error == "" { t.Errorf("second patch should have error (file not found)") } - if !r.Results[2].Success { - t.Errorf("third patch should succeed (independent of second patch failure), got: %s", r.Results[2].Error) + if r.Results[2].Success || !strings.Contains(r.Results[2].Error, "skipped") { + t.Errorf("third patch should be skipped (early-stop), got: %+v", r.Results[2]) } - // Verify file content: first patch changed hello→hi, third changed hi→bye + // Edits applied BEFORE the failure are kept: hello→hi stuck, and the + // skipped third patch never ran (file stays "hi", not "bye"). data, _ := os.ReadFile(path1) - if string(data) != "bye" { - t.Errorf("file content should be 'bye', got: %s", string(data)) + if string(data) != "hi" { + t.Errorf("file content should be 'hi' (first edit kept, third skipped), got: %s", string(data)) } } diff --git a/cmd/odek/subagent_tool.go b/cmd/odek/subagent_tool.go index b13ac60..a2b9288 100644 --- a/cmd/odek/subagent_tool.go +++ b/cmd/odek/subagent_tool.go @@ -226,7 +226,7 @@ func (t *delegateTasksTool) Schema() any { "trust_level": map[string]any{ "type": "string", "enum": []string{"trusted", "untrusted"}, - "description": "Trust level of the goal/context strings. Set to \"untrusted\" when any portion was derived from external content (fetched pages, files outside CWD, MCP tool output). Untrusted tasks run with stricter approval defaults in the sub-agent.", + "description": "Trust level of the goal/context strings. Omitted = \"untrusted\" — stricter approval defaults, the safe choice. Set \"trusted\" only when every part of goal/context is internally sourced (no fetched pages, no outside-CWD files, no MCP tool output); trust never increases downward.", }, "max_risk": map[string]any{ "type": "string",