feat(agent): prefer dedicated tools over bash (#207)
* feat(agent): prefer dedicated tools over bash * test(core): isolate FIFO setup by platform * fix(ci): address cross-platform test failures
- commit
- 58133ea4b8f85b8ab0e7a16145a48986f6c68ad4
- author
- bnema · · 2026-09-13 15:04 UTC
- committer
- GitHub · · 2026-09-13 15:04 UTC
- parent
- e363e10
- signature
- GPG Unverified
B5690EEEBB952194The key is unknown - refs
- tag: v0.38.0
7 files · +41 -16
M packages/agent/sessionscmd_test.go +4 -0
| @@ -366,6 +366,10 @@ func createPruneTestSession(t *testing.T, root, cwd string) string { | |||
| 366 | 366 | Role: provider.RoleUser, | |
| 367 | 367 | Content: []provider.Content{provider.TextBlock{Text: "hello"}}, | |
| 368 | 368 | }); err != nil { | |
| 369 | + | _ = session.Close() | |
| 370 | + | t.Fatal(err) | |
| 371 | + | } | |
| 372 | + | if err := session.Close(); err != nil { | |
| 369 | 373 | t.Fatal(err) | |
| 370 | 374 | } | |
| 371 | 375 | return session.Path | |
| @@ -366,6 +366,10 @@ func createPruneTestSession(t *testing.T, root, cwd string) string { | |||||
| 366 | Role: provider.RoleUser, | 366 | Role: provider.RoleUser, | ||
| 367 | Content: []provider.Content{provider.TextBlock{Text: "hello"}}, | 367 | Content: []provider.Content{provider.TextBlock{Text: "hello"}}, | ||
| 368 | }); err != nil { | 368 | }); err != nil { | ||
| 369 | + | _ = session.Close() | |||
| 370 | + | t.Fatal(err) | |||
| 371 | + | } | |||
| 372 | + | if err := session.Close(); err != nil { | |||
| 369 | t.Fatal(err) | 373 | t.Fatal(err) | ||
| 370 | } | 374 | } | ||
| 371 | return session.Path | 375 | return session.Path | ||
M packages/agent/systemprompt.go +8 -4
| @@ -35,12 +35,12 @@ type SystemPromptOpts struct { | |||
| 35 | 35 | // | |
| 36 | 36 | // - A one-paragraph identity (who zut is, what the name means, | |
| 37 | 37 | // what the TUI expects for output format). | |
| 38 | - | // - Compact task, skill-priority, handoff, and writing guidance that survive | |
| 39 | - | // custom identities and appended context. | |
| 38 | + | // - Compact tool-selection, task, skill-priority, handoff, and writing | |
| 39 | + | // guidance that survive custom identities and appended context. | |
| 40 | 40 | // - The date + cwd footer so the model has current-context. | |
| 41 | 41 | // | |
| 42 | - | // Everything else (tool listing, operating guidelines, "don't run | |
| 43 | - | // sudo", "prefer edit over write", etc.) is left out because the | |
| 42 | + | // Everything else (tool listing, broad operating guidelines, "don't run | |
| 43 | + | // sudo", etc.) is left out because the | |
| 44 | 44 | // current-generation frontier models already internalise it, and | |
| 45 | 45 | // the tool schemas sent alongside the request carry each tool's | |
| 46 | 46 | // own description. | |
| @@ -85,6 +85,8 @@ func BuildSystemPrompt(o SystemPromptOpts) string { | |||
| 85 | 85 | ||
| 86 | 86 | // Keep shared guidance after optional context, with writing policy last. | |
| 87 | 87 | sb.WriteString("\n\n") | |
| 88 | + | sb.WriteString(toolSelectionGuidance) | |
| 89 | + | sb.WriteString("\n\n") | |
| 88 | 90 | sb.WriteString(taskExecutionGuidance) | |
| 89 | 91 | sb.WriteString("\n\n") | |
| 90 | 92 | sb.WriteString(skillPriorityGuidance) | |
| @@ -101,6 +103,8 @@ Your output renders in a TUI that understands markdown for prose and plain text | |||
| 101 | 103 | ||
| 102 | 104 | For focused changes to an existing file, inspect its current contents and use edit with verbatim oldText taken from that same file. Include only enough context to make each match unambiguous. Use write when creating a file or replacing it wholesale. Do not mutate files through bash redirections or commands such as cat, echo, sed, or tee, because those changes appear as opaque shell output instead of a readable edit diff. Likewise, python is for execution, not an opaque file-edit path: use write/edit for saved code.` | |
| 103 | 105 | ||
| 106 | + | const toolSelectionGuidance = `Prefer dedicated tools over bash: glob for file discovery, grep for text search, ast for syntax-aware search or rewrites, read for inspection, and edit/write for file changes. Use bash when no dedicated tool fits.` | |
| 107 | + | ||
| 104 | 108 | const compactedSummaryHandoffInstruction = `When you see a "## Context Summary (compacted)" message, treat it as a handoff from earlier work. Keep its active constraints and preferences in force, continue the most recent unresolved user request without waiting for the user to type "continue", and follow a newer user request when it supersedes the summary.` | |
| 105 | 109 | ||
| 106 | 110 | const taskExecutionGuidance = `Treat requests such as "can you" as instructions to do the requested work within the user's intended scope; do not stop at a plan or an offer to continue. Proceed with authorized, non-destructive work and reasonable assumptions until the task is complete or genuinely blocked. Follow each progress statement with the action it describes: pair every intermediate update with tool calls or a concrete next step in the same turn. Final answers contain the result of the work or a concrete blocker, never a bare promise to continue. Respect requests for explanation, planning, or review without making unsolicited changes. Ask only when a necessary decision blocks safe, correct progress; complete independent authorized work first. Preserve tool permissions, required confirmations, and explicit approval requirements; do not infer permission for destructive or irreversible actions.` | |
| @@ -35,12 +35,12 @@ type SystemPromptOpts struct { | |||||
| 35 | // | 35 | // | ||
| 36 | // - A one-paragraph identity (who zut is, what the name means, | 36 | // - A one-paragraph identity (who zut is, what the name means, | ||
| 37 | // what the TUI expects for output format). | 37 | // what the TUI expects for output format). | ||
| 38 | - | // - Compact task, skill-priority, handoff, and writing guidance that survive | 38 | + | // - Compact tool-selection, task, skill-priority, handoff, and writing |
| 39 | - | // custom identities and appended context. | 39 | + | // guidance that survive custom identities and appended context. |
| 40 | // - The date + cwd footer so the model has current-context. | 40 | // - The date + cwd footer so the model has current-context. | ||
| 41 | // | 41 | // | ||
| 42 | - | // Everything else (tool listing, operating guidelines, "don't run | 42 | + | // Everything else (tool listing, broad operating guidelines, "don't run |
| 43 | - | // sudo", "prefer edit over write", etc.) is left out because the | 43 | + | // sudo", etc.) is left out because the |
| 44 | // current-generation frontier models already internalise it, and | 44 | // current-generation frontier models already internalise it, and | ||
| 45 | // the tool schemas sent alongside the request carry each tool's | 45 | // the tool schemas sent alongside the request carry each tool's | ||
| 46 | // own description. | 46 | // own description. | ||
| @@ -85,6 +85,8 @@ func BuildSystemPrompt(o SystemPromptOpts) string { | |||||
| 85 | 85 | ||||
| 86 | // Keep shared guidance after optional context, with writing policy last. | 86 | // Keep shared guidance after optional context, with writing policy last. | ||
| 87 | sb.WriteString("\n\n") | 87 | sb.WriteString("\n\n") | ||
| 88 | + | sb.WriteString(toolSelectionGuidance) | |||
| 89 | + | sb.WriteString("\n\n") | |||
| 88 | sb.WriteString(taskExecutionGuidance) | 90 | sb.WriteString(taskExecutionGuidance) | ||
| 89 | sb.WriteString("\n\n") | 91 | sb.WriteString("\n\n") | ||
| 90 | sb.WriteString(skillPriorityGuidance) | 92 | sb.WriteString(skillPriorityGuidance) | ||
| @@ -101,6 +103,8 @@ Your output renders in a TUI that understands markdown for prose and plain text | |||||
| 101 | 103 | ||||
| 102 | For focused changes to an existing file, inspect its current contents and use edit with verbatim oldText taken from that same file. Include only enough context to make each match unambiguous. Use write when creating a file or replacing it wholesale. Do not mutate files through bash redirections or commands such as cat, echo, sed, or tee, because those changes appear as opaque shell output instead of a readable edit diff. Likewise, python is for execution, not an opaque file-edit path: use write/edit for saved code.` | 104 | For focused changes to an existing file, inspect its current contents and use edit with verbatim oldText taken from that same file. Include only enough context to make each match unambiguous. Use write when creating a file or replacing it wholesale. Do not mutate files through bash redirections or commands such as cat, echo, sed, or tee, because those changes appear as opaque shell output instead of a readable edit diff. Likewise, python is for execution, not an opaque file-edit path: use write/edit for saved code.` | ||
| 103 | 105 | ||||
| 106 | + | const toolSelectionGuidance = `Prefer dedicated tools over bash: glob for file discovery, grep for text search, ast for syntax-aware search or rewrites, read for inspection, and edit/write for file changes. Use bash when no dedicated tool fits.` | |||
| 107 | + | ||||
| 104 | const compactedSummaryHandoffInstruction = `When you see a "## Context Summary (compacted)" message, treat it as a handoff from earlier work. Keep its active constraints and preferences in force, continue the most recent unresolved user request without waiting for the user to type "continue", and follow a newer user request when it supersedes the summary.` | 108 | const compactedSummaryHandoffInstruction = `When you see a "## Context Summary (compacted)" message, treat it as a handoff from earlier work. Keep its active constraints and preferences in force, continue the most recent unresolved user request without waiting for the user to type "continue", and follow a newer user request when it supersedes the summary.` | ||
| 105 | 109 | ||||
| 106 | const taskExecutionGuidance = `Treat requests such as "can you" as instructions to do the requested work within the user's intended scope; do not stop at a plan or an offer to continue. Proceed with authorized, non-destructive work and reasonable assumptions until the task is complete or genuinely blocked. Follow each progress statement with the action it describes: pair every intermediate update with tool calls or a concrete next step in the same turn. Final answers contain the result of the work or a concrete blocker, never a bare promise to continue. Respect requests for explanation, planning, or review without making unsolicited changes. Ask only when a necessary decision blocks safe, correct progress; complete independent authorized work first. Preserve tool permissions, required confirmations, and explicit approval requirements; do not infer permission for destructive or irreversible actions.` | 110 | const taskExecutionGuidance = `Treat requests such as "can you" as instructions to do the requested work within the user's intended scope; do not stop at a plan or an offer to continue. Proceed with authorized, non-destructive work and reasonable assumptions until the task is complete or genuinely blocked. Follow each progress statement with the action it describes: pair every intermediate update with tool calls or a concrete next step in the same turn. Final answers contain the result of the work or a concrete blocker, never a bare promise to continue. Respect requests for explanation, planning, or review without making unsolicited changes. Ask only when a necessary decision blocks safe, correct progress; complete independent authorized work first. Preserve tool permissions, required confirmations, and explicit approval requirements; do not infer permission for destructive or irreversible actions.` | ||
M packages/agent/systemprompt_test.go +9 -0
| @@ -6,6 +6,15 @@ import ( | |||
| 6 | 6 | "time" | |
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | + | func TestBuildSystemPromptAlwaysPrefersDedicatedTools(t *testing.T) { | |
| 10 | + | for _, custom := range []string{"", "Custom identity"} { | |
| 11 | + | prompt := BuildSystemPrompt(SystemPromptOpts{Custom: custom}) | |
| 12 | + | if count := strings.Count(prompt, toolSelectionGuidance); count != 1 { | |
| 13 | + | t.Fatalf("dedicated tool guidance count = %d, want 1:\n%s", count, prompt) | |
| 14 | + | } | |
| 15 | + | } | |
| 16 | + | } | |
| 17 | + | ||
| 9 | 18 | func TestBuildSystemPromptAlwaysIncludesFinalWritingGuidance(t *testing.T) { | |
| 10 | 19 | for _, tt := range []struct { | |
| 11 | 20 | name string | |
| @@ -6,6 +6,15 @@ import ( | |||||
| 6 | "time" | 6 | "time" | ||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | + | func TestBuildSystemPromptAlwaysPrefersDedicatedTools(t *testing.T) { | |||
| 10 | + | for _, custom := range []string{"", "Custom identity"} { | |||
| 11 | + | prompt := BuildSystemPrompt(SystemPromptOpts{Custom: custom}) | |||
| 12 | + | if count := strings.Count(prompt, toolSelectionGuidance); count != 1 { | |||
| 13 | + | t.Fatalf("dedicated tool guidance count = %d, want 1:\n%s", count, prompt) | |||
| 14 | + | } | |||
| 15 | + | } | |||
| 16 | + | } | |||
| 17 | + | ||||
| 9 | func TestBuildSystemPromptAlwaysIncludesFinalWritingGuidance(t *testing.T) { | 18 | func TestBuildSystemPromptAlwaysIncludesFinalWritingGuidance(t *testing.T) { | ||
| 10 | for _, tt := range []struct { | 19 | for _, tt := range []struct { | ||
| 11 | name string | 20 | name string | ||
M packages/agent/tools/glob.go +2 -3
| @@ -108,8 +108,7 @@ func (t *GlobTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 108 | 108 | var discovered []string | |
| 109 | 109 | truncated := false | |
| 110 | 110 | ||
| 111 | - | var walkErr error | |
| 112 | - | walkErr = filepath.WalkDir(searchDir, func(path string, d os.DirEntry, err error) error { | |
| 111 | + | walkErr := filepath.WalkDir(searchDir, func(path string, d os.DirEntry, err error) error { | |
| 113 | 112 | if ctxErr := ctx.Err(); ctxErr != nil { | |
| 114 | 113 | return ctxErr | |
| 115 | 114 | } | |
| @@ -201,7 +200,7 @@ func (t *GlobTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||
| 201 | 200 | } | |
| 202 | 201 | if matched { | |
| 203 | 202 | matches = append(matches, t.globDisplayPath(root, args.Path, relSlash)) | |
| 204 | - | discovered = append(discovered, canonicalResourcePath(filepath.Join(searchDir, filepath.FromSlash(relSlash)))) | |
| 203 | + | discovered = append(discovered, canonicalResourcePath(filepath.Join(root, filepath.FromSlash(relSlash)))) | |
| 205 | 204 | if len(matches) >= maxGlobMatches { | |
| 206 | 205 | truncated = true | |
| 207 | 206 | return filepath.SkipAll | |
| @@ -108,8 +108,7 @@ func (t *GlobTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 108 | var discovered []string | 108 | var discovered []string | ||
| 109 | truncated := false | 109 | truncated := false | ||
| 110 | 110 | ||||
| 111 | - | var walkErr error | 111 | + | walkErr := filepath.WalkDir(searchDir, func(path string, d os.DirEntry, err error) error { |
| 112 | - | walkErr = filepath.WalkDir(searchDir, func(path string, d os.DirEntry, err error) error { | |||
| 113 | if ctxErr := ctx.Err(); ctxErr != nil { | 112 | if ctxErr := ctx.Err(); ctxErr != nil { | ||
| 114 | return ctxErr | 113 | return ctxErr | ||
| 115 | } | 114 | } | ||
| @@ -201,7 +200,7 @@ func (t *GlobTool) Execute(ctx context.Context, raw json.RawMessage, progress fu | |||||
| 201 | } | 200 | } | ||
| 202 | if matched { | 201 | if matched { | ||
| 203 | matches = append(matches, t.globDisplayPath(root, args.Path, relSlash)) | 202 | matches = append(matches, t.globDisplayPath(root, args.Path, relSlash)) | ||
| 204 | - | discovered = append(discovered, canonicalResourcePath(filepath.Join(searchDir, filepath.FromSlash(relSlash)))) | 203 | + | discovered = append(discovered, canonicalResourcePath(filepath.Join(root, filepath.FromSlash(relSlash)))) |
| 205 | if len(matches) >= maxGlobMatches { | 204 | if len(matches) >= maxGlobMatches { | ||
| 206 | truncated = true | 205 | truncated = true | ||
| 207 | return filepath.SkipAll | 206 | return filepath.SkipAll | ||
A packages/core/session_prune_fifo_other_test.go +9 -0
| @@ -0,0 +1,9 @@ | |||
| 1 | + | //go:build !unix | |
| 2 | + | ||
| 3 | + | package core | |
| 4 | + | ||
| 5 | + | import "errors" | |
| 6 | + | ||
| 7 | + | func makeFifo(string) error { | |
| 8 | + | return errors.New("unsupported on this platform") | |
| 9 | + | } | |
| @@ -0,0 +1,9 @@ | |||||
| 1 | + | //go:build !unix | |||
| 2 | + | ||||
| 3 | + | package core | |||
| 4 | + | ||||
| 5 | + | import "errors" | |||
| 6 | + | ||||
| 7 | + | func makeFifo(string) error { | |||
| 8 | + | return errors.New("unsupported on this platform") | |||
| 9 | + | } | |||
A packages/core/session_prune_fifo_unix_test.go +9 -0
| @@ -0,0 +1,9 @@ | |||
| 1 | + | //go:build unix | |
| 2 | + | ||
| 3 | + | package core | |
| 4 | + | ||
| 5 | + | import "golang.org/x/sys/unix" | |
| 6 | + | ||
| 7 | + | func makeFifo(path string) error { | |
| 8 | + | return unix.Mkfifo(path, 0o600) | |
| 9 | + | } | |
| @@ -0,0 +1,9 @@ | |||||
| 1 | + | //go:build unix | |||
| 2 | + | ||||
| 3 | + | package core | |||
| 4 | + | ||||
| 5 | + | import "golang.org/x/sys/unix" | |||
| 6 | + | ||||
| 7 | + | func makeFifo(path string) error { | |||
| 8 | + | return unix.Mkfifo(path, 0o600) | |||
| 9 | + | } | |||
M packages/core/session_prune_test.go +0 -9
| @@ -7,8 +7,6 @@ import ( | |||
| 7 | 7 | "strings" | |
| 8 | 8 | "testing" | |
| 9 | 9 | "time" | |
| 10 | - | ||
| 11 | - | "golang.org/x/sys/unix" | |
| 12 | 10 | ) | |
| 13 | 11 | ||
| 14 | 12 | func writePruneTestSession(t *testing.T, path, cwd string) { | |
| @@ -36,10 +34,6 @@ func markHiddenFromSessions(t *testing.T, path string) { | |||
| 36 | 34 | } | |
| 37 | 35 | } | |
| 38 | 36 | ||
| 39 | - | func makeFifo(path string) error { | |
| 40 | - | return unix.Mkfifo(path, 0o600) | |
| 41 | - | } | |
| 42 | - | ||
| 43 | 37 | func strconvQuote(s string) string { | |
| 44 | 38 | quoted := strings.ReplaceAll(s, `\`, `\\`) | |
| 45 | 39 | quoted = strings.ReplaceAll(quoted, `"`, `\"`) | |
| @@ -104,9 +98,6 @@ func TestScanStoredSessionGroupsPreservesProblemEntries(t *testing.T) { | |||
| 104 | 98 | } | |
| 105 | 99 | ||
| 106 | 100 | func TestScanStoredSessionGroupsSkipsNonRegularEntries(t *testing.T) { | |
| 107 | - | if runtime.GOOS == "windows" { | |
| 108 | - | t.Skip("fifo tests need unix privileges") | |
| 109 | - | } | |
| 110 | 101 | root := t.TempDir() | |
| 111 | 102 | writePruneTestSession(t, filepath.Join(root, "ok.jsonl"), "/work/a") | |
| 112 | 103 | fifo := filepath.Join(root, "pipe.jsonl") | |
| @@ -7,8 +7,6 @@ import ( | |||||
| 7 | "strings" | 7 | "strings" | ||
| 8 | "testing" | 8 | "testing" | ||
| 9 | "time" | 9 | "time" | ||
| 10 | - | ||||
| 11 | - | "golang.org/x/sys/unix" | |||
| 12 | ) | 10 | ) | ||
| 13 | 11 | ||||
| 14 | func writePruneTestSession(t *testing.T, path, cwd string) { | 12 | func writePruneTestSession(t *testing.T, path, cwd string) { | ||
| @@ -36,10 +34,6 @@ func markHiddenFromSessions(t *testing.T, path string) { | |||||
| 36 | } | 34 | } | ||
| 37 | } | 35 | } | ||
| 38 | 36 | ||||
| 39 | - | func makeFifo(path string) error { | |||
| 40 | - | return unix.Mkfifo(path, 0o600) | |||
| 41 | - | } | |||
| 42 | - | ||||
| 43 | func strconvQuote(s string) string { | 37 | func strconvQuote(s string) string { | ||
| 44 | quoted := strings.ReplaceAll(s, `\`, `\\`) | 38 | quoted := strings.ReplaceAll(s, `\`, `\\`) | ||
| 45 | quoted = strings.ReplaceAll(quoted, `"`, `\"`) | 39 | quoted = strings.ReplaceAll(quoted, `"`, `\"`) | ||
| @@ -104,9 +98,6 @@ func TestScanStoredSessionGroupsPreservesProblemEntries(t *testing.T) { | |||||
| 104 | } | 98 | } | ||
| 105 | 99 | ||||
| 106 | func TestScanStoredSessionGroupsSkipsNonRegularEntries(t *testing.T) { | 100 | func TestScanStoredSessionGroupsSkipsNonRegularEntries(t *testing.T) { | ||
| 107 | - | if runtime.GOOS == "windows" { | |||
| 108 | - | t.Skip("fifo tests need unix privileges") | |||
| 109 | - | } | |||
| 110 | root := t.TempDir() | 101 | root := t.TempDir() | ||
| 111 | writePruneTestSession(t, filepath.Join(root, "ok.jsonl"), "/work/a") | 102 | writePruneTestSession(t, filepath.Join(root, "ok.jsonl"), "/work/a") | ||
| 112 | fifo := filepath.Join(root, "pipe.jsonl") | 103 | fifo := filepath.Join(root, "pipe.jsonl") | ||