diff --git a/pkg/model/provider/gemini/schema_boolean_test.go b/pkg/model/provider/gemini/schema_boolean_test.go index d1875d594..f18e2c364 100644 --- a/pkg/model/provider/gemini/schema_boolean_test.go +++ b/pkg/model/provider/gemini/schema_boolean_test.go @@ -2,6 +2,8 @@ package gemini import ( "testing" + + "github.com/stretchr/testify/require" ) // A tool input schema containing a boolean sub-schema — the shape a JSON Schema @@ -30,12 +32,8 @@ func TestConvertParametersToSchema_BooleanSubSchema(t *testing.T) { } schema, err := ConvertParametersToSchema(params) - if err != nil { - t.Fatalf("ConvertParametersToSchema: %v", err) - } - if schema == nil { - t.Fatal("nil schema") - } + require.NoError(t, err, "ConvertParametersToSchema failed") + require.NotNil(t, schema, "nil schema") if _, ok := schema.Properties["count"]; !ok { t.Errorf("count property dropped; got %v", schema.Properties) } diff --git a/pkg/selfupdate/exec_windows.go b/pkg/selfupdate/exec_windows.go index 6517efcc9..d00dd09e5 100644 --- a/pkg/selfupdate/exec_windows.go +++ b/pkg/selfupdate/exec_windows.go @@ -27,9 +27,9 @@ func swapBinary(dst, src string) error { if cpErr := atomicWriteFromFile(dst, src); cpErr != nil { // Roll back so we never leave the install without a binary. if rbErr := os.Rename(old, dst); rbErr != nil { - return fmt.Errorf("installing new binary: %w (copy fallback failed: %v; rollback also failed: %v)", err, cpErr, rbErr) + return fmt.Errorf("installing new binary: %w (copy fallback failed: %w; rollback also failed: %w)", err, cpErr, rbErr) } - return fmt.Errorf("installing new binary: %w (copy fallback failed: %v)", err, cpErr) + return fmt.Errorf("installing new binary: %w (copy fallback failed: %w)", err, cpErr) } _ = os.Remove(src) } @@ -48,7 +48,7 @@ func reExecProcess(path string, args, env []string) error { childArgs = args[1:] } - cmd := exec.Command(path, childArgs...) //nolint:gosec // path is our own freshly installed binary + cmd := exec.Command(path, childArgs...) //nolint:noctx // re-exec has no parent context cmd.Env = env cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout diff --git a/pkg/session/session_options_test.go b/pkg/session/session_options_test.go index 5d0c4108f..89791b6da 100644 --- a/pkg/session/session_options_test.go +++ b/pkg/session/session_options_test.go @@ -76,10 +76,13 @@ func TestNewSession_ConsistencyBetweenInitialAndSpawned(t *testing.T) { func TestAddAttachedFile(t *testing.T) { t.Parallel() - foo := filepath.Join(t.TempDir(), "abs", "foo.go") - bar := filepath.Join(t.TempDir(), "abs", "bar.go") + t.Run("deduplicates and preserves order", func(t *testing.T) { t.Parallel() + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + bar := filepath.Join(dir, "bar.go") + s := New() s.AddAttachedFile(foo) s.AddAttachedFile(bar) @@ -105,6 +108,9 @@ func TestAddAttachedFile(t *testing.T) { t.Run("snapshot is independent of session storage", func(t *testing.T) { t.Parallel() + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + s := New() s.AddAttachedFile(foo) snap := s.AttachedFilesSnapshot() @@ -116,24 +122,28 @@ func TestAddAttachedFile(t *testing.T) { func TestRemoveAttachedFile(t *testing.T) { t.Parallel() - root := t.TempDir() - foo := filepath.Join(root, "foo.go") - bar := filepath.Join(root, "bar.go") - baz := filepath.Join(root, "baz.go") - other := filepath.Join(root, "other.go") + t.Run("removes and reports presence", func(t *testing.T) { t.Parallel() + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + bar := filepath.Join(dir, "bar.go") + baz := filepath.Join(dir, "baz.go") + s := New() s.AddAttachedFile(foo) s.AddAttachedFile(bar) s.AddAttachedFile(baz) - assert.True(t, s.RemoveAttachedFile(bar)) assert.Equal(t, []string{foo, baz}, s.AttachedFilesSnapshot()) }) t.Run("reports absent paths", func(t *testing.T) { t.Parallel() + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + other := filepath.Join(dir, "other.go") + s := New() s.AddAttachedFile(foo) assert.False(t, s.RemoveAttachedFile(other)) @@ -143,6 +153,9 @@ func TestRemoveAttachedFile(t *testing.T) { t.Run("no-op on empty list", func(t *testing.T) { t.Parallel() + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + s := New() assert.False(t, s.RemoveAttachedFile(foo)) assert.Empty(t, s.AttachedFilesSnapshot()) @@ -150,6 +163,9 @@ func TestRemoveAttachedFile(t *testing.T) { t.Run("file can be re-attached after removal", func(t *testing.T) { t.Parallel() + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + s := New() s.AddAttachedFile(foo) require.True(t, s.RemoveAttachedFile(foo)) @@ -160,8 +176,9 @@ func TestRemoveAttachedFile(t *testing.T) { func TestWithAttachedFiles(t *testing.T) { t.Parallel() - foo := filepath.Join(t.TempDir(), "abs", "foo.go") - bar := filepath.Join(t.TempDir(), "abs", "bar.go") + dir := t.TempDir() + foo := filepath.Join(dir, "foo.go") + bar := filepath.Join(dir, "bar.go") s := New(WithAttachedFiles([]string{foo, "", "relative/path.go", bar, foo})) assert.Equal(t, []string{foo, bar}, s.AttachedFilesSnapshot()) } diff --git a/pkg/tools/builtin/backgroundjobs/cmd_windows.go b/pkg/tools/builtin/backgroundjobs/cmd_windows.go index d25a83ef1..5a4ce50c2 100644 --- a/pkg/tools/builtin/backgroundjobs/cmd_windows.go +++ b/pkg/tools/builtin/backgroundjobs/cmd_windows.go @@ -31,13 +31,13 @@ func createProcessGroup(proc *os.Process) (*processGroup, error) { if _, err := windows.SetInformationJobObject( job, windows.JobObjectExtendedLimitInformation, - uintptr(unsafe.Pointer(&info)), + uintptr(unsafe.Pointer(&info)), //nolint:gosec // interacting with Windows API uint32(unsafe.Sizeof(info))); err != nil { _ = windows.CloseHandle(job) return nil, err } - handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) + handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) //nolint:gosec // PID fits in uint32 on Windows if err != nil { _ = windows.CloseHandle(job) return nil, err diff --git a/pkg/tools/builtin/filesystem/filesystem.go b/pkg/tools/builtin/filesystem/filesystem.go index ad28b2d20..8e90aee68 100644 --- a/pkg/tools/builtin/filesystem/filesystem.go +++ b/pkg/tools/builtin/filesystem/filesystem.go @@ -589,7 +589,7 @@ func (t *ToolSet) executePostEditCommands(ctx context.Context, filePath string) if len(t.postEditCommands) == 0 { return nil } - return runPostEditCommands(ctx, t.postEditCommands, filePath) + return runPostEditCommands(ctx, t.workingDir, t.postEditCommands, filePath) } // resolvePath resolves a path relative to the working directory. diff --git a/pkg/tools/builtin/filesystem/filesystem_paths_test.go b/pkg/tools/builtin/filesystem/filesystem_paths_test.go index ad68ef368..1f511f9cd 100644 --- a/pkg/tools/builtin/filesystem/filesystem_paths_test.go +++ b/pkg/tools/builtin/filesystem/filesystem_paths_test.go @@ -174,6 +174,7 @@ func TestFilesystemTool_DenyList_TakesPrecedenceOverAllowList(t *testing.T) { tool := newTestToolSet(t, wd, WithAllowList([]string{"."}), WithDenyList([]string{"src/vendor"})) + defer tool.Close() // Allowed by allow-list, not denied. _, err := tool.resolveAndCheckPath("src/main.go") diff --git a/pkg/tools/builtin/filesystem/postedit.go b/pkg/tools/builtin/filesystem/postedit.go index 52935ec9f..5d78e2f8e 100644 --- a/pkg/tools/builtin/filesystem/postedit.go +++ b/pkg/tools/builtin/filesystem/postedit.go @@ -8,19 +8,15 @@ import ( "log/slog" "os/exec" "path/filepath" + "strings" "github.com/docker/docker-agent/pkg/shellpath" ) // runPostEditCommands executes configured shell commands after a file edit. -func runPostEditCommands(ctx context.Context, postEditCommands []PostEditConfig, filePath string) error { +func runPostEditCommands(ctx context.Context, workingDir string, postEditCommands []PostEditConfig, filePath string) error { for _, postEdit := range postEditCommands { - matched, err := filepath.Match(postEdit.Path, filepath.Base(filePath)) - if err != nil { - slog.WarnContext(ctx, "Invalid post-edit pattern", "pattern", postEdit.Path, "error", err) - continue - } - if !matched { + if !matchPostEdit(ctx, postEdit.Path, workingDir, filePath) { continue } @@ -35,3 +31,27 @@ func runPostEditCommands(ctx context.Context, postEditCommands []PostEditConfig, } return nil } + +func matchPostEdit(ctx context.Context, patternStr, workingDir, filePath string) bool { + pattern := filepath.ToSlash(patternStr) + target := filepath.Base(filePath) + if strings.Contains(pattern, "/") { + if workingDir != "" { + rel, err := filepath.Rel(workingDir, filePath) + if err == nil { + target = filepath.ToSlash(rel) + } else { + slog.DebugContext(ctx, "Failed to resolve relative path for post-edit pattern", "workingDir", workingDir, "filePath", filePath, "error", err) + target = filepath.ToSlash(filePath) + } + } else { + target = filepath.ToSlash(filePath) + } + } + matched, err := filepath.Match(pattern, target) + if err != nil { + slog.WarnContext(ctx, "Invalid post-edit pattern", "pattern", patternStr, "error", err) + return false + } + return matched +} diff --git a/pkg/tools/builtin/filesystem/postedit_js.go b/pkg/tools/builtin/filesystem/postedit_js.go index 057aa5252..ece867fc6 100644 --- a/pkg/tools/builtin/filesystem/postedit_js.go +++ b/pkg/tools/builtin/filesystem/postedit_js.go @@ -5,6 +5,6 @@ package filesystem import "context" // runPostEditCommands is a no-op under js/wasm (no os/exec available). -func runPostEditCommands(_ context.Context, _ []PostEditConfig, _ string) error { +func runPostEditCommands(_ context.Context, _ string, _ []PostEditConfig, _ string) error { return nil } diff --git a/pkg/tools/builtin/filesystem/postedit_test.go b/pkg/tools/builtin/filesystem/postedit_test.go new file mode 100644 index 000000000..7a16e8cfe --- /dev/null +++ b/pkg/tools/builtin/filesystem/postedit_test.go @@ -0,0 +1,94 @@ +//go:build !js + +package filesystem + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMatchPostEdit(t *testing.T) { + ctx := t.Context() + workDir := filepath.Join(string(filepath.Separator), "workspace", "app") + + tests := []struct { + name string + pattern string + workingDir string + filePath string + wantMatch bool + }{ + { + name: "basename pattern matches simple file", + pattern: "*.go", + workingDir: workDir, + filePath: filepath.Join(workDir, "main.go"), + wantMatch: true, + }, + { + name: "basename pattern matches nested file", + pattern: "*.go", + workingDir: workDir, + filePath: filepath.Join(workDir, "pkg", "sub", "foo.go"), + wantMatch: true, + }, + { + name: "path-scoped pattern matches relative subpath", + pattern: "pkg/*.go", + workingDir: workDir, + filePath: filepath.Join(workDir, "pkg", "foo.go"), + wantMatch: true, + }, + { + name: "path-scoped pattern does not match different subpath", + pattern: "cmd/*.go", + workingDir: workDir, + filePath: filepath.Join(workDir, "pkg", "foo.go"), + wantMatch: false, + }, + { + name: "nested slash pattern matches multi-level path", + pattern: "pkg/sub/*.go", + workingDir: workDir, + filePath: filepath.Join(workDir, "pkg", "sub", "bar.go"), + wantMatch: true, + }, + { + name: "empty working dir falls back to slash-normalized file path", + pattern: "*.go", + workingDir: "", + filePath: filepath.Join("pkg", "foo.go"), + wantMatch: true, + }, + { + name: "invalid pattern returns false", + pattern: "[invalid", + workingDir: workDir, + filePath: filepath.Join(workDir, "foo.go"), + wantMatch: false, + }, + { + name: "file outside workingDir does not match path-scoped pattern", + pattern: "pkg/*.go", + workingDir: workDir, + filePath: filepath.Join(workDir, "..", "outside", "foo.go"), + wantMatch: false, + }, + { + name: "relative workingDir vs absolute filePath debug log fallback does not match relative pattern", + pattern: "pkg/*.go", + workingDir: "relative/dir", + filePath: filepath.Join(workDir, "pkg", "foo.go"), + wantMatch: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := matchPostEdit(ctx, tt.pattern, tt.workingDir, tt.filePath) + assert.Equal(t, tt.wantMatch, got) + }) + } +} diff --git a/pkg/tools/builtin/shell/cmd_windows.go b/pkg/tools/builtin/shell/cmd_windows.go index 05e11368a..9c2946376 100644 --- a/pkg/tools/builtin/shell/cmd_windows.go +++ b/pkg/tools/builtin/shell/cmd_windows.go @@ -31,13 +31,13 @@ func createProcessGroup(proc *os.Process) (*processGroup, error) { if _, err := windows.SetInformationJobObject( job, windows.JobObjectExtendedLimitInformation, - uintptr(unsafe.Pointer(&info)), + uintptr(unsafe.Pointer(&info)), //nolint:gosec // interacting with Windows API uint32(unsafe.Sizeof(info))); err != nil { _ = windows.CloseHandle(job) return nil, err } - handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) + handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) //nolint:gosec // PID fits in uint32 on Windows if err != nil { _ = windows.CloseHandle(job) return nil, err diff --git a/pkg/tui/internal/editorname/editorname_test.go b/pkg/tui/internal/editorname/editorname_test.go index 95a953319..29d34b546 100644 --- a/pkg/tui/internal/editorname/editorname_test.go +++ b/pkg/tui/internal/editorname/editorname_test.go @@ -9,6 +9,13 @@ import ( "github.com/stretchr/testify/assert" ) +func defaultEditor() string { + if goruntime.GOOS == "windows" { + return "Notepad" + } + return "Vi" +} + func TestFromEnv(t *testing.T) { t.Parallel() @@ -70,7 +77,7 @@ func TestFromEnv(t *testing.T) { name: "Empty (uses platform default)", visual: "", editorEnv: "", - want: map[bool]string{true: "Notepad", false: "Vi"}[goruntime.GOOS == "windows"], + want: defaultEditor(), }, { name: "VSCode Insiders",