mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: use per-chat plan file paths (#24268)
> 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.
This commit is contained in:
@@ -2,6 +2,7 @@ package chattool
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
|
||||
"charm.land/fantasy"
|
||||
|
||||
@@ -10,6 +11,7 @@ import (
|
||||
|
||||
type EditFilesOptions struct {
|
||||
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
|
||||
ResolvePlanPath func(context.Context) (chatPath string, home string, err error)
|
||||
}
|
||||
|
||||
type EditFilesArgs struct {
|
||||
@@ -29,7 +31,7 @@ func EditFiles(options EditFilesOptions) fantasy.AgentTool {
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
return executeEditFilesTool(ctx, conn, args)
|
||||
return executeEditFilesTool(ctx, conn, args, options.ResolvePlanPath)
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -38,11 +40,42 @@ func executeEditFilesTool(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
args EditFilesArgs,
|
||||
resolvePlanPath func(context.Context) (chatPath string, home string, err error),
|
||||
) (fantasy.ToolResponse, error) {
|
||||
if len(args.Files) == 0 {
|
||||
return fantasy.NewTextErrorResponse("files is required"), nil
|
||||
}
|
||||
|
||||
var (
|
||||
chatPath string
|
||||
home string
|
||||
planPathErr error
|
||||
planPathLoaded bool
|
||||
)
|
||||
for i := range args.Files {
|
||||
args.Files[i].Path = strings.TrimSpace(args.Files[i].Path)
|
||||
file := args.Files[i]
|
||||
|
||||
hasPlanFileName := looksLikePlanFileName(file.Path)
|
||||
if hasPlanFileName && !isAbsolutePath(file.Path) {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"plan files must use absolute paths; use the chat-specific absolute plan path; no files in this batch were applied",
|
||||
), nil
|
||||
}
|
||||
if resolvePlanPath == nil || !hasPlanFileName {
|
||||
continue
|
||||
}
|
||||
if !planPathLoaded {
|
||||
chatPath, home, planPathErr = resolvePlanPath(ctx)
|
||||
planPathLoaded = true
|
||||
}
|
||||
if resp, rejected := rejectSharedPlanPath(file.Path, home, chatPath, planPathErr); rejected {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
resp.Content + "; no files in this batch were applied",
|
||||
), nil
|
||||
}
|
||||
}
|
||||
|
||||
if err := conn.EditFiles(ctx, workspacesdk.FileEditRequest{Files: args.Files}); err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,290 @@
|
||||
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)
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,90 @@
|
||||
package chattool
|
||||
|
||||
import (
|
||||
"context"
|
||||
"path"
|
||||
"strings"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
||||
)
|
||||
|
||||
const planFileNamePrefix = "PLAN-"
|
||||
|
||||
// LegacySharedPlanPath is the original shared plan file path used by
|
||||
// every chat in a workspace.
|
||||
const LegacySharedPlanPath = "/home/coder/PLAN.md"
|
||||
|
||||
// ResolveWorkspaceHome returns the workspace user's home directory.
|
||||
func ResolveWorkspaceHome(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
) (string, error) {
|
||||
if conn == nil {
|
||||
return "", xerrors.New("workspace connection is required")
|
||||
}
|
||||
|
||||
resp, err := conn.LS(ctx, "", workspacesdk.LSRequest{
|
||||
Path: []string{},
|
||||
Relativity: workspacesdk.LSRelativityHome,
|
||||
})
|
||||
if err != nil {
|
||||
return "", xerrors.Errorf("resolve workspace home: %w", err)
|
||||
}
|
||||
|
||||
home := strings.TrimSpace(resp.AbsolutePathString)
|
||||
if home == "" {
|
||||
return "", xerrors.New("workspace home path is empty")
|
||||
}
|
||||
|
||||
return home, nil
|
||||
}
|
||||
|
||||
// PlanPathForChat returns the per-chat plan file path rooted in the
|
||||
// workspace home directory.
|
||||
func PlanPathForChat(home string, chatID uuid.UUID) string {
|
||||
return path.Join(
|
||||
home,
|
||||
".coder",
|
||||
"plans",
|
||||
planFileNamePrefix+chatID.String()+".md",
|
||||
)
|
||||
}
|
||||
|
||||
// chatd consumes agent-normalized POSIX paths. Workspace agents are
|
||||
// expected to convert separators to forward slashes before these
|
||||
// helpers run.
|
||||
|
||||
// isAbsolutePath reports whether p is an absolute POSIX path.
|
||||
func isAbsolutePath(p string) bool {
|
||||
return path.IsAbs(p)
|
||||
}
|
||||
|
||||
// looksLikePlanFileName reports whether the base name of requestedPath
|
||||
// is "plan.md" (case-insensitive), ignoring the directory component.
|
||||
func looksLikePlanFileName(requestedPath string) bool {
|
||||
cleaned := path.Clean(requestedPath)
|
||||
return strings.EqualFold(path.Base(cleaned), "plan.md")
|
||||
}
|
||||
|
||||
// LooksLikeHomePlanFile reports whether requestedPath is a plan.md
|
||||
// variant (case-insensitive) sitting directly in the workspace home
|
||||
// directory.
|
||||
// The filename is compared case-insensitively because LLM output varies.
|
||||
func LooksLikeHomePlanFile(requestedPath, home string) bool {
|
||||
normalized := path.Clean(requestedPath)
|
||||
normalizedHome := path.Clean(home)
|
||||
|
||||
return looksLikePlanFileName(normalized) &&
|
||||
strings.EqualFold(path.Dir(normalized), normalizedHome)
|
||||
}
|
||||
|
||||
// looksLikeLegacySharedPlanPath reports whether requestedPath
|
||||
// matches the legacy shared plan path (case-insensitive). Used as a
|
||||
// narrow fallback when the workspace home cannot be resolved.
|
||||
func looksLikeLegacySharedPlanPath(requestedPath string) bool {
|
||||
normalized := path.Clean(requestedPath)
|
||||
return strings.EqualFold(normalized, LegacySharedPlanPath)
|
||||
}
|
||||
@@ -0,0 +1,132 @@
|
||||
package chattool
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestIsAbsolutePath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
path string
|
||||
want bool
|
||||
}{
|
||||
{"/home/coder/PLAN.md", true},
|
||||
{"/workspace/project/plan.md", true},
|
||||
{"plan.md", false},
|
||||
{"./plan.md", false},
|
||||
{"../plan.md", false},
|
||||
{"", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.path, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
require.Equal(t, tt.want, isAbsolutePath(tt.path))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLooksLikePlanFileName(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
require.True(t, looksLikePlanFileName("plan.md"))
|
||||
require.True(t, looksLikePlanFileName("./Plan.md"))
|
||||
require.True(t, looksLikePlanFileName("/home/coder/PLAN.md"))
|
||||
require.False(t, looksLikePlanFileName("/home/coder/README.md"))
|
||||
}
|
||||
|
||||
func TestLooksLikeLegacySharedPlanPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
requested string
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "ExactMatch",
|
||||
requested: "/home/coder/PLAN.md",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "CaseInsensitive",
|
||||
requested: "/home/coder/plan.md",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "MixedCase",
|
||||
requested: "/home/coder/Plan.md",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "NestedPath",
|
||||
requested: "/home/coder/myproject/plan.md",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "DifferentHome",
|
||||
requested: "/Users/dev/PLAN.md",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "PerChatPath",
|
||||
requested: "/home/coder/.coder/plans/PLAN-123e4567-e89b-12d3-a456-426614174000.md",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "EmptyString",
|
||||
requested: "",
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range tests {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
require.Equal(t, testCase.want, looksLikeLegacySharedPlanPath(testCase.requested))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRejectSharedPlanPath(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
resp, rejected := rejectSharedPlanPath(
|
||||
LegacySharedPlanPath,
|
||||
"/Users/dev",
|
||||
"/Users/dev/.coder/plans/PLAN-chat.md",
|
||||
nil,
|
||||
)
|
||||
|
||||
require.True(t, rejected)
|
||||
require.True(t, resp.IsError)
|
||||
require.Equal(
|
||||
t,
|
||||
sharedPlanPathMessage(
|
||||
LegacySharedPlanPath,
|
||||
"/Users/dev/.coder/plans/PLAN-chat.md",
|
||||
),
|
||||
resp.Content,
|
||||
)
|
||||
}
|
||||
|
||||
func TestSharedPlanPathMessage(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
require.Equal(
|
||||
t,
|
||||
"the plan path /home/coder/plan.md is no longer supported at the home root; use the chat-specific plan path: /home/coder/.coder/plans/PLAN-chat.md",
|
||||
sharedPlanPathMessage(
|
||||
"/home/coder/plan.md",
|
||||
"/home/coder/.coder/plans/PLAN-chat.md",
|
||||
),
|
||||
)
|
||||
require.Equal(
|
||||
t,
|
||||
"the plan path /home/coder/plan.md could not be verified because the workspace is currently unavailable to resolve the chat-specific plan path, try again shortly",
|
||||
planPathVerificationMessage("/home/coder/plan.md"),
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,219 @@
|
||||
package chattool_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"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 TestResolveWorkspaceHome(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
resp workspacesdk.LSResponse
|
||||
lsErr error
|
||||
want string
|
||||
wantErr bool
|
||||
errMatch string
|
||||
}{
|
||||
{
|
||||
name: "StandardLinuxHome",
|
||||
resp: workspacesdk.LSResponse{AbsolutePathString: "/home/coder"},
|
||||
want: "/home/coder",
|
||||
},
|
||||
{
|
||||
name: "NonStandardHome",
|
||||
resp: workspacesdk.LSResponse{AbsolutePathString: "/Users/dev"},
|
||||
want: "/Users/dev",
|
||||
},
|
||||
{
|
||||
name: "LSError",
|
||||
lsErr: xerrors.New("list failed"),
|
||||
wantErr: true,
|
||||
errMatch: "list failed",
|
||||
},
|
||||
{
|
||||
name: "EmptyAbsolutePathString",
|
||||
resp: workspacesdk.LSResponse{AbsolutePathString: ""},
|
||||
wantErr: true,
|
||||
errMatch: "workspace home path is empty",
|
||||
},
|
||||
{
|
||||
name: "WhitespaceOnlyAbsolutePathString",
|
||||
resp: workspacesdk.LSResponse{AbsolutePathString: " \t\n "},
|
||||
wantErr: true,
|
||||
errMatch: "workspace home path is empty",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range tests {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
conn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
conn.EXPECT().LS(
|
||||
gomock.Any(),
|
||||
"",
|
||||
workspacesdk.LSRequest{
|
||||
Path: []string{},
|
||||
Relativity: workspacesdk.LSRelativityHome,
|
||||
},
|
||||
).Return(testCase.resp, testCase.lsErr)
|
||||
|
||||
got, err := chattool.ResolveWorkspaceHome(context.Background(), conn)
|
||||
if testCase.wantErr {
|
||||
require.Error(t, err)
|
||||
require.ErrorContains(t, err, testCase.errMatch)
|
||||
require.Empty(t, got)
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, testCase.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlanPathForChat(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("StandardHome", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
chatID := uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")
|
||||
|
||||
got := chattool.PlanPathForChat("/home/coder", chatID)
|
||||
|
||||
require.Equal(
|
||||
t,
|
||||
"/home/coder/.coder/plans/PLAN-123e4567-e89b-12d3-a456-426614174000.md",
|
||||
got,
|
||||
)
|
||||
})
|
||||
|
||||
t.Run("NonStandardHome", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
chatID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
|
||||
|
||||
got := chattool.PlanPathForChat("/Users/dev", chatID)
|
||||
|
||||
require.Equal(
|
||||
t,
|
||||
"/Users/dev/.coder/plans/PLAN-aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee.md",
|
||||
got,
|
||||
)
|
||||
})
|
||||
|
||||
t.Run("MatchesExpectedFormat", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
home := "/workspace/home"
|
||||
chatID := uuid.MustParse("f47ac10b-58cc-4372-a567-0e02b2c3d479")
|
||||
|
||||
got := chattool.PlanPathForChat(home, chatID)
|
||||
|
||||
require.True(t, strings.HasPrefix(got, home+"/.coder/plans/PLAN-"))
|
||||
require.True(t, strings.HasSuffix(got, chatID.String()+".md"))
|
||||
})
|
||||
}
|
||||
|
||||
func TestLooksLikeHomePlanFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
requested string
|
||||
home string
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "UppercaseHomeRootPlan",
|
||||
requested: "/home/coder/PLAN.md",
|
||||
home: "/home/coder",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "LowercaseHomeRootPlan",
|
||||
requested: "/home/coder/plan.md",
|
||||
home: "/home/coder",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "MixedCaseHomeRootPlan",
|
||||
requested: "/home/coder/Plan.md",
|
||||
home: "/home/coder",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "UppercaseExtension",
|
||||
requested: "/home/coder/PLAN.MD",
|
||||
home: "/home/coder",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "CustomHomeRootPlan",
|
||||
requested: "/Users/dev/plan.md",
|
||||
home: "/Users/dev",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "NestedPlanUnderHome",
|
||||
requested: "/home/coder/myproject/plan.md",
|
||||
home: "/home/coder",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "PerChatPlanPath",
|
||||
requested: "/home/coder/.coder/plans/PLAN-123e4567-e89b-12d3-a456-426614174000.md",
|
||||
home: "/home/coder",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "DifferentFilename",
|
||||
requested: "/home/coder/README.md",
|
||||
home: "/home/coder",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "DifferentExtension",
|
||||
requested: "/home/coder/plan.txt",
|
||||
home: "/home/coder",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "EmptyPath",
|
||||
requested: "",
|
||||
home: "/home/coder",
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "DifferentHomeMismatch",
|
||||
requested: "/home/coder/plan.md",
|
||||
home: "/Users/dev",
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range tests {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
got := chattool.LooksLikeHomePlanFile(testCase.requested, testCase.home)
|
||||
|
||||
require.Equal(t, testCase.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,54 @@
|
||||
package chattool
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"charm.land/fantasy"
|
||||
)
|
||||
|
||||
// rejectSharedPlanPath reports whether requestedPath targets the shared
|
||||
// home-root plan file and, if so, returns a rejection response that
|
||||
// points callers at the chat-specific plan path.
|
||||
func rejectSharedPlanPath(
|
||||
requestedPath string,
|
||||
home string,
|
||||
chatPath string,
|
||||
planPathErr error,
|
||||
) (fantasy.ToolResponse, bool) {
|
||||
if planPathErr != nil {
|
||||
// When the resolver fails, we cannot determine the actual
|
||||
// home directory. Fall back to rejecting only the exact
|
||||
// legacy shared path (case-insensitive) rather than every
|
||||
// file named plan.md.
|
||||
if !looksLikeLegacySharedPlanPath(requestedPath) {
|
||||
return fantasy.ToolResponse{}, false
|
||||
}
|
||||
|
||||
return fantasy.NewTextErrorResponse(
|
||||
planPathVerificationMessage(requestedPath),
|
||||
), true
|
||||
}
|
||||
|
||||
if !LooksLikeHomePlanFile(requestedPath, home) && !looksLikeLegacySharedPlanPath(requestedPath) {
|
||||
return fantasy.ToolResponse{}, false
|
||||
}
|
||||
|
||||
return fantasy.NewTextErrorResponse(
|
||||
sharedPlanPathMessage(requestedPath, chatPath),
|
||||
), true
|
||||
}
|
||||
|
||||
func sharedPlanPathMessage(requestedPath, chatPath string) string {
|
||||
return fmt.Sprintf(
|
||||
"the plan path %s is no longer supported at the home root; use the chat-specific plan path: %s",
|
||||
requestedPath,
|
||||
chatPath,
|
||||
)
|
||||
}
|
||||
|
||||
func planPathVerificationMessage(requestedPath string) string {
|
||||
return fmt.Sprintf(
|
||||
"the plan path %s could not be verified because the workspace is currently unavailable to resolve the chat-specific plan path, try again shortly",
|
||||
requestedPath,
|
||||
)
|
||||
}
|
||||
@@ -17,6 +17,7 @@ const maxProposePlanSize = 32 * 1024 // 32 KiB
|
||||
// ProposePlanOptions configures the propose_plan tool.
|
||||
type ProposePlanOptions struct {
|
||||
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
|
||||
ResolvePlanPath func(context.Context) (chatPath string, home string, err error)
|
||||
StoreFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error)
|
||||
}
|
||||
|
||||
@@ -31,8 +32,9 @@ func ProposePlan(options ProposePlanOptions) fantasy.AgentTool {
|
||||
return fantasy.NewAgentTool(
|
||||
"propose_plan",
|
||||
"Present a Markdown plan file from the workspace for user review. "+
|
||||
"The file must already exist with a .md extension — use write_file to create it or edit_files to refine it before calling this tool. "+
|
||||
"Pass the absolute file path (e.g. /home/coder/PLAN.md). The tool reads the content from the workspace.",
|
||||
"The file must already exist with a .md extension. Use write_file to create it or edit_files to refine it before calling this tool. "+
|
||||
"Pass the absolute file path to the plan. Important: use the chat-specific absolute plan path, not a generic path like PLAN.md in the home directory. "+
|
||||
"The tool reads the content from the workspace.",
|
||||
func(ctx context.Context, args ProposePlanArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) {
|
||||
if options.GetWorkspaceConn == nil {
|
||||
return fantasy.NewTextErrorResponse("workspace connection resolver is not configured"), nil
|
||||
@@ -44,7 +46,7 @@ func ProposePlan(options ProposePlanOptions) fantasy.AgentTool {
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
return executeProposePlanTool(ctx, conn, args, options.StoreFile)
|
||||
return executeProposePlanTool(ctx, conn, args, options.ResolvePlanPath, options.StoreFile)
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -53,17 +55,32 @@ func executeProposePlanTool(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
args ProposePlanArgs,
|
||||
resolvePlanPath func(context.Context) (chatPath string, home string, err error),
|
||||
storeFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error),
|
||||
) (fantasy.ToolResponse, error) {
|
||||
path := strings.TrimSpace(args.Path)
|
||||
if path == "" {
|
||||
return fantasy.NewTextErrorResponse("path is required (use an absolute path, e.g. /home/coder/PLAN.md)"), nil
|
||||
requestedPath := strings.TrimSpace(args.Path)
|
||||
if requestedPath == "" {
|
||||
return fantasy.NewTextErrorResponse("path is required (use the chat-specific absolute plan path)"), nil
|
||||
}
|
||||
if !strings.HasSuffix(path, ".md") {
|
||||
if !strings.HasSuffix(requestedPath, ".md") {
|
||||
return fantasy.NewTextErrorResponse("path must end with .md"), nil
|
||||
}
|
||||
|
||||
rc, _, err := conn.ReadFile(ctx, path, 0, maxProposePlanSize+1)
|
||||
hasPlanFileName := looksLikePlanFileName(requestedPath)
|
||||
if hasPlanFileName && !isAbsolutePath(requestedPath) {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"plan files must use absolute paths; use the chat-specific absolute plan path",
|
||||
), nil
|
||||
}
|
||||
|
||||
if resolvePlanPath != nil && hasPlanFileName {
|
||||
chatPath, home, err := resolvePlanPath(ctx)
|
||||
if resp, rejected := rejectSharedPlanPath(requestedPath, home, chatPath, err); rejected {
|
||||
return resp, nil
|
||||
}
|
||||
}
|
||||
|
||||
rc, _, err := conn.ReadFile(ctx, requestedPath, 0, maxProposePlanSize+1)
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
@@ -77,14 +94,14 @@ func executeProposePlanTool(
|
||||
return fantasy.NewTextErrorResponse("plan file exceeds 32 KiB size limit"), nil
|
||||
}
|
||||
|
||||
fileID, err := storeFile(ctx, filepath.Base(path), "text/markdown", data)
|
||||
fileID, err := storeFile(ctx, filepath.Base(requestedPath), "text/markdown", data)
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse("failed to store plan file: " + err.Error()), nil
|
||||
}
|
||||
|
||||
return toolResponse(map[string]any{
|
||||
"ok": true,
|
||||
"path": path,
|
||||
"path": requestedPath,
|
||||
"kind": "plan",
|
||||
"file_id": fileID.String(),
|
||||
"media_type": "text/markdown",
|
||||
|
||||
@@ -82,6 +82,34 @@ func TestProposePlan(t *testing.T) {
|
||||
assert.Contains(t, resp.Content, "path must end with .md")
|
||||
})
|
||||
|
||||
t.Run("RelativePlanPathReturnsError", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
storeFile, _ := fakeStoreFile(t)
|
||||
resolvePlanPathCalled := false
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
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: "propose_plan",
|
||||
Input: `{"path":"plan.md"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.IsError)
|
||||
assert.False(t, resolvePlanPathCalled)
|
||||
assert.Equal(t, relativePlanPathMessage(), resp.Content)
|
||||
})
|
||||
|
||||
t.Run("OversizedFileRejected", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
@@ -135,7 +163,16 @@ func TestProposePlan(t *testing.T) {
|
||||
Return(io.NopCloser(strings.NewReader("# Plan\n\nContent")), "text/markdown", nil)
|
||||
|
||||
storeFile, stored := fakeStoreFile(t)
|
||||
tool := newProposePlanTool(t, mockConn, storeFile)
|
||||
planPathCalled := false
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
func(context.Context) (string, string, error) {
|
||||
planPathCalled = true
|
||||
return "/home/coder/.coder/plans/PLAN-xxx.md", "/home/coder", nil
|
||||
},
|
||||
)
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "propose_plan",
|
||||
@@ -143,6 +180,7 @@ func TestProposePlan(t *testing.T) {
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.True(t, planPathCalled)
|
||||
|
||||
result := decodeProposePlanResponse(t, resp)
|
||||
assert.True(t, result.OK)
|
||||
@@ -154,6 +192,41 @@ func TestProposePlan(t *testing.T) {
|
||||
assert.NotContains(t, resp.Content, "content")
|
||||
})
|
||||
|
||||
t.Run("NestedPlanPathUnderHomeIsAllowed", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
mockConn.EXPECT().
|
||||
ReadFile(gomock.Any(), "/home/coder/myproject/plan.md", int64(0), int64(32*1024+1)).
|
||||
Return(io.NopCloser(strings.NewReader("# Nested Plan")), "text/markdown", nil)
|
||||
|
||||
storeFile, stored := fakeStoreFile(t)
|
||||
planPathCalled := false
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
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: "propose_plan",
|
||||
Input: `{"path":"/home/coder/myproject/plan.md"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.True(t, planPathCalled)
|
||||
|
||||
result := decodeProposePlanResponse(t, resp)
|
||||
assert.True(t, result.OK)
|
||||
assert.Equal(t, "/home/coder/myproject/plan.md", result.Path)
|
||||
assert.Equal(t, []byte("# Nested Plan"), *stored)
|
||||
})
|
||||
|
||||
t.Run("FileNotFound", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
@@ -218,6 +291,128 @@ func TestProposePlan(t *testing.T) {
|
||||
assert.Contains(t, resp.Content, "storage unavailable")
|
||||
})
|
||||
|
||||
t.Run("RejectsSharedPlanPathWithResolvedPath", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
storeFile, _ := fakeStoreFile(t)
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
func(context.Context) (string, string, error) {
|
||||
return "/home/coder/.coder/plans/PLAN-chat.md", "/home/coder", nil
|
||||
},
|
||||
)
|
||||
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "propose_plan",
|
||||
Input: `{"path":"` + chattool.LegacySharedPlanPath + `"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.IsError)
|
||||
assert.Equal(
|
||||
t,
|
||||
sharedPlanPathResolvedMessage(chattool.LegacySharedPlanPath, "/home/coder/.coder/plans/PLAN-chat.md"),
|
||||
resp.Content,
|
||||
)
|
||||
})
|
||||
|
||||
t.Run("RejectsSharedPlanPathWhenResolverFails", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
storeFile, _ := fakeStoreFile(t)
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
func(context.Context) (string, string, error) {
|
||||
return "", "", xerrors.New("workspace unavailable")
|
||||
},
|
||||
)
|
||||
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "propose_plan",
|
||||
Input: `{"path":"` + chattool.LegacySharedPlanPath + `"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.IsError)
|
||||
assert.Equal(t, planPathVerificationMessage(chattool.LegacySharedPlanPath), 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"
|
||||
|
||||
mockConn.EXPECT().
|
||||
ReadFile(gomock.Any(), chatPlanPath, int64(0), int64(32*1024+1)).
|
||||
Return(io.NopCloser(strings.NewReader("# Per-Chat Plan")), "text/markdown", nil)
|
||||
|
||||
storeFile, stored := fakeStoreFile(t)
|
||||
resolvePlanPathCalled := false
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
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: "propose_plan",
|
||||
Input: `{"path":"` + chatPlanPath + `"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.False(t, resolvePlanPathCalled)
|
||||
|
||||
result := decodeProposePlanResponse(t, resp)
|
||||
assert.True(t, result.OK)
|
||||
assert.Equal(t, chatPlanPath, result.Path)
|
||||
assert.Equal(t, []byte("# Per-Chat Plan"), *stored)
|
||||
})
|
||||
|
||||
t.Run("NestedPlanPathAllowedWhenResolverFails", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
|
||||
mockConn.EXPECT().
|
||||
ReadFile(gomock.Any(), "/home/coder/myproject/plan.md", int64(0), int64(32*1024+1)).
|
||||
Return(io.NopCloser(strings.NewReader("# Nested Plan")), "text/markdown", nil)
|
||||
|
||||
storeFile, stored := fakeStoreFile(t)
|
||||
tool := newProposePlanToolWithPlanPath(
|
||||
t,
|
||||
mockConn,
|
||||
storeFile,
|
||||
func(context.Context) (string, string, error) {
|
||||
return "", "", xerrors.New("workspace unavailable")
|
||||
},
|
||||
)
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "propose_plan",
|
||||
Input: `{"path":"/home/coder/myproject/plan.md"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
|
||||
result := decodeProposePlanResponse(t, resp)
|
||||
assert.True(t, result.OK)
|
||||
assert.Equal(t, "/home/coder/myproject/plan.md", result.Path)
|
||||
assert.Equal(t, []byte("# Nested Plan"), *stored)
|
||||
})
|
||||
|
||||
t.Run("WorkspaceConnectionError", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
storeFile, _ := fakeStoreFile(t)
|
||||
@@ -278,16 +473,45 @@ func newProposePlanTool(
|
||||
t *testing.T,
|
||||
mockConn *agentconnmock.MockAgentConn,
|
||||
storeFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error),
|
||||
) fantasy.AgentTool {
|
||||
t.Helper()
|
||||
return newProposePlanToolWithPlanPath(t, mockConn, storeFile, nil)
|
||||
}
|
||||
|
||||
func newProposePlanToolWithPlanPath(
|
||||
t *testing.T,
|
||||
mockConn *agentconnmock.MockAgentConn,
|
||||
storeFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error),
|
||||
resolvePlanPath func(context.Context) (string, string, error),
|
||||
) fantasy.AgentTool {
|
||||
t.Helper()
|
||||
return chattool.ProposePlan(chattool.ProposePlanOptions{
|
||||
GetWorkspaceConn: func(_ context.Context) (workspacesdk.AgentConn, error) {
|
||||
return mockConn, nil
|
||||
},
|
||||
StoreFile: storeFile,
|
||||
ResolvePlanPath: resolvePlanPath,
|
||||
StoreFile: storeFile,
|
||||
})
|
||||
}
|
||||
|
||||
func sharedPlanPathResolvedMessage(requestedPath, planPath string) string {
|
||||
return "the plan path " + requestedPath +
|
||||
" is no longer supported at the home root; use the chat-specific plan path: " + planPath
|
||||
}
|
||||
|
||||
func planPathVerificationMessage(requestedPath string) string {
|
||||
return "the plan path " + requestedPath +
|
||||
" could not be verified because the workspace is currently unavailable to resolve the chat-specific plan path, try again shortly"
|
||||
}
|
||||
|
||||
func editFilesBatchRejectedMessage(message string) string {
|
||||
return message + "; no files in this batch were applied"
|
||||
}
|
||||
|
||||
func relativePlanPathMessage() string {
|
||||
return "plan files must use absolute paths; use the chat-specific absolute plan path"
|
||||
}
|
||||
|
||||
func fakeStoreFile(t *testing.T) (func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error), *[]byte) {
|
||||
t.Helper()
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
|
||||
type WriteFileOptions struct {
|
||||
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
|
||||
ResolvePlanPath func(context.Context) (chatPath string, home string, err error)
|
||||
}
|
||||
|
||||
type WriteFileArgs struct {
|
||||
@@ -30,7 +31,7 @@ func WriteFile(options WriteFileOptions) fantasy.AgentTool {
|
||||
if err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
return executeWriteFileTool(ctx, conn, args)
|
||||
return executeWriteFileTool(ctx, conn, args, options.ResolvePlanPath)
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -39,12 +40,28 @@ func executeWriteFileTool(
|
||||
ctx context.Context,
|
||||
conn workspacesdk.AgentConn,
|
||||
args WriteFileArgs,
|
||||
resolvePlanPath func(context.Context) (chatPath string, home string, err error),
|
||||
) (fantasy.ToolResponse, error) {
|
||||
if args.Path == "" {
|
||||
requestedPath := strings.TrimSpace(args.Path)
|
||||
if requestedPath == "" {
|
||||
return fantasy.NewTextErrorResponse("path is required"), nil
|
||||
}
|
||||
|
||||
if err := conn.WriteFile(ctx, args.Path, strings.NewReader(args.Content)); err != nil {
|
||||
hasPlanFileName := looksLikePlanFileName(requestedPath)
|
||||
if hasPlanFileName && !isAbsolutePath(requestedPath) {
|
||||
return fantasy.NewTextErrorResponse(
|
||||
"plan files must use absolute paths; use the chat-specific absolute plan path",
|
||||
), nil
|
||||
}
|
||||
|
||||
if resolvePlanPath != nil && hasPlanFileName {
|
||||
chatPath, home, err := resolvePlanPath(ctx)
|
||||
if resp, rejected := rejectSharedPlanPath(requestedPath, home, chatPath, err); rejected {
|
||||
return resp, nil
|
||||
}
|
||||
}
|
||||
|
||||
if err := conn.WriteFile(ctx, requestedPath, strings.NewReader(args.Content)); err != nil {
|
||||
return fantasy.NewTextErrorResponse(err.Error()), nil
|
||||
}
|
||||
return toolResponse(map[string]any{"ok": true}), nil
|
||||
|
||||
@@ -0,0 +1,321 @@
|
||||
package chattool_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"strings"
|
||||
"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 TestWriteFile(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("RejectsHomeRootPlanVariantsWhenResolvePlanPathIsConfigured", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
requested string
|
||||
home string
|
||||
}{
|
||||
{
|
||||
name: "ExactLegacyPath",
|
||||
requested: chattool.LegacySharedPlanPath,
|
||||
home: "/home/coder",
|
||||
},
|
||||
{
|
||||
name: "LowercasePlanAtHomeRoot",
|
||||
requested: "/home/coder/plan.md",
|
||||
home: "/home/coder",
|
||||
},
|
||||
{
|
||||
name: "MixedCasePlanAtHomeRoot",
|
||||
requested: "/home/coder/Plan.md",
|
||||
home: "/home/coder",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range tests {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
||||
return mockConn, nil
|
||||
},
|
||||
ResolvePlanPath: func(context.Context) (string, string, error) {
|
||||
return "/home/coder/.coder/plans/PLAN-chat.md", testCase.home, nil
|
||||
},
|
||||
})
|
||||
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "write_file",
|
||||
Input: `{"path":"` + testCase.requested + `","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.IsError)
|
||||
assert.Equal(
|
||||
t,
|
||||
sharedPlanPathResolvedMessage(
|
||||
testCase.requested,
|
||||
"/home/coder/.coder/plans/PLAN-chat.md",
|
||||
),
|
||||
resp.Content,
|
||||
)
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("RejectsRelativePlanPathsWhenResolvePlanPathIsConfigured", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
requested string
|
||||
}{
|
||||
{
|
||||
name: "PlainRelativePath",
|
||||
requested: "plan.md",
|
||||
},
|
||||
{
|
||||
name: "DotSlashRelativePath",
|
||||
requested: "./plan.md",
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range tests {
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
resolvePlanPathCalled := false
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
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: "write_file",
|
||||
Input: `{"path":"` + testCase.requested + `","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.IsError)
|
||||
assert.False(t, resolvePlanPathCalled)
|
||||
assert.Equal(t, relativePlanPathMessage(), resp.Content)
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("RejectsSharedPlanPathWhenResolverFails", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
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: "write_file",
|
||||
Input: `{"path":"/home/coder/plan.md","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.True(t, resp.IsError)
|
||||
assert.Equal(t, planPathVerificationMessage("/home/coder/plan.md"), 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"
|
||||
mockConn.EXPECT().
|
||||
WriteFile(gomock.Any(), chatPlanPath, gomock.Any()).
|
||||
DoAndReturn(func(_ context.Context, path string, reader io.Reader) error {
|
||||
data, err := io.ReadAll(reader)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, chatPlanPath, path)
|
||||
require.Equal(t, "# Plan", string(data))
|
||||
return nil
|
||||
})
|
||||
|
||||
resolvePlanPathCalled := false
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
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: "write_file",
|
||||
Input: `{"path":"` + chatPlanPath + `","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.False(t, resolvePlanPathCalled)
|
||||
assert.Equal(t, `{"ok":true}`, strings.TrimSpace(resp.Content))
|
||||
})
|
||||
|
||||
t.Run("NestedPlanPathAllowedWhenResolverFails", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
mockConn.EXPECT().
|
||||
WriteFile(gomock.Any(), "/home/coder/myproject/plan.md", gomock.Any()).
|
||||
DoAndReturn(func(_ context.Context, path string, reader io.Reader) error {
|
||||
data, err := io.ReadAll(reader)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "/home/coder/myproject/plan.md", path)
|
||||
require.Equal(t, "# Plan", string(data))
|
||||
return nil
|
||||
})
|
||||
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
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: "write_file",
|
||||
Input: `{"path":"/home/coder/myproject/plan.md","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.Equal(t, `{"ok":true}`, strings.TrimSpace(resp.Content))
|
||||
})
|
||||
|
||||
t.Run("NestedPlanPathUnderHomeIsAllowed", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
mockConn.EXPECT().
|
||||
WriteFile(gomock.Any(), "/home/coder/myproject/plan.md", gomock.Any()).
|
||||
DoAndReturn(func(_ context.Context, path string, reader io.Reader) error {
|
||||
data, err := io.ReadAll(reader)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "/home/coder/myproject/plan.md", path)
|
||||
require.Equal(t, "# Plan", string(data))
|
||||
return nil
|
||||
})
|
||||
|
||||
planPathCalled := false
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
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: "write_file",
|
||||
Input: `{"path":"/home/coder/myproject/plan.md","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.True(t, planPathCalled)
|
||||
assert.Equal(t, `{"ok":true}`, strings.TrimSpace(resp.Content))
|
||||
})
|
||||
|
||||
t.Run("AllowsNonSharedPath", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
mockConn.EXPECT().
|
||||
WriteFile(gomock.Any(), "/home/dev/my-plan.md", gomock.Any()).
|
||||
DoAndReturn(func(_ context.Context, path string, reader io.Reader) error {
|
||||
data, err := io.ReadAll(reader)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "/home/dev/my-plan.md", path)
|
||||
require.Equal(t, "# Plan", string(data))
|
||||
return nil
|
||||
})
|
||||
|
||||
resolvePlanPathCalled := false
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
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: "write_file",
|
||||
Input: `{"path":"/home/dev/my-plan.md","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
assert.False(t, resolvePlanPathCalled)
|
||||
assert.Equal(t, `{"ok":true}`, strings.TrimSpace(resp.Content))
|
||||
})
|
||||
|
||||
t.Run("AllowsSharedPlanPathWhenResolvePlanPathIsNil", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctrl := gomock.NewController(t)
|
||||
mockConn := agentconnmock.NewMockAgentConn(ctrl)
|
||||
mockConn.EXPECT().
|
||||
WriteFile(gomock.Any(), chattool.LegacySharedPlanPath, gomock.Any()).
|
||||
DoAndReturn(func(_ context.Context, _ string, reader io.Reader) error {
|
||||
data, err := io.ReadAll(reader)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "# Plan", string(data))
|
||||
return nil
|
||||
})
|
||||
|
||||
tool := chattool.WriteFile(chattool.WriteFileOptions{
|
||||
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
|
||||
return mockConn, nil
|
||||
},
|
||||
})
|
||||
|
||||
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
|
||||
ID: "call-1",
|
||||
Name: "write_file",
|
||||
Input: `{"path":"` + chattool.LegacySharedPlanPath + `","content":"# Plan"}`,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
assert.False(t, resp.IsError)
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user