mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
fix(agent/agentproc): track the default working directory for chat processes (#30674)
A process started through the agent's process API without a `workdir` runs in the agent's default directory, but only an explicit `workdir` was recorded in the chat's path store. Repositories touched that way were never watched by the git panel. The path store now records `proc.workDir`, the directory the process actually ran in, whether requested or defaulted. ### Before https://github.com/user-attachments/assets/b98c290a-0c77-4ce4-bb00-eb4088d2067e ### After https://github.com/user-attachments/assets/0383aa1a-eced-4338-97af-3c54501cdedb <details> <summary>Decision log</summary> The git panel's refresh button should show everything a page reload would. Investigation found five ways it didn't, each fixed in its own PR on the current `changes` protocol: 1. `execute` without a workdir didn't record the directory the process ran in (`agent/agentproc`). 2. Refresh only rescanned repos already being watched, so a repo created or recreated after its path was recorded stayed hidden (`agent/agentgit`). A connection with no repos also got no first message. 3. After a git watch reconnect, the new full state was merged into the previous connection's map, so deleted repos stayed visible (`useGitWatcher`). 4. A chat watch reconnect didn't refetch the open chat, so missed `diff_status_change` events were lost (`AgentsPageLayout`). 5. Refresh didn't refetch the chat, whose diff statuses back the PR tabs, and was disabled until git context existed (`GitPanel`, `AgentChatPageView`). Considered and deferred: a snapshot protocol with diffs fetched by hash, which would keep large multi-repo diffs under the 4 MiB proxy read limit, and making scans faster at scale. Neither is needed to fix refresh, and together they made a single ~5900-line change that couldn't be reviewed. The refresh spinner still runs for a fixed time, since tying it to the response needs a request ID in the protocol. </details> --- _Generated by Coder Agents on behalf of @DanielleMaywood._
This commit is contained in:
@@ -106,14 +106,15 @@ func (api *API) handleStartProcess(rw http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// Notify git watchers after the process finishes so that
|
||||
// file changes made by the command are visible in the scan.
|
||||
// If a workdir is provided, track it as a path as well.
|
||||
// The directory the process ran in counts as touched, whether
|
||||
// it was requested or defaulted.
|
||||
if api.pathStore != nil {
|
||||
if chatContext, ok := agentchat.FromContext(ctx); ok {
|
||||
allIDs := append([]uuid.UUID{chatContext.ID}, chatContext.AncestorIDs...)
|
||||
go func() {
|
||||
<-proc.done
|
||||
if req.WorkDir != "" {
|
||||
api.pathStore.AddPaths(allIDs, []string{req.WorkDir})
|
||||
if proc.workDir != "" {
|
||||
api.pathStore.AddPaths(allIDs, []string{proc.workDir})
|
||||
} else {
|
||||
api.pathStore.Notify(allIDs)
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/spf13/afero"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
@@ -1110,7 +1111,7 @@ func TestChatIsolation(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T) {
|
||||
func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_TracksDefaultDir(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
pathStore := agentgit.NewPathStore()
|
||||
@@ -1118,10 +1119,11 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T)
|
||||
ch, unsub := pathStore.Subscribe(chatID)
|
||||
defer unsub()
|
||||
|
||||
defaultDir := t.TempDir()
|
||||
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, pathStore, nil, func(current []string) ([]string, error) {
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, afero.NewOsFs(), pathStore, nil, func(current []string) ([]string, error) {
|
||||
return current, nil
|
||||
}, nil)
|
||||
}, func() string { return defaultDir })
|
||||
defer api.Close()
|
||||
|
||||
routes := agentchat.Middleware(api.Routes())
|
||||
@@ -1138,16 +1140,15 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T)
|
||||
|
||||
require.Equal(t, http.StatusOK, rw.Code)
|
||||
|
||||
// The subscriber should be notified even though no paths
|
||||
// were added.
|
||||
// A command without a workdir runs in the default directory,
|
||||
// which counts as touched by the chat.
|
||||
select {
|
||||
case <-ch:
|
||||
case <-time.After(testutil.WaitShort):
|
||||
t.Fatal("timed out waiting for path store notification")
|
||||
}
|
||||
|
||||
// No paths should have been stored for this chat.
|
||||
require.Nil(t, pathStore.GetPaths(chatID))
|
||||
require.Equal(t, []string{defaultDir}, pathStore.GetPaths(chatID))
|
||||
}
|
||||
|
||||
func TestProcessLifecycle(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user