diff --git a/internal/config/config.go b/internal/config/config.go index 630fac9b6e..1bf3d0244b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -222,6 +222,9 @@ func configureViper() { viper.AddConfigPath("$HOME") viper.AddConfigPath(fmt.Sprintf("$XDG_CONFIG_HOME/%s", appName)) viper.AddConfigPath(fmt.Sprintf("$HOME/.config/%s", appName)) + // this allows set variables more easily + // e.g. OPENCODE_AGENTS_CODER_MODEL=gpt-4.1-mini + viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) viper.SetEnvPrefix(strings.ToUpper(appName)) viper.AutomaticEnv() } diff --git a/internal/llm/models/openai.go b/internal/llm/models/openai.go index abe0e30c53..0461358a20 100644 --- a/internal/llm/models/openai.go +++ b/internal/llm/models/openai.go @@ -35,7 +35,7 @@ var OpenAIModels = map[ModelID]Model{ ID: GPT41Mini, Name: "GPT 4.1 mini", Provider: ProviderOpenAI, - APIModel: "gpt-4.1", + APIModel: "gpt-4.1-mini", CostPer1MIn: 0.40, CostPer1MInCached: 0.10, CostPer1MOutCached: 0.0, diff --git a/internal/llm/prompt/coder.go b/internal/llm/prompt/coder.go index 4cfa1314e0..4bc8398dc3 100644 --- a/internal/llm/prompt/coder.go +++ b/internal/llm/prompt/coder.go @@ -1,7 +1,6 @@ package prompt import ( - "context" "fmt" "os" "path/filepath" @@ -167,15 +166,25 @@ NEVER commit changes unless the user explicitly asks you to. It is VERY IMPORTAN You MUST answer concisely with fewer than 4 lines of text (not including tool use or code generation), unless user asks for detail.` +// MAX_PROJECT_SIZE limits the project hierarchy to 4K +const MAX_PROJECT_SIZE = 4 * 1024 + func getEnvironmentInfo() string { cwd := config.WorkingDirectory() isGit := isGitRepo(cwd) platform := runtime.GOOS date := time.Now().Format("1/2/2006") ls := tools.NewLsTool() - r, _ := ls.Run(context.Background(), tools.ToolCall{ - Input: `{"path":"."}`, + r, _ := ls.RunWithParams(tools.LSParams{ + Path: ".", + MaxDepth: 6, }) + + // no more than 4k + projectContent := r.Content + if len(r.Content) > MAX_PROJECT_SIZE { + projectContent = r.Content[:MAX_PROJECT_SIZE] + "..." + } return fmt.Sprintf(`Here is useful information about the environment you are running in: Working directory: %s @@ -186,7 +195,7 @@ Today's date: %s %s - `, cwd, boolToYesNo(isGit), platform, date, r.Content) + `, cwd, boolToYesNo(isGit), platform, date, projectContent) } func isGitRepo(dir string) bool { diff --git a/internal/llm/tools/ls.go b/internal/llm/tools/ls.go index 0febbf8e8f..5d428edbfd 100644 --- a/internal/llm/tools/ls.go +++ b/internal/llm/tools/ls.go @@ -12,8 +12,9 @@ import ( ) type LSParams struct { - Path string `json:"path"` - Ignore []string `json:"ignore"` + Path string `json:"path"` + MaxDepth int `json:"max_depth"` + Ignore []string `json:"ignore"` } type TreeNode struct { @@ -63,7 +64,7 @@ TIPS: - Combine with other tools for more effective exploration` ) -func NewLsTool() BaseTool { +func NewLsTool() *lsTool { return &lsTool{} } @@ -76,6 +77,11 @@ func (l *lsTool) Info() ToolInfo { "type": "string", "description": "The path to the directory to list (defaults to current working directory)", }, + "max_depth": map[string]any{ + // json schema integer: https://json-schema.org/understanding-json-schema/reference/numeric + "type": "integer", + "description": "Limit the maximum depth to traversing depths(defaults to no limit, 0 or negative values also treated as no limit)", + }, "ignore": map[string]any{ "type": "array", "description": "List of glob patterns to ignore", @@ -93,7 +99,10 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) { if err := json.Unmarshal([]byte(call.Input), ¶ms); err != nil { return NewTextErrorResponse(fmt.Sprintf("error parsing parameters: %s", err)), nil } + return l.RunWithParams(params) +} +func (l *lsTool) RunWithParams(params LSParams) (ToolResponse, error) { searchPath := params.Path if searchPath == "" { searchPath = config.WorkingDirectory() @@ -107,7 +116,7 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) { return NewTextErrorResponse(fmt.Sprintf("path does not exist: %s", searchPath)), nil } - files, truncated, err := listDirectory(searchPath, params.Ignore, MaxLSFiles) + files, truncated, err := listDirectory(searchPath, params.Ignore, MaxLSFiles, params.MaxDepth) if err != nil { return ToolResponse{}, fmt.Errorf("error listing directory: %w", err) } @@ -128,7 +137,31 @@ func (l *lsTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error) { ), nil } -func listDirectory(initialPath string, ignorePatterns []string, limit int) ([]string, bool, error) { +// shouldSkipDirDueToDepth checks if a directory should be skipped due to depth limits +// Returns true if the directory should be skipped, false otherwise +func shouldSkipDirDueToDepth(initialPath, currentPath string, maxDepth int) bool { + if maxDepth <= 0 { + return false + } + + relPath, err := filepath.Rel(initialPath, currentPath) + if err != nil { + return false // Don't skip on error + } + + // Calculate depth by counting directory separators + depth := 0 + if relPath != "." { + // If there are no separators, it's still depth 1 (direct child) + // If there are separators, count them and add 1 + depth = strings.Count(relPath, string(filepath.Separator)) + 1 + } + + // Skip directory traversal if we're at or beyond max depth + return depth >= maxDepth +} + +func listDirectory(initialPath string, ignorePatterns []string, limit int, maxDepth int) ([]string, bool, error) { var results []string truncated := false @@ -144,6 +177,16 @@ func listDirectory(initialPath string, ignorePatterns []string, limit int) ([]st return nil } + // Check if we should skip due to depth limits (only for directories) + if info.IsDir() && shouldSkipDirDueToDepth(initialPath, path, maxDepth) { + // Still include this directory in results if it's not the initial path + if path != initialPath { + path = path + string(filepath.Separator) + results = append(results, path) + } + return filepath.SkipDir + } + if path != initialPath { if info.IsDir() { path = path + string(filepath.Separator) diff --git a/internal/llm/tools/ls_test.go b/internal/llm/tools/ls_test.go index 508cb98d36..e63447d91f 100644 --- a/internal/llm/tools/ls_test.go +++ b/internal/llm/tools/ls_test.go @@ -83,19 +83,19 @@ func TestLsTool_Run(t *testing.T) { response, err := tool.Run(context.Background(), call) require.NoError(t, err) - + // Check that visible directories and files are included assert.Contains(t, response.Content, "dir1") assert.Contains(t, response.Content, "dir2") assert.Contains(t, response.Content, "dir3") assert.Contains(t, response.Content, "file1.txt") assert.Contains(t, response.Content, "file2.txt") - + // Check that hidden files and directories are not included assert.NotContains(t, response.Content, ".hidden_dir") assert.NotContains(t, response.Content, ".hidden_file.txt") assert.NotContains(t, response.Content, ".hidden_root_file.txt") - + // Check that __pycache__ is not included assert.NotContains(t, response.Content, "__pycache__") }) @@ -122,7 +122,7 @@ func TestLsTool_Run(t *testing.T) { t.Run("handles empty path parameter", func(t *testing.T) { // For this test, we need to mock the config.WorkingDirectory function // Since we can't easily do that, we'll just check that the response doesn't contain an error message - + tool := NewLsTool() params := LSParams{ Path: "", @@ -138,7 +138,7 @@ func TestLsTool_Run(t *testing.T) { response, err := tool.Run(context.Background(), call) require.NoError(t, err) - + // The response should either contain a valid directory listing or an error // We'll just check that it's not empty assert.NotEmpty(t, response.Content) @@ -173,11 +173,11 @@ func TestLsTool_Run(t *testing.T) { response, err := tool.Run(context.Background(), call) require.NoError(t, err) - + // The output format is a tree, so we need to check for specific patterns // Check that file1.txt is not directly mentioned assert.NotContains(t, response.Content, "- file1.txt") - + // Check that dir1/ is not directly mentioned assert.NotContains(t, response.Content, "- dir1/") }) @@ -189,12 +189,12 @@ func TestLsTool_Run(t *testing.T) { defer func() { os.Chdir(origWd) }() - + // Change to a directory above the temp directory parentDir := filepath.Dir(tempDir) err = os.Chdir(parentDir) require.NoError(t, err) - + tool := NewLsTool() params := LSParams{ Path: filepath.Base(tempDir), @@ -210,7 +210,7 @@ func TestLsTool_Run(t *testing.T) { response, err := tool.Run(context.Background(), call) require.NoError(t, err) - + // Should list the temp directory contents assert.Contains(t, response.Content, "dir1") assert.Contains(t, response.Content, "file1.txt") @@ -291,22 +291,22 @@ func TestCreateFileTree(t *testing.T) { } tree := createFileTree(paths) - + // Check the structure of the tree assert.Len(t, tree, 1) // Should have one root node - + // Check the root node rootNode := tree[0] assert.Equal(t, "path", rootNode.Name) assert.Equal(t, "directory", rootNode.Type) assert.Len(t, rootNode.Children, 1) - + // Check the "to" node toNode := rootNode.Children[0] assert.Equal(t, "to", toNode.Name) assert.Equal(t, "directory", toNode.Type) assert.Len(t, toNode.Children, 3) // file1.txt, dir1, dir2 - + // Find the dir1 node var dir1Node *TreeNode for _, child := range toNode.Children { @@ -315,7 +315,7 @@ func TestCreateFileTree(t *testing.T) { break } } - + require.NotNil(t, dir1Node) assert.Equal(t, "directory", dir1Node.Type) assert.Len(t, dir1Node.Children, 2) // file2.txt and subdir @@ -354,9 +354,9 @@ func TestPrintTree(t *testing.T) { Type: "file", }, } - + result := printTree(tree, "/root") - + // Check the output format assert.Contains(t, result, "- /root/") assert.Contains(t, result, " - dir1/") @@ -402,10 +402,10 @@ func TestListDirectory(t *testing.T) { } t.Run("lists files with no limit", func(t *testing.T) { - files, truncated, err := listDirectory(tempDir, []string{}, 1000) + files, truncated, err := listDirectory(tempDir, []string{}, 1000, 0) require.NoError(t, err) assert.False(t, truncated) - + // Check that visible files and directories are included containsPath := func(paths []string, target string) bool { targetPath := filepath.Join(tempDir, target) @@ -416,34 +416,34 @@ func TestListDirectory(t *testing.T) { } return false } - + assert.True(t, containsPath(files, "dir1")) assert.True(t, containsPath(files, "file1.txt")) assert.True(t, containsPath(files, "file2.txt")) assert.True(t, containsPath(files, "dir1/file3.txt")) - + // Check that hidden files and directories are not included assert.False(t, containsPath(files, ".hidden_dir")) assert.False(t, containsPath(files, ".hidden_file.txt")) }) t.Run("respects limit and returns truncated flag", func(t *testing.T) { - files, truncated, err := listDirectory(tempDir, []string{}, 2) + files, truncated, err := listDirectory(tempDir, []string{}, 2, 0) require.NoError(t, err) assert.True(t, truncated) assert.Len(t, files, 2) }) t.Run("respects ignore patterns", func(t *testing.T) { - files, truncated, err := listDirectory(tempDir, []string{"*.txt"}, 1000) + files, truncated, err := listDirectory(tempDir, []string{"*.txt"}, 1000, 0) require.NoError(t, err) assert.False(t, truncated) - + // Check that no .txt files are included for _, file := range files { assert.False(t, strings.HasSuffix(file, ".txt"), "Found .txt file: %s", file) } - + // But directories should still be included containsDir := false for _, file := range files { @@ -454,4 +454,222 @@ func TestListDirectory(t *testing.T) { } assert.True(t, containsDir) }) -} \ No newline at end of file +} + +func TestListDirectoryWithMaxDepth(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "list_directory_max_depth_test") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + // Create a deeper directory structure + testDirs := []string{ + "level1", + "level1/level2", + "level1/level2/level3", + "level1/level2/level3/level4", + "other_level1", + } + + testFiles := []string{ + "root_file.txt", + "level1/level1_file.txt", + "level1/level2/level2_file.txt", + "level1/level2/level3/level3_file.txt", + "level1/level2/level3/level4/level4_file.txt", + "other_level1/other_file.txt", + } + + // Create directories + for _, dir := range testDirs { + dirPath := filepath.Join(tempDir, dir) + err := os.MkdirAll(dirPath, 0755) + require.NoError(t, err) + } + + // Create files + for _, file := range testFiles { + filePath := filepath.Join(tempDir, file) + err := os.WriteFile(filePath, []byte("test content"), 0644) + require.NoError(t, err) + } + + t.Run("maxDepth 1 should include only first level", func(t *testing.T) { + files, truncated, err := listDirectory(tempDir, []string{}, 1000, 1) + require.NoError(t, err) + assert.False(t, truncated) + + // Should include first level directories and files + containsPath := func(paths []string, target string) bool { + targetPath := filepath.Join(tempDir, target) + for _, path := range paths { + if strings.HasPrefix(path, targetPath) { + return true + } + } + return false + } + + // Should include level 1 items + assert.True(t, containsPath(files, "level1/")) + assert.True(t, containsPath(files, "other_level1/")) + assert.True(t, containsPath(files, "root_file.txt")) + + // Should not include contents of level 1 directories (since we don't traverse them) + assert.False(t, containsPath(files, "level1/level1_file.txt")) + assert.False(t, containsPath(files, "other_level1/other_file.txt")) + + // Should not include level 2 and deeper items + assert.False(t, containsPath(files, "level1/level2/")) + assert.False(t, containsPath(files, "level1/level2/level2_file.txt")) + assert.False(t, containsPath(files, "level1/level2/level3/")) + }) + + t.Run("maxDepth 2 should include up to second level", func(t *testing.T) { + files, truncated, err := listDirectory(tempDir, []string{}, 1000, 2) + require.NoError(t, err) + assert.False(t, truncated) + + containsPath := func(paths []string, target string) bool { + targetPath := filepath.Join(tempDir, target) + for _, path := range paths { + if strings.HasPrefix(path, targetPath) { + return true + } + } + return false + } + + // Should include up to level 2 directories + assert.True(t, containsPath(files, "level1/level2/")) + // Should include level 1 files inside directories + assert.True(t, containsPath(files, "level1/level1_file.txt")) + // Should include other level 1 files + assert.True(t, containsPath(files, "other_level1/other_file.txt")) + + // Should not include files inside level 2 directories (they would be at depth 3) + assert.False(t, containsPath(files, "level1/level2/level2_file.txt")) + // Should not include level 3 and deeper items + assert.False(t, containsPath(files, "level1/level2/level3/")) + assert.False(t, containsPath(files, "level1/level2/level3/level3_file.txt")) + }) + + t.Run("maxDepth 0 should traverse all levels", func(t *testing.T) { + files, truncated, err := listDirectory(tempDir, []string{}, 1000, 0) + require.NoError(t, err) + assert.False(t, truncated) + + containsPath := func(paths []string, target string) bool { + targetPath := filepath.Join(tempDir, target) + for _, path := range paths { + if strings.HasPrefix(path, targetPath) { + return true + } + } + return false + } + + // Should include all levels + assert.True(t, containsPath(files, "level1/level2/level3/level4/")) + assert.True(t, containsPath(files, "level1/level2/level3/level4/level4_file.txt")) + }) +} + +func TestShouldSkipDirDueToDepth(t *testing.T) { + t.Run("maxDepth <= 0 should never skip", func(t *testing.T) { + testCases := []struct { + name string + maxDepth int + expected bool + }{ + {"maxDepth 0", 0, false}, + {"maxDepth -1", -1, false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := shouldSkipDirDueToDepth("/root", "/root/subdir", tc.maxDepth) + assert.Equal(t, tc.expected, result, "maxDepth <= 0 should never skip") + }) + } + }) + + t.Run("directories at or beyond maxDepth should be skipped", func(t *testing.T) { + testCases := []struct { + name string + initialPath string + currentPath string + maxDepth int + expected bool + }{ + {"dir at depth 1, maxDepth 1", "/root", "/root/subdir", 1, true}, + {"dir at depth 2, maxDepth 1", "/root", "/root/dir1/dir2", 1, true}, + {"dir at depth 2, maxDepth 2", "/root", "/root/dir1/dir2", 2, true}, + {"dir at depth 1, maxDepth 2", "/root", "/root/subdir", 2, false}, + {"dir at depth 2, maxDepth 3", "/root", "/root/dir1/dir2", 3, false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := shouldSkipDirDueToDepth(tc.initialPath, tc.currentPath, tc.maxDepth) + assert.Equal(t, tc.expected, result, tc.name) + }) + } + }) + + t.Run("same path as initial should not be skipped", func(t *testing.T) { + result := shouldSkipDirDueToDepth("/root", "/root", 1) + assert.False(t, result, "initial path should not be skipped") + }) + + t.Run("handles path with separators correctly", func(t *testing.T) { + testCases := []struct { + name string + initialPath string + currentPath string + maxDepth int + expected bool + }{ + {"Unix path depth 1", "/root", "/root/subdir", 1, true}, + {"Unix path depth 2", "/root", "/root/dir1/dir2", 2, true}, + {"Unix path depth 3", "/root", "/root/dir1/dir2/dir3", 3, true}, + {"Unix path depth 2 with maxDepth 3", "/root", "/root/dir1/dir2", 3, false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := shouldSkipDirDueToDepth(tc.initialPath, tc.currentPath, tc.maxDepth) + assert.Equal(t, tc.expected, result, tc.name) + }) + } + }) + + t.Run("handles error in filepath.Rel gracefully", func(t *testing.T) { + // This test case should handle scenarios where filepath.Rel might fail + // For example, when paths are on different drives on Windows + result := shouldSkipDirDueToDepth("", "", 1) + assert.False(t, result, "should not skip when filepath.Rel fails") + }) + + t.Run("complex nested paths", func(t *testing.T) { + initialPath := "/home/user/project" + + testCases := []struct { + name string + currentPath string + maxDepth int + expected bool + }{ + {"nested dir at exact maxDepth", "/home/user/project/src/main", 2, true}, + {"nested dir under maxDepth", "/home/user/project/src", 2, false}, + {"deeply nested dir", "/home/user/project/src/main/java/com", 3, true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := shouldSkipDirDueToDepth(initialPath, tc.currentPath, tc.maxDepth) + assert.Equal(t, tc.expected, result, tc.name) + }) + } + }) +}