From 119030d79552ca674d08a56898ba2e7cedaf6465 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 18 Mar 2026 18:46:26 +0200 Subject: [PATCH] fix(agent): default process working directory to agent dir or $HOME (#23224) Processes started via the agent process API inherited the agent's own working directory (/tmp/coder.xxx) when no WorkDir was specified. SSH sessions already use a fallback chain: configured agent directory > $HOME. This wires the same manifest directory closure into the process manager so the priority is now: explicit req.WorkDir > agent configured dir > $HOME The resolved directory is recorded on the process struct so ProcessInfo.WorkDir and pathStore notifications reflect where the process actually ran. --- agent/agent.go | 7 ++- agent/agentproc/api.go | 4 +- agent/agentproc/api_test.go | 108 +++++++++++++++++++++++++++++++++++- agent/agentproc/process.go | 55 ++++++++++++------ 4 files changed, 151 insertions(+), 23 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 1787619041..7f17a5f762 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -385,7 +385,12 @@ func (a *agent) init() { pathStore := agentgit.NewPathStore() a.filesAPI = agentfiles.NewAPI(a.logger.Named("files"), a.filesystem, pathStore) - a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.updateCommandEnv, pathStore) + a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.updateCommandEnv, pathStore, func() string { + if m := a.manifest.Load(); m != nil { + return m.Directory + } + return "" + }) gitOpts := append([]agentgit.Option{agentgit.WithClock(a.clock)}, a.gitAPIOptions...) a.gitAPI = agentgit.NewAPI(a.logger.Named("git"), pathStore, gitOpts...) desktop := agentdesktop.NewPortableDesktop( diff --git a/agent/agentproc/api.go b/agent/agentproc/api.go index 0116114039..0db5bb0ac8 100644 --- a/agent/agentproc/api.go +++ b/agent/agentproc/api.go @@ -26,10 +26,10 @@ type API struct { } // NewAPI creates a new process API handler. -func NewAPI(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), pathStore *agentgit.PathStore) *API { +func NewAPI(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), pathStore *agentgit.PathStore, workingDir func() string) *API { return &API{ logger: logger, - manager: newManager(logger, execer, updateEnv), + manager: newManager(logger, execer, updateEnv, workingDir), pathStore: pathStore, } } diff --git a/agent/agentproc/api_test.go b/agent/agentproc/api_test.go index 0d2677f5be..7e7640de04 100644 --- a/agent/agentproc/api_test.go +++ b/agent/agentproc/api_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" "runtime" "strings" "testing" @@ -97,18 +98,25 @@ func postSignal(t *testing.T, handler http.Handler, id string, req workspacesdk. // execer, returning the handler and API. func newTestAPI(t *testing.T) http.Handler { t.Helper() - return newTestAPIWithUpdateEnv(t, nil) + return newTestAPIWithOptions(t, nil, nil) } // newTestAPIWithUpdateEnv creates a new API with an optional // updateEnv hook for testing environment injection. func newTestAPIWithUpdateEnv(t *testing.T, updateEnv func([]string) ([]string, error)) http.Handler { t.Helper() + return newTestAPIWithOptions(t, updateEnv, nil) +} + +// newTestAPIWithOptions creates a new API with optional +// updateEnv and workingDir hooks. +func newTestAPIWithOptions(t *testing.T, updateEnv func([]string) ([]string, error), workingDir func() string) http.Handler { + t.Helper() logger := slogtest.Make(t, &slogtest.Options{ IgnoreErrors: true, }).Leveled(slog.LevelDebug) - api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil) + api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil, workingDir) t.Cleanup(func() { _ = api.Close() }) @@ -253,6 +261,100 @@ func TestStartProcess(t *testing.T) { require.Contains(t, resp.Output, "marker.txt") }) + t.Run("DefaultWorkDirIsHome", func(t *testing.T) { + t.Parallel() + + // No working directory closure, so the process + // should fall back to $HOME. We verify through + // the process list API which reports the resolved + // working directory using native OS paths, + // avoiding shell path format mismatches on + // Windows (Git Bash returns POSIX paths). + handler := newTestAPI(t) + + homeDir, err := os.UserHomeDir() + require.NoError(t, err) + + id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{ + Command: "echo ok", + }) + + resp := waitForExit(t, handler, id) + require.NotNil(t, resp.ExitCode) + require.Equal(t, 0, *resp.ExitCode) + + w := getList(t, handler) + require.Equal(t, http.StatusOK, w.Code) + var listResp workspacesdk.ListProcessesResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&listResp)) + var proc *workspacesdk.ProcessInfo + for i := range listResp.Processes { + if listResp.Processes[i].ID == id { + proc = &listResp.Processes[i] + break + } + } + require.NotNil(t, proc, "process not found in list") + require.Equal(t, homeDir, proc.WorkDir) + }) + + t.Run("DefaultWorkDirFromClosure", func(t *testing.T) { + t.Parallel() + + // The closure provides a valid directory, so the + // process should start there. Use the marker file + // pattern to avoid path format mismatches on + // Windows. + tmpDir := t.TempDir() + handler := newTestAPIWithOptions(t, nil, func() string { + return tmpDir + }) + + id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{ + Command: "touch marker.txt && ls marker.txt", + }) + + resp := waitForExit(t, handler, id) + require.NotNil(t, resp.ExitCode) + require.Equal(t, 0, *resp.ExitCode) + require.Contains(t, resp.Output, "marker.txt") + }) + + t.Run("DefaultWorkDirClosureNonExistentFallsBackToHome", func(t *testing.T) { + t.Parallel() + + // The closure returns a path that doesn't exist, + // so the process should fall back to $HOME. + handler := newTestAPIWithOptions(t, nil, func() string { + return "/tmp/nonexistent-dir-" + fmt.Sprintf("%d", time.Now().UnixNano()) + }) + + homeDir, err := os.UserHomeDir() + require.NoError(t, err) + + id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{ + Command: "echo ok", + }) + + resp := waitForExit(t, handler, id) + require.NotNil(t, resp.ExitCode) + require.Equal(t, 0, *resp.ExitCode) + + w := getList(t, handler) + require.Equal(t, http.StatusOK, w.Code) + var listResp workspacesdk.ListProcessesResponse + require.NoError(t, json.NewDecoder(w.Body).Decode(&listResp)) + var proc *workspacesdk.ProcessInfo + for i := range listResp.Processes { + if listResp.Processes[i].ID == id { + proc = &listResp.Processes[i] + break + } + } + require.NotNil(t, proc, "process not found in list") + require.Equal(t, homeDir, proc.WorkDir) + }) + t.Run("CustomEnv", func(t *testing.T) { t.Parallel() @@ -781,7 +883,7 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T) logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) api := agentproc.NewAPI(logger, agentexec.DefaultExecer, func(current []string) ([]string, error) { return current, nil - }, pathStore) + }, pathStore, nil) defer api.Close() routes := api.Routes() diff --git a/agent/agentproc/process.go b/agent/agentproc/process.go index 57cbb0fbfb..ed1279409c 100644 --- a/agent/agentproc/process.go +++ b/agent/agentproc/process.go @@ -70,23 +70,25 @@ func (p *process) output() (string, *workspacesdk.ProcessTruncation) { // manager tracks processes spawned by the agent. type manager struct { - mu sync.Mutex - logger slog.Logger - execer agentexec.Execer - clock quartz.Clock - procs map[string]*process - closed bool - updateEnv func(current []string) (updated []string, err error) + mu sync.Mutex + logger slog.Logger + execer agentexec.Execer + clock quartz.Clock + procs map[string]*process + closed bool + updateEnv func(current []string) (updated []string, err error) + workingDir func() string } // newManager creates a new process manager. -func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error)) *manager { +func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager { return &manager{ - logger: logger, - execer: execer, - clock: quartz.NewReal(), - procs: make(map[string]*process), - updateEnv: updateEnv, + logger: logger, + execer: execer, + clock: quartz.NewReal(), + procs: make(map[string]*process), + updateEnv: updateEnv, + workingDir: workingDir, } } @@ -109,9 +111,7 @@ func (m *manager) start(req workspacesdk.StartProcessRequest, chatID string) (*p // the process is not tied to any HTTP request. ctx, cancel := context.WithCancel(context.Background()) cmd := m.execer.CommandContext(ctx, "sh", "-c", req.Command) - if req.WorkDir != "" { - cmd.Dir = req.WorkDir - } + cmd.Dir = m.resolveWorkDir(req.WorkDir) cmd.Stdin = nil cmd.SysProcAttr = procSysProcAttr() @@ -158,7 +158,7 @@ func (m *manager) start(req workspacesdk.StartProcessRequest, chatID string) (*p proc := &process{ id: id, command: req.Command, - workDir: req.WorkDir, + workDir: cmd.Dir, background: req.Background, chatID: chatID, cmd: cmd, @@ -319,3 +319,24 @@ func (m *manager) Close() error { return nil } + +// resolveWorkDir returns the directory a process should start in. +// Priority: explicit request dir > agent configured dir > $HOME. +// Falls through when a candidate is empty or does not exist on +// disk, matching the behavior of SSH sessions. +func (m *manager) resolveWorkDir(requested string) string { + if requested != "" { + return requested + } + if m.workingDir != nil { + if dir := m.workingDir(); dir != "" { + if info, err := os.Stat(dir); err == nil && info.IsDir() { + return dir + } + } + } + if home, err := os.UserHomeDir(); err == nil { + return home + } + return "" +}