Skip to content

Commit 119030d

Browse files
mafredrimafredri
authored
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.
1 parent 483adc5 commit 119030d

4 files changed

Lines changed: 151 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: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"net/http"
99
"net/http/httptest"
10+
"os"
1011
"runtime"
1112
"strings"
1213
"testing"
@@ -97,18 +98,25 @@ func postSignal(t *testing.T, handler http.Handler, id string, req workspacesdk.
9798
// execer, returning the handler and API.
9899
func newTestAPI(t *testing.T) http.Handler {
99100
t.Helper()
100-
return newTestAPIWithUpdateEnv(t, nil)
101+
return newTestAPIWithOptions(t, nil, nil)
101102
}
102103

103104
// newTestAPIWithUpdateEnv creates a new API with an optional
104105
// updateEnv hook for testing environment injection.
105106
func newTestAPIWithUpdateEnv(t *testing.T, updateEnv func([]string) ([]string, error)) http.Handler {
106107
t.Helper()
108+
return newTestAPIWithOptions(t, updateEnv, nil)
109+
}
110+
111+
// newTestAPIWithOptions creates a new API with optional
112+
// updateEnv and workingDir hooks.
113+
func newTestAPIWithOptions(t *testing.T, updateEnv func([]string) ([]string, error), workingDir func() string) http.Handler {
114+
t.Helper()
107115

108116
logger := slogtest.Make(t, &slogtest.Options{
109117
IgnoreErrors: true,
110118
}).Leveled(slog.LevelDebug)
111-
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil)
119+
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil, workingDir)
112120
t.Cleanup(func() {
113121
_ = api.Close()
114122
})
@@ -253,6 +261,100 @@ func TestStartProcess(t *testing.T) {
253261
require.Contains(t, resp.Output, "marker.txt")
254262
})
255263

264+
t.Run("DefaultWorkDirIsHome", func(t *testing.T) {
265+
t.Parallel()
266+
267+
// No working directory closure, so the process
268+
// should fall back to $HOME. We verify through
269+
// the process list API which reports the resolved
270+
// working directory using native OS paths,
271+
// avoiding shell path format mismatches on
272+
// Windows (Git Bash returns POSIX paths).
273+
handler := newTestAPI(t)
274+
275+
homeDir, err := os.UserHomeDir()
276+
require.NoError(t, err)
277+
278+
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
279+
Command: "echo ok",
280+
})
281+
282+
resp := waitForExit(t, handler, id)
283+
require.NotNil(t, resp.ExitCode)
284+
require.Equal(t, 0, *resp.ExitCode)
285+
286+
w := getList(t, handler)
287+
require.Equal(t, http.StatusOK, w.Code)
288+
var listResp workspacesdk.ListProcessesResponse
289+
require.NoError(t, json.NewDecoder(w.Body).Decode(&listResp))
290+
var proc *workspacesdk.ProcessInfo
291+
for i := range listResp.Processes {
292+
if listResp.Processes[i].ID == id {
293+
proc = &listResp.Processes[i]
294+
break
295+
}
296+
}
297+
require.NotNil(t, proc, "process not found in list")
298+
require.Equal(t, homeDir, proc.WorkDir)
299+
})
300+
301+
t.Run("DefaultWorkDirFromClosure", func(t *testing.T) {
302+
t.Parallel()
303+
304+
// The closure provides a valid directory, so the
305+
// process should start there. Use the marker file
306+
// pattern to avoid path format mismatches on
307+
// Windows.
308+
tmpDir := t.TempDir()
309+
handler := newTestAPIWithOptions(t, nil, func() string {
310+
return tmpDir
311+
})
312+
313+
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
314+
Command: "touch marker.txt && ls marker.txt",
315+
})
316+
317+
resp := waitForExit(t, handler, id)
318+
require.NotNil(t, resp.ExitCode)
319+
require.Equal(t, 0, *resp.ExitCode)
320+
require.Contains(t, resp.Output, "marker.txt")
321+
})
322+
323+
t.Run("DefaultWorkDirClosureNonExistentFallsBackToHome", func(t *testing.T) {
324+
t.Parallel()
325+
326+
// The closure returns a path that doesn't exist,
327+
// so the process should fall back to $HOME.
328+
handler := newTestAPIWithOptions(t, nil, func() string {
329+
return "/tmp/nonexistent-dir-" + fmt.Sprintf("%d", time.Now().UnixNano())
330+
})
331+
332+
homeDir, err := os.UserHomeDir()
333+
require.NoError(t, err)
334+
335+
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
336+
Command: "echo ok",
337+
})
338+
339+
resp := waitForExit(t, handler, id)
340+
require.NotNil(t, resp.ExitCode)
341+
require.Equal(t, 0, *resp.ExitCode)
342+
343+
w := getList(t, handler)
344+
require.Equal(t, http.StatusOK, w.Code)
345+
var listResp workspacesdk.ListProcessesResponse
346+
require.NoError(t, json.NewDecoder(w.Body).Decode(&listResp))
347+
var proc *workspacesdk.ProcessInfo
348+
for i := range listResp.Processes {
349+
if listResp.Processes[i].ID == id {
350+
proc = &listResp.Processes[i]
351+
break
352+
}
353+
}
354+
require.NotNil(t, proc, "process not found in list")
355+
require.Equal(t, homeDir, proc.WorkDir)
356+
})
357+
256358
t.Run("CustomEnv", func(t *testing.T) {
257359
t.Parallel()
258360

@@ -781,7 +883,7 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T)
781883
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
782884
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, func(current []string) ([]string, error) {
783885
return current, nil
784-
}, pathStore)
886+
}, pathStore, nil)
785887
defer api.Close()
786888

787889
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)