From 13acdb7df92105485b10b707cca7218e455d5aa3 Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Thu, 23 Jul 2026 13:53:52 +0530 Subject: [PATCH 1/2] fix(tui): resolve cross-platform root detection and fallback paths in pickers --- pkg/tui/dialog/file_picker.go | 2 +- pkg/tui/dialog/working_dir_picker.go | 4 ++-- pkg/tui/tui.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/tui/dialog/file_picker.go b/pkg/tui/dialog/file_picker.go index 20db586c5..4c02e4db0 100644 --- a/pkg/tui/dialog/file_picker.go +++ b/pkg/tui/dialog/file_picker.go @@ -144,7 +144,7 @@ func (d *filePickerDialog) loadDirectory() { d.scrollview.SetScrollOffset(0) d.err = nil - if d.currentDir != "/" { + if filepath.Dir(d.currentDir) != d.currentDir { d.entries = append(d.entries, fileEntry{ name: "..", path: filepath.Dir(d.currentDir), diff --git a/pkg/tui/dialog/working_dir_picker.go b/pkg/tui/dialog/working_dir_picker.go index f2f1ba4bf..30af95d41 100644 --- a/pkg/tui/dialog/working_dir_picker.go +++ b/pkg/tui/dialog/working_dir_picker.go @@ -168,7 +168,7 @@ func NewWorkingDirPickerDialog(ctx context.Context, recentDirs, favoriteDirs []s var err error cwd, err = os.Getwd() if err != nil { - cwd = "/" + cwd = "." } } @@ -258,7 +258,7 @@ func (d *workingDirPickerDialog) loadBrowseDirectory() { kind: entryUseThisDir, }) - if d.currentDir != "/" { + if filepath.Dir(d.currentDir) != d.currentDir { d.browseEntries = append(d.browseEntries, dirEntry{ name: "..", path: filepath.Dir(d.currentDir), diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index 5dff84ce8..278afd5aa 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -1999,7 +1999,7 @@ func (m *appModel) handleCloseTab(sessionID string) (tea.Model, tea.Cmd) { workingDir, _ = os.Getwd() } if workingDir == "" { - workingDir = "/" + workingDir = "." } return m.handleSpawnSession(workingDir) } From 88d73b26749ad3163dcbad82bf81f80664a5d4d5 Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Tue, 4 Aug 2026 17:17:03 +0530 Subject: [PATCH 2/2] test(tui): add regression tests and fix linting issues Add regression tests for root directory fallback and empty initial working directory as requested by maintainer. Also fix unrelated failing tests on Windows due to slash normalization, and resolve gosec, errorlint, and forbidigo lint issues. --- pkg/selfupdate/exec_windows.go | 6 +-- .../builtin/backgroundjobs/cmd_windows.go | 4 +- pkg/tools/builtin/shell/cmd_windows.go | 4 +- pkg/tui/dialog/file_picker_test.go | 18 +++++++++ pkg/tui/dialog/session_browser_test.go | 2 +- pkg/tui/dialog/skills_test.go | 4 +- pkg/tui/dialog/working_dir_picker_test.go | 40 +++++++++++++++++++ 7 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 pkg/tui/dialog/working_dir_picker_test.go diff --git a/pkg/selfupdate/exec_windows.go b/pkg/selfupdate/exec_windows.go index 6517efcc9..e5098d1af 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 // path is our own freshly installed binary; no context needed for re-exec cmd.Env = env cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout diff --git a/pkg/tools/builtin/backgroundjobs/cmd_windows.go b/pkg/tools/builtin/backgroundjobs/cmd_windows.go index d25a83ef1..0196b62a4 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 // Windows API requires unsafe pointer 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 is safe to convert to uint32 on Windows if err != nil { _ = windows.CloseHandle(job) return nil, err diff --git a/pkg/tools/builtin/shell/cmd_windows.go b/pkg/tools/builtin/shell/cmd_windows.go index 05e11368a..03908fcc0 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 // Windows API requires unsafe pointer 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 is safe to convert to uint32 on Windows if err != nil { _ = windows.CloseHandle(job) return nil, err diff --git a/pkg/tui/dialog/file_picker_test.go b/pkg/tui/dialog/file_picker_test.go index d9a4c6226..8ec18683c 100644 --- a/pkg/tui/dialog/file_picker_test.go +++ b/pkg/tui/dialog/file_picker_test.go @@ -317,6 +317,24 @@ func TestFilePickerParentDirEntry(t *testing.T) { require.Equal(t, filepath.Dir(dir), d.entries[0].path) } +func TestFilePickerRootHasNoParentDirEntry(t *testing.T) { + t.Parallel() + + // Get the root of the current working directory to test root behavior cross-platform + cwd, err := os.Getwd() + require.NoError(t, err) + root := filepath.VolumeName(cwd) + string(filepath.Separator) + + d := newTestFilePickerDialog(root) + + // Ensure there's no ".." entry + for _, e := range d.entries { + if e.name == ".." { + t.Errorf("root directory should not have a parent dir entry, but got '..'") + } + } +} + func TestFilePickerFilterPreservesParentDir(t *testing.T) { t.Parallel() dir := setupTestDir(t) diff --git a/pkg/tui/dialog/session_browser_test.go b/pkg/tui/dialog/session_browser_test.go index dba80f25c..5ff499e3e 100644 --- a/pkg/tui/dialog/session_browser_test.go +++ b/pkg/tui/dialog/session_browser_test.go @@ -361,7 +361,7 @@ func TestSessionBrowserWorkspaceGrouping(t *testing.T) { view := d.View() require.Contains(t, view, sessionBrowserHeaderWorkspace) require.Contains(t, view, sessionBrowserHeaderElsewhere) - require.Contains(t, view, "/work/other", "sessions from another workspace should show their directory") + require.Contains(t, view, filepath.FromSlash("/work/other"), "sessions from another workspace should show their directory") } func TestSessionBrowserWorkspaceGroupingFlatWithoutWorkspace(t *testing.T) { diff --git a/pkg/tui/dialog/skills_test.go b/pkg/tui/dialog/skills_test.go index ee34fa8c1..978718a56 100644 --- a/pkg/tui/dialog/skills_test.go +++ b/pkg/tui/dialog/skills_test.go @@ -42,12 +42,12 @@ func TestNewSkillsDialog_RendersSkills(t *testing.T) { assert.Contains(t, out, "Skills (2)") assert.Contains(t, out, "commit") assert.Contains(t, out, "Commit local changes") - assert.Contains(t, out, "from: skills/commit") + assert.Contains(t, out, filepath.FromSlash("from: skills/commit")) assert.Contains(t, out, "local") assert.Contains(t, out, "fork") assert.Contains(t, out, "poem") assert.Contains(t, out, "Prints a poem") - assert.Contains(t, out, "from: cache/skills/poem") + assert.Contains(t, out, filepath.FromSlash("from: cache/skills/poem")) assert.Contains(t, out, "remote") } diff --git a/pkg/tui/dialog/working_dir_picker_test.go b/pkg/tui/dialog/working_dir_picker_test.go new file mode 100644 index 000000000..1a85abcb8 --- /dev/null +++ b/pkg/tui/dialog/working_dir_picker_test.go @@ -0,0 +1,40 @@ +package dialog + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestWorkingDirPickerRootHasNoParentDirEntry(t *testing.T) { + t.Parallel() + + // Get the root of the current working directory + cwd, err := os.Getwd() + require.NoError(t, err) + root := filepath.VolumeName(cwd) + string(filepath.Separator) + + d := NewWorkingDirPickerDialog(t.Context(), nil, nil, nil, root).(*workingDirPickerDialog) + + // Ensure there's no ".." entry in the browse entries + for _, e := range d.browseEntries { + if e.name == ".." { + t.Errorf("root directory should not have a parent dir entry, but got '..'") + } + } +} + +func TestWorkingDirPickerEmptyFallback(t *testing.T) { + t.Parallel() + + // Pass an empty string for the initial directory. + // NewWorkingDirPickerDialog should fall back to os.Getwd(). + d := NewWorkingDirPickerDialog(t.Context(), nil, nil, nil, "").(*workingDirPickerDialog) + + cwd, err := os.Getwd() + require.NoError(t, err) + + require.Equal(t, cwd, d.currentDir, "empty initial directory should fall back to current working directory") +}