Skip to content

Commit 71cff44

Browse files
mafredrimafredri
committed
fix(agent): default process working directory to agent dir or $HOME
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.
1 parent b83b93e commit 71cff44

4 files changed

Lines changed: 141 additions & 23 deletions

File tree

‎agent/agent.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,12 @@ func (a *agent) init() {
385385

386386
pathStore := agentgit.NewPathStore()
387387
a.filesAPI = agentfiles.NewAPI(a.logger.Named("files"), a.filesystem, pathStore)
388-
a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.updateCommandEnv, pathStore)
388+
a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.updateCommandEnv, pathStore, func() string {
389+
if m := a.manifest.Load(); m != nil {
390+
return m.Directory
391+
}
392+
return ""
393+
})
389394
gitOpts := append([]agentgit.Option{agentgit.WithClock(a.clock)}, a.gitAPIOptions...)
390395
a.gitAPI = agentgit.NewAPI(a.logger.Named("git"), pathStore, gitOpts...)
391396
desktop := agentdesktop.NewPortableDesktop(

‎agent/agentproc/api.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ type API struct {
2626
}
2727

2828
// NewAPI creates a new process API handler.
29-
func NewAPI(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), pathStore *agentgit.PathStore) *API {
29+
func NewAPI(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), pathStore *agentgit.PathStore, workingDir func() string) *API {
3030
return &API{
3131
logger: logger,
32-
manager: newManager(logger, execer, updateEnv),
32+
manager: newManager(logger, execer, updateEnv, workingDir),
3333
pathStore: pathStore,
3434
}
3535
}

‎agent/agentproc/api_test.go‎

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"fmt"
88
"net/http"
99
"net/http/httptest"
10+
"os"
11+
"path/filepath"
1012
"runtime"
1113
"strings"
1214
"testing"
@@ -97,18 +99,25 @@ func postSignal(t *testing.T, handler http.Handler, id string, req workspacesdk.
9799
// execer, returning the handler and API.
98100
func newTestAPI(t *testing.T) http.Handler {
99101
t.Helper()
100-
return newTestAPIWithUpdateEnv(t, nil)
102+
return newTestAPIWithOptions(t, nil, nil)
101103
}
102104

103105
// newTestAPIWithUpdateEnv creates a new API with an optional
104106
// updateEnv hook for testing environment injection.
105107
func newTestAPIWithUpdateEnv(t *testing.T, updateEnv func([]string) ([]string, error)) http.Handler {
106108
t.Helper()
109+
return newTestAPIWithOptions(t, updateEnv, nil)
110+
}
111+
112+
// newTestAPIWithOptions creates a new API with optional
113+
// updateEnv and workingDir hooks.
114+
func newTestAPIWithOptions(t *testing.T, updateEnv func([]string) ([]string, error), workingDir func() string) http.Handler {
115+
t.Helper()
107116

108117
logger := slogtest.Make(t, &slogtest.Options{
109118
IgnoreErrors: true,
110119
}).Leveled(slog.LevelDebug)
111-
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil)
120+
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil, workingDir)
112121
t.Cleanup(func() {
113122
_ = api.Close()
114123
})
@@ -253,6 +262,89 @@ func TestStartProcess(t *testing.T) {
253262
require.Contains(t, resp.Output, "marker.txt")
254263
})
255264

265+
t.Run("DefaultWorkDirIsHome", func(t *testing.T) {
266+
t.Parallel()
267+
268+
// No working directory closure, so the process
269+
// should fall back to $HOME. We verify via a
270+
// marker file instead of comparing pwd output,
271+
// which resolves symlinks and breaks on macOS
272+
// where /tmp -> /private/tmp.
273+
handler := newTestAPI(t)
274+
275+
homeDir, err := os.UserHomeDir()
276+
require.NoError(t, err)
277+
278+
markerName := fmt.Sprintf("marker-%d.txt", time.Now().UnixNano())
279+
markerPath := filepath.Join(homeDir, markerName)
280+
require.NoError(t, os.WriteFile(markerPath, nil, 0o600))
281+
t.Cleanup(func() { os.Remove(markerPath) })
282+
283+
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
284+
Command: fmt.Sprintf("ls %s", markerName),
285+
})
286+
287+
resp := waitForExit(t, handler, id)
288+
require.NotNil(t, resp.ExitCode)
289+
require.Equal(t, 0, *resp.ExitCode)
290+
require.Contains(t, resp.Output, markerName)
291+
})
292+
293+
t.Run("DefaultWorkDirFromClosure", func(t *testing.T) {
294+
t.Parallel()
295+
296+
// The closure provides a valid directory, so the
297+
// process should start there. We verify via a
298+
// marker file instead of comparing pwd output,
299+
// which resolves symlinks and breaks on macOS
300+
// where /tmp -> /private/tmp.
301+
tmpDir := t.TempDir()
302+
handler := newTestAPIWithOptions(t, nil, func() string {
303+
return tmpDir
304+
})
305+
306+
markerName := fmt.Sprintf("marker-%d.txt", time.Now().UnixNano())
307+
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, markerName), nil, 0o600))
308+
309+
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
310+
Command: fmt.Sprintf("ls %s", markerName),
311+
})
312+
313+
resp := waitForExit(t, handler, id)
314+
require.NotNil(t, resp.ExitCode)
315+
require.Equal(t, 0, *resp.ExitCode)
316+
require.Contains(t, resp.Output, markerName)
317+
})
318+
319+
t.Run("DefaultWorkDirClosureNonExistentFallsBackToHome", func(t *testing.T) {
320+
t.Parallel()
321+
322+
// The closure returns a path that doesn't exist,
323+
// so the process should fall back to $HOME. We
324+
// verify via a marker file instead of comparing
325+
// pwd output, which resolves symlinks.
326+
handler := newTestAPIWithOptions(t, nil, func() string {
327+
return "/tmp/nonexistent-dir-" + fmt.Sprintf("%d", time.Now().UnixNano())
328+
})
329+
330+
homeDir, err := os.UserHomeDir()
331+
require.NoError(t, err)
332+
333+
markerName := fmt.Sprintf("marker-%d.txt", time.Now().UnixNano())
334+
markerPath := filepath.Join(homeDir, markerName)
335+
require.NoError(t, os.WriteFile(markerPath, nil, 0o600))
336+
t.Cleanup(func() { os.Remove(markerPath) })
337+
338+
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
339+
Command: fmt.Sprintf("ls %s", markerName),
340+
})
341+
342+
resp := waitForExit(t, handler, id)
343+
require.NotNil(t, resp.ExitCode)
344+
require.Equal(t, 0, *resp.ExitCode)
345+
require.Contains(t, resp.Output, markerName)
346+
})
347+
256348
t.Run("CustomEnv", func(t *testing.T) {
257349
t.Parallel()
258350

@@ -781,7 +873,7 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T)
781873
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
782874
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, func(current []string) ([]string, error) {
783875
return current, nil
784-
}, pathStore)
876+
}, pathStore, nil)
785877
defer api.Close()
786878

787879
routes := api.Routes()

‎agent/agentproc/process.go‎

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -70,23 +70,25 @@ func (p *process) output() (string, *workspacesdk.ProcessTruncation) {
7070

7171
// manager tracks processes spawned by the agent.
7272
type manager struct {
73-
mu sync.Mutex
74-
logger slog.Logger
75-
execer agentexec.Execer
76-
clock quartz.Clock
77-
procs map[string]*process
78-
closed bool
79-
updateEnv func(current []string) (updated []string, err error)
73+
mu sync.Mutex
74+
logger slog.Logger
75+
execer agentexec.Execer
76+
clock quartz.Clock
77+
procs map[string]*process
78+
closed bool
79+
updateEnv func(current []string) (updated []string, err error)
80+
workingDir func() string
8081
}
8182

8283
// newManager creates a new process manager.
83-
func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error)) *manager {
84+
func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager {
8485
return &manager{
85-
logger: logger,
86-
execer: execer,
87-
clock: quartz.NewReal(),
88-
procs: make(map[string]*process),
89-
updateEnv: updateEnv,
86+
logger: logger,
87+
execer: execer,
88+
clock: quartz.NewReal(),
89+
procs: make(map[string]*process),
90+
updateEnv: updateEnv,
91+
workingDir: workingDir,
9092
}
9193
}
9294

@@ -109,9 +111,7 @@ func (m *manager) start(req workspacesdk.StartProcessRequest, chatID string) (*p
109111
// the process is not tied to any HTTP request.
110112
ctx, cancel := context.WithCancel(context.Background())
111113
cmd := m.execer.CommandContext(ctx, "sh", "-c", req.Command)
112-
if req.WorkDir != "" {
113-
cmd.Dir = req.WorkDir
114-
}
114+
cmd.Dir = m.resolveWorkDir(req.WorkDir)
115115
cmd.Stdin = nil
116116
cmd.SysProcAttr = procSysProcAttr()
117117

@@ -158,7 +158,7 @@ func (m *manager) start(req workspacesdk.StartProcessRequest, chatID string) (*p
158158
proc := &process{
159159
id: id,
160160
command: req.Command,
161-
workDir: req.WorkDir,
161+
workDir: cmd.Dir,
162162
background: req.Background,
163163
chatID: chatID,
164164
cmd: cmd,
@@ -319,3 +319,24 @@ func (m *manager) Close() error {
319319

320320
return nil
321321
}
322+
323+
// resolveWorkDir returns the directory a process should start in.
324+
// Priority: explicit request dir > agent configured dir > $HOME.
325+
// Falls through when a candidate is empty or does not exist on
326+
// disk, matching the behavior of SSH sessions.
327+
func (m *manager) resolveWorkDir(requested string) string {
328+
if requested != "" {
329+
return requested
330+
}
331+
if m.workingDir != nil {
332+
if dir := m.workingDir(); dir != "" {
333+
if info, err := os.Stat(dir); err == nil && info.IsDir() {
334+
return dir
335+
}
336+
}
337+
}
338+
if home, err := os.UserHomeDir(); err == nil {
339+
return home
340+
}
341+
return ""
342+
}

0 commit comments

Comments
 (0)