diff --git a/docs/features/cli/index.md b/docs/features/cli/index.md index 70df4d6c48..3e1c3aed6a 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -56,7 +56,7 @@ $ docker agent run [config] [message...] [flags] | `-w, --worktree [name]` | Run the agent in a fresh git worktree of the working directory, isolating its changes from your checkout. Optionally name it (`--worktree=my-feature`); otherwise a name is generated. Requires the working directory to be inside a git repository. Every tool (the shell included) runs inside the worktree. Combine with `--working-dir` to branch from another repository, and with `--session` to resume into the same worktree later. Cannot be combined with `--remote` or `--sandbox`. When the session ends, a clean worktree is removed automatically; one with work prompts to keep or remove (never in `--exec`). | | `--worktree-base ` | Branch the `--worktree` from `` (a branch, tag, commit, or remote-tracking ref like `origin/main`) instead of the current `HEAD`. A remote-tracking ref is fetched first so the worktree starts from the latest remote state. Requires `--worktree`; cannot be combined with `--worktree-pr`, `--remote`, or `--sandbox`. | | `--worktree-pr ` | Run the agent in a git worktree checked out on an existing GitHub pull request (PR number, `#123`, or PR URL). Continues the PR's branch so commits push back to it. Requires the [GitHub CLI](https://cli.github.com/) (`gh`). Cannot be combined with `--worktree`, `--remote`, or `--sandbox`. | -| `--working-dir ` | Set the working directory for the session (applies to tools and relative paths) | +| `--working-dir ` | Set the working directory for the session (applies to tools and relative paths). In the full TUI, an explicitly supplied path also becomes the default directory for new sessions (`/new`, Ctrl+T, the `+` buttons); `/new ` overrides it for one session | | `--env-from-file ` | Load environment variables from file (repeatable) | | `--flavor ` | Enable a config flavor, a YAML patch defined under the config's `flavors` section (repeatable, applied in order). See [Flavors](../../configuration/flavors/index.md). | | `--code-mode-tools` | Provide a single tool to call other tools via JavaScript (forces code-mode tools globally) | @@ -813,7 +813,7 @@ These flags are accepted by every command that loads an agent (`run`, `run --exe | Flag | Description | | ------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | -| `--working-dir ` | Set the working directory for the session (applies to tools and relative paths). | +| `--working-dir ` | Set the working directory for the session (applies to tools and relative paths). In the full TUI, an explicitly supplied path also becomes the default directory for new sessions (`/new`, Ctrl+T, the `+` buttons); `/new ` overrides it for one session. | | `--env-from-file ` | Load environment variables from file (repeatable). | | `--flavor ` | Enable a config flavor, a YAML patch defined under the config's `flavors` section (repeatable, applied in order). See [Flavors](../../configuration/flavors/index.md). | | `--code-mode-tools` | Provide a single tool to call other tools via JavaScript (forces code-mode tools globally). | diff --git a/docs/features/tui/index.md b/docs/features/tui/index.md index bf000e8430..e28846c5a7 100644 --- a/docs/features/tui/index.md +++ b/docs/features/tui/index.md @@ -69,7 +69,7 @@ Type `/` during a session to see available commands, or press Ctrl+) and wins over + // any configured default; empty keeps the generic new-session behavior. + NewSessionMsg struct{ WorkingDir string } // ClearSessionMsg resets the current tab and starts a new session // in the same working directory. diff --git a/pkg/tui/spawn_default_dir_test.go b/pkg/tui/spawn_default_dir_test.go index 44d35acaf8..19cdd54b3d 100644 --- a/pkg/tui/spawn_default_dir_test.go +++ b/pkg/tui/spawn_default_dir_test.go @@ -2,6 +2,8 @@ package tui import ( "context" + "os" + "path/filepath" "testing" tea "charm.land/bubbletea/v2" @@ -12,6 +14,7 @@ import ( "github.com/docker/docker-agent/pkg/session" "github.com/docker/docker-agent/pkg/tui/animation" "github.com/docker/docker-agent/pkg/tui/commands" + "github.com/docker/docker-agent/pkg/tui/components/notification" "github.com/docker/docker-agent/pkg/tui/components/spinner" "github.com/docker/docker-agent/pkg/tui/components/statusbar" "github.com/docker/docker-agent/pkg/tui/components/tabbar" @@ -114,3 +117,98 @@ func TestNewSessionMsg_UsesConfiguredDefaultDir(t *testing.T) { assert.Equal(t, 2, m.supervisor.Count(), "the new session must be added instead of opening the picker") } + +// /new must win over the configured default (#4046). Before the fix +// the /new command dropped its argument, so the default always won. +func TestNewSessionMsg_ExplicitDirOverridesDefault(t *testing.T) { + t.Parallel() + + spy := &spySpawner{} + m := newSpawnTestModel(t, spy, WithDefaultWorkingDir("/default/dir")) + dir := t.TempDir() + + _, _ = m.Update(messages.NewSessionMsg{WorkingDir: dir}) + + assert.Equal(t, []string{dir}, spy.dirs, + "an explicit /new directory must win over the configured default") + assert.Equal(t, 2, m.supervisor.Count(), + "the new session must be added instead of opening the picker") +} + +// A relative /new argument resolves against the active session's working +// directory, not the process CWD. +func TestNewSessionMsg_RelativeDirResolvesFromActiveSession(t *testing.T) { + t.Parallel() + + base := t.TempDir() + sub := filepath.Join(base, "sub") + require.NoError(t, os.Mkdir(sub, 0o755)) + + spy := &spySpawner{} + m := newSpawnTestModel(t, spy) + // Point the active session at base; the helper registers it under + // "/initial", which does not exist on disk. + m.supervisor.GetRunner(m.supervisor.ActiveID()).WorkingDir = base + + _, _ = m.Update(messages.NewSessionMsg{WorkingDir: "sub"}) + + assert.Equal(t, []string{sub}, spy.dirs, + "a relative directory must resolve from the active session's working dir") +} + +// ~ and environment variables in an explicit /new directory are expanded via +// path.ExpandPath. HOME is overridden (ExpandHomeDir prefers it over the OS +// account lookup) so the test stays hermetic; t.Setenv forbids t.Parallel. +func TestNewSessionMsg_ExpandsTildeAndEnv(t *testing.T) { + base := t.TempDir() + sub := filepath.Join(base, "sub") + require.NoError(t, os.Mkdir(sub, 0o755)) + t.Setenv("HOME", base) + t.Setenv("CAGENT_TEST_NEW_DIR", base) + + spy := &spySpawner{} + m := newSpawnTestModel(t, spy) + + _, _ = m.Update(messages.NewSessionMsg{WorkingDir: "~/sub"}) + _, _ = m.Update(messages.NewSessionMsg{WorkingDir: "$CAGENT_TEST_NEW_DIR/sub"}) + + assert.Equal(t, []string{sub, sub}, spy.dirs, + "~ and environment variables must be expanded") +} + +// A /new directory that does not exist must not reach the spawner; the user +// gets an error notification instead. +func TestNewSessionMsg_MissingDirErrorsWithoutSpawning(t *testing.T) { + t.Parallel() + + spy := &spySpawner{} + m := newSpawnTestModel(t, spy, WithDefaultWorkingDir("/default/dir")) + missing := filepath.Join(t.TempDir(), "missing") + + _, cmd := m.Update(messages.NewSessionMsg{WorkingDir: missing}) + + assert.Empty(t, spy.dirs, "the spawner must not run for a missing directory") + assert.Equal(t, 1, m.supervisor.Count(), "no session must be added") + note, ok := firstOfType[notification.ShowMsg](collectMsgs(cmd)) + require.True(t, ok, "an error notification must be shown") + assert.Equal(t, notification.TypeError, note.Type) + assert.Contains(t, note.Text, missing) +} + +// A /new argument pointing at a file must not reach the spawner either. +func TestNewSessionMsg_FileArgErrorsWithoutSpawning(t *testing.T) { + t.Parallel() + + spy := &spySpawner{} + m := newSpawnTestModel(t, spy) + file := filepath.Join(t.TempDir(), "file.txt") + require.NoError(t, os.WriteFile(file, []byte("x"), 0o644)) + + _, cmd := m.Update(messages.NewSessionMsg{WorkingDir: file}) + + assert.Empty(t, spy.dirs, "the spawner must not run for a non-directory path") + note, ok := firstOfType[notification.ShowMsg](collectMsgs(cmd)) + require.True(t, ok, "an error notification must be shown") + assert.Equal(t, notification.TypeError, note.Type) + assert.Contains(t, note.Text, "is not a directory") +} diff --git a/pkg/tui/tui.go b/pkg/tui/tui.go index aa59e5a0b8..fabfde0004 100644 --- a/pkg/tui/tui.go +++ b/pkg/tui/tui.go @@ -6,6 +6,7 @@ import ( "fmt" "log/slog" "os" + "path/filepath" "strings" "sync" "time" @@ -20,6 +21,7 @@ import ( "github.com/docker/docker-agent/pkg/app" "github.com/docker/docker-agent/pkg/audio/transcribe" "github.com/docker/docker-agent/pkg/history" + "github.com/docker/docker-agent/pkg/path" "github.com/docker/docker-agent/pkg/plans" "github.com/docker/docker-agent/pkg/runtime" "github.com/docker/docker-agent/pkg/session" @@ -1202,7 +1204,7 @@ func (m *appModel) update(msg tea.Msg) (tea.Model, tea.Cmd) { case messages.NewSessionMsg: // /new spawns a new tab when a session spawner is configured. - return m.handleSpawnSession("") + return m.handleNewSession(msg) case messages.ClearSessionMsg: // /clear resets the current tab with a fresh session in the same working dir. @@ -1819,6 +1821,54 @@ func (m *appModel) handleClearSession() (tea.Model, tea.Cmd) { ) } +// handleNewSession handles /new. Without a directory argument it keeps the +// generic behavior (configured default or picker); with one it resolves and +// validates the requested directory before spawning there, so an explicit +// argument wins over the configured default. +func (m *appModel) handleNewSession(msg messages.NewSessionMsg) (tea.Model, tea.Cmd) { + requested := strings.TrimSpace(msg.WorkingDir) + if requested == "" { + return m.handleSpawnSession("") + } + workingDir, err := m.resolveNewSessionDir(requested) + if err != nil { + return m, notification.ErrorCmd("Cannot start a new session: " + err.Error()) + } + return m.handleSpawnSession(workingDir) +} + +// resolveNewSessionDir turns a user-supplied /new argument into an absolute, +// existing directory. ~ and environment variables are expanded; a relative +// path resolves against the active session's working directory rather than +// the process CWD. +func (m *appModel) resolveNewSessionDir(requested string) (string, error) { + dir := path.ExpandPath(requested) + if dir == "" { + return "", fmt.Errorf("%q expands to an empty path", requested) + } + if !filepath.IsAbs(dir) { + var base string + if runner := m.supervisor.GetRunner(m.supervisor.ActiveID()); runner != nil { + base = runner.WorkingDir + } + dir = filepath.Join(base, dir) + } + abs, err := filepath.Abs(dir) + if err != nil { + return "", err + } + info, err := os.Stat(abs) + switch { + case os.IsNotExist(err): + return "", fmt.Errorf("%s does not exist", abs) + case err != nil: + return "", err + case !info.IsDir(): + return "", fmt.Errorf("%s is not a directory", abs) + } + return abs, nil +} + // handleSpawnSession spawns a new session. func (m *appModel) handleSpawnSession(workingDir string) (tea.Model, tea.Cmd) { // A generic request (no directory) inherits the explicit --working-dir