mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
Michael Suchacz
a554de372a
> This PR was authored by Mux on behalf of Mike. Chats sharing one workspace (e.g. sibling subagents) all wrote to `/home/coder/PLAN.md`, causing plan file collisions. This change derives a unique plan path per chat from the workspace home directory and chat ID. ## Changes * `write_file`, `edit_files`, and `propose_plan` reject any `plan.md` variant (case-insensitive) at the workspace home root, with a clear error pointing to the chat-specific path. * Root chats receive a `<plan-file-path>` block inlined in the main system prompt with the concrete path. * Prompt and tool descriptions no longer hardcode `/home/coder/PLAN.md`. * Plan path handling is POSIX-only (forward-slash), relying on the contract that workspace agent paths are normalized before reaching chatd. * Updated `ProposePlanTool.stories.tsx` to use per-chat path examples. * Full test coverage for plan path detection, legacy-path rejection in all three tools, inline prompt rendering, and fallback behavior.
291 lines
9.2 KiB
Go
291 lines
9.2 KiB
Go
package chattool_test
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
"charm.land/fantasy"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"go.uber.org/mock/gomock"
|
|
"golang.org/x/xerrors"
|
|
|
|
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
|
|
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
|
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
|
|
)
|
|
|
|
func TestEditFiles(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("RejectsPlanPathsWhenResolvePlanPathIsConfigured", func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
input string
|
|
expectedRejectedPath string
|
|
}{
|
|
{
|
|
name: "SingleHomeRootPlanPath",
|
|
input: `{"files":[{"path":"/Users/dev/plan.md","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
expectedRejectedPath: "/Users/dev/plan.md",
|
|
},
|
|
{
|
|
name: "MultiFileBatchWithHomeRootPlanPath",
|
|
input: `{"files":[` +
|
|
`{"path":"/Users/dev/subdir/plan.md","edits":[{"search":"old","replace":"new"}]},` +
|
|
`{"path":"/Users/dev/plan.md","edits":[{"search":"old","replace":"new"}]}` +
|
|
`]}`,
|
|
expectedRejectedPath: "/Users/dev/plan.md",
|
|
},
|
|
}
|
|
|
|
for _, testCase := range tests {
|
|
t.Run(testCase.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
resolvePlanPathCalls := 0
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
resolvePlanPathCalls++
|
|
return "/Users/dev/.coder/plans/PLAN-chat.md", "/Users/dev", nil
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: testCase.input,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.True(t, resp.IsError)
|
|
assert.Equal(t, 1, resolvePlanPathCalls)
|
|
assert.Equal(
|
|
t,
|
|
editFilesBatchRejectedMessage(sharedPlanPathResolvedMessage(
|
|
testCase.expectedRejectedPath,
|
|
"/Users/dev/.coder/plans/PLAN-chat.md",
|
|
)),
|
|
resp.Content,
|
|
)
|
|
})
|
|
}
|
|
})
|
|
|
|
t.Run("RejectsSharedPlanPathWhenResolverFails", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
return "", "", xerrors.New("workspace unavailable")
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"/home/coder/plan.md","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.True(t, resp.IsError)
|
|
assert.Equal(t, editFilesBatchRejectedMessage(planPathVerificationMessage("/home/coder/plan.md")), resp.Content)
|
|
})
|
|
|
|
t.Run("RejectsRelativePlanPathsWhenResolvePlanPathIsConfigured", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
resolvePlanPathCalled := false
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
resolvePlanPathCalled = true
|
|
return "/home/coder/.coder/plans/PLAN-chat.md", "/home/coder", nil
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"plan.md","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.True(t, resp.IsError)
|
|
assert.False(t, resolvePlanPathCalled)
|
|
assert.Equal(t, editFilesBatchRejectedMessage(relativePlanPathMessage()), resp.Content)
|
|
})
|
|
|
|
t.Run("PerChatPlanPathIsAllowed", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
chatPlanPath := "/home/coder/.coder/plans/PLAN-123e4567-e89b-12d3-a456-426614174000.md"
|
|
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
|
|
Path: chatPlanPath,
|
|
Edits: []workspacesdk.FileEdit{{
|
|
Search: "old",
|
|
Replace: "new",
|
|
}},
|
|
}}}
|
|
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
|
|
|
|
resolvePlanPathCalled := false
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
resolvePlanPathCalled = true
|
|
return chatPlanPath, "/home/coder", nil
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"` + chatPlanPath + `","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.False(t, resp.IsError)
|
|
assert.False(t, resolvePlanPathCalled)
|
|
})
|
|
|
|
t.Run("NestedPlanPathAllowedWhenResolverFails", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
|
|
Path: "/home/coder/myproject/plan.md",
|
|
Edits: []workspacesdk.FileEdit{{
|
|
Search: "old",
|
|
Replace: "new",
|
|
}},
|
|
}}}
|
|
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
|
|
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
return "", "", xerrors.New("workspace unavailable")
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"/home/coder/myproject/plan.md","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.False(t, resp.IsError)
|
|
})
|
|
|
|
t.Run("NestedPlanPathUnderHomeIsAllowed", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
|
|
Path: "/home/coder/myproject/plan.md",
|
|
Edits: []workspacesdk.FileEdit{{
|
|
Search: "old",
|
|
Replace: "new",
|
|
}},
|
|
}}}
|
|
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
|
|
|
|
planPathCalled := false
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
planPathCalled = true
|
|
return "/home/coder/.coder/plans/PLAN-chat.md", "/home/coder", nil
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"/home/coder/myproject/plan.md","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.False(t, resp.IsError)
|
|
assert.True(t, planPathCalled)
|
|
})
|
|
|
|
t.Run("AllowsNonSharedPath", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
|
|
Path: "/home/dev/my-plan.md",
|
|
Edits: []workspacesdk.FileEdit{{
|
|
Search: "old",
|
|
Replace: "new",
|
|
}},
|
|
}}}
|
|
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
|
|
|
|
resolvePlanPathCalled := false
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
ResolvePlanPath: func(context.Context) (string, string, error) {
|
|
resolvePlanPathCalled = true
|
|
return "", "", xerrors.New("should not be called")
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"/home/dev/my-plan.md","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.False(t, resp.IsError)
|
|
assert.False(t, resolvePlanPathCalled)
|
|
})
|
|
|
|
t.Run("AllowsSharedPlanPathWhenResolvePlanPathIsNil", func(t *testing.T) {
|
|
t.Parallel()
|
|
ctrl := gomock.NewController(t)
|
|
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
|
request := workspacesdk.FileEditRequest{Files: []workspacesdk.FileEdits{{
|
|
Path: chattool.LegacySharedPlanPath,
|
|
Edits: []workspacesdk.FileEdit{{
|
|
Search: "old",
|
|
Replace: "new",
|
|
}},
|
|
}}}
|
|
mockConn.EXPECT().EditFiles(gomock.Any(), request).Return(nil)
|
|
|
|
tool := chattool.EditFiles(chattool.EditFilesOptions{
|
|
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
|
return mockConn, nil
|
|
},
|
|
})
|
|
|
|
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
|
ID: "call-1",
|
|
Name: "edit_files",
|
|
Input: `{"files":[{"path":"` + chattool.LegacySharedPlanPath + `","edits":[{"search":"old","replace":"new"}]}]}`,
|
|
})
|
|
require.NoError(t, err)
|
|
assert.False(t, resp.IsError)
|
|
})
|
|
}
|