Skip to content

Commit 4bf63b4

Browse files
authored
feat: add coder_workspace_read_file MCP tool (#19562)
Follows similarly to the bash tool (and some code to connect to an agent was extracted from it). There are two main parts: a new agent endpoint, and then a new MCP tool that consumes that endpoint.
1 parent f402ec9 commit 4bf63b4

File tree

10 files changed

+678
-116
lines changed

10 files changed

+678
-116
lines changed

agent/api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ func (a *agent) apiHandler() http.Handler {
6060
r.Get("/api/v0/listening-ports", lp.handler)
6161
r.Get("/api/v0/netcheck", a.HandleNetcheck)
6262
r.Post("/api/v0/list-directory", a.HandleLS)
63+
r.Get("/api/v0/read-file", a.HandleReadFile)
6364
r.Get("/debug/logs", a.HandleHTTPDebugLogs)
6465
r.Get("/debug/magicsock", a.HandleHTTPDebugMagicsock)
6566
r.Get("/debug/magicsock/debug-logging/{state}", a.HandleHTTPMagicsockDebugLoggingState)

agent/files.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package agent
2+
3+
import (
4+
"context"
5+
"errors"
6+
"io"
7+
"mime"
8+
"net/http"
9+
"os"
10+
"path/filepath"
11+
"strconv"
12+
13+
"golang.org/x/xerrors"
14+
15+
"cdr.dev/slog"
16+
"github.com/coder/coder/v2/coderd/httpapi"
17+
"github.com/coder/coder/v2/codersdk"
18+
)
19+
20+
func (a *agent) HandleReadFile(rw http.ResponseWriter, r *http.Request) {
21+
ctx := r.Context()
22+
23+
query := r.URL.Query()
24+
parser := httpapi.NewQueryParamParser().RequiredNotEmpty("path")
25+
path := parser.String(query, "", "path")
26+
offset := parser.PositiveInt64(query, 0, "offset")
27+
limit := parser.PositiveInt64(query, 0, "limit")
28+
parser.ErrorExcessParams(query)
29+
if len(parser.Errors) > 0 {
30+
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
31+
Message: "Query parameters have invalid values.",
32+
Validations: parser.Errors,
33+
})
34+
return
35+
}
36+
37+
status, err := a.streamFile(ctx, rw, path, offset, limit)
38+
if err != nil {
39+
httpapi.Write(ctx, rw, status, codersdk.Response{
40+
Message: err.Error(),
41+
})
42+
return
43+
}
44+
}
45+
46+
func (a *agent) streamFile(ctx context.Context, rw http.ResponseWriter, path string, offset, limit int64) (int, error) {
47+
if !filepath.IsAbs(path) {
48+
return http.StatusBadRequest, xerrors.Errorf("file path must be absolute: %q", path)
49+
}
50+
51+
f, err := a.filesystem.Open(path)
52+
if err != nil {
53+
status := http.StatusInternalServerError
54+
switch {
55+
case errors.Is(err, os.ErrNotExist):
56+
status = http.StatusNotFound
57+
case errors.Is(err, os.ErrPermission):
58+
status = http.StatusForbidden
59+
}
60+
return status, err
61+
}
62+
defer f.Close()
63+
64+
stat, err := f.Stat()
65+
if err != nil {
66+
return http.StatusInternalServerError, err
67+
}
68+
69+
if stat.IsDir() {
70+
return http.StatusBadRequest, xerrors.Errorf("open %s: not a file", path)
71+
}
72+
73+
size := stat.Size()
74+
if limit == 0 {
75+
limit = size
76+
}
77+
bytesRemaining := max(size-offset, 0)
78+
bytesToRead := min(bytesRemaining, limit)
79+
80+
// Relying on just the file name for the mime type for now.
81+
mimeType := mime.TypeByExtension(filepath.Ext(path))
82+
if mimeType == "" {
83+
mimeType = "application/octet-stream"
84+
}
85+
rw.Header().Set("Content-Type", mimeType)
86+
rw.Header().Set("Content-Length", strconv.FormatInt(bytesToRead, 10))
87+
rw.WriteHeader(http.StatusOK)
88+
89+
reader := io.NewSectionReader(f, offset, bytesToRead)
90+
_, err = io.Copy(rw, reader)
91+
if err != nil && !errors.Is(err, io.EOF) && ctx.Err() == nil {
92+
a.logger.Error(ctx, "workspace agent read file", slog.Error(err))
93+
}
94+
95+
return 0, nil
96+
}

agent/files_test.go

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
package agent_test
2+
3+
import (
4+
"context"
5+
"io"
6+
"net/http"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
11+
"github.com/spf13/afero"
12+
"github.com/stretchr/testify/require"
13+
14+
"github.com/coder/coder/v2/agent"
15+
"github.com/coder/coder/v2/agent/agenttest"
16+
"github.com/coder/coder/v2/coderd/coderdtest"
17+
"github.com/coder/coder/v2/codersdk/agentsdk"
18+
"github.com/coder/coder/v2/testutil"
19+
)
20+
21+
type testFs struct {
22+
afero.Fs
23+
// intercept can return an error for testing when a call fails.
24+
intercept func(call, file string) error
25+
}
26+
27+
func newTestFs(base afero.Fs, intercept func(call, file string) error) *testFs {
28+
return &testFs{
29+
Fs: base,
30+
intercept: intercept,
31+
}
32+
}
33+
34+
func (fs *testFs) Open(name string) (afero.File, error) {
35+
if err := fs.intercept("open", name); err != nil {
36+
return nil, err
37+
}
38+
return fs.Fs.Open(name)
39+
}
40+
41+
func TestReadFile(t *testing.T) {
42+
t.Parallel()
43+
44+
tmpdir := os.TempDir()
45+
noPermsFilePath := filepath.Join(tmpdir, "no-perms")
46+
//nolint:dogsled
47+
conn, _, _, fs, _ := setupAgent(t, agentsdk.Manifest{}, 0, func(_ *agenttest.Client, opts *agent.Options) {
48+
opts.Filesystem = newTestFs(opts.Filesystem, func(call, file string) error {
49+
if file == noPermsFilePath {
50+
return os.ErrPermission
51+
}
52+
return nil
53+
})
54+
})
55+
56+
dirPath := filepath.Join(tmpdir, "a-directory")
57+
err := fs.MkdirAll(dirPath, 0o755)
58+
require.NoError(t, err)
59+
60+
filePath := filepath.Join(tmpdir, "file")
61+
err = afero.WriteFile(fs, filePath, []byte("content"), 0o644)
62+
require.NoError(t, err)
63+
64+
imagePath := filepath.Join(tmpdir, "file.png")
65+
err = afero.WriteFile(fs, imagePath, []byte("not really an image"), 0o644)
66+
require.NoError(t, err)
67+
68+
tests := []struct {
69+
name string
70+
path string
71+
limit int64
72+
offset int64
73+
bytes []byte
74+
mimeType string
75+
errCode int
76+
error string
77+
}{
78+
{
79+
name: "NoPath",
80+
path: "",
81+
errCode: http.StatusBadRequest,
82+
error: "\"path\" is required",
83+
},
84+
{
85+
name: "RelativePath",
86+
path: "./relative",
87+
errCode: http.StatusBadRequest,
88+
error: "file path must be absolute",
89+
},
90+
{
91+
name: "RelativePath",
92+
path: "also-relative",
93+
errCode: http.StatusBadRequest,
94+
error: "file path must be absolute",
95+
},
96+
{
97+
name: "NegativeLimit",
98+
path: filePath,
99+
limit: -10,
100+
errCode: http.StatusBadRequest,
101+
error: "value is negative",
102+
},
103+
{
104+
name: "NegativeOffset",
105+
path: filePath,
106+
offset: -10,
107+
errCode: http.StatusBadRequest,
108+
error: "value is negative",
109+
},
110+
{
111+
name: "NonExistent",
112+
path: filepath.Join(tmpdir, "does-not-exist"),
113+
errCode: http.StatusNotFound,
114+
error: "file does not exist",
115+
},
116+
{
117+
name: "IsDir",
118+
path: dirPath,
119+
errCode: http.StatusBadRequest,
120+
error: "not a file",
121+
},
122+
{
123+
name: "NoPermissions",
124+
path: noPermsFilePath,
125+
errCode: http.StatusForbidden,
126+
error: "permission denied",
127+
},
128+
{
129+
name: "Defaults",
130+
path: filePath,
131+
bytes: []byte("content"),
132+
mimeType: "application/octet-stream",
133+
},
134+
{
135+
name: "Limit1",
136+
path: filePath,
137+
limit: 1,
138+
bytes: []byte("c"),
139+
mimeType: "application/octet-stream",
140+
},
141+
{
142+
name: "Offset1",
143+
path: filePath,
144+
offset: 1,
145+
bytes: []byte("ontent"),
146+
mimeType: "application/octet-stream",
147+
},
148+
{
149+
name: "Limit1Offset2",
150+
path: filePath,
151+
limit: 1,
152+
offset: 2,
153+
bytes: []byte("n"),
154+
mimeType: "application/octet-stream",
155+
},
156+
{
157+
name: "Limit7Offset0",
158+
path: filePath,
159+
limit: 7,
160+
offset: 0,
161+
bytes: []byte("content"),
162+
mimeType: "application/octet-stream",
163+
},
164+
{
165+
name: "Limit100",
166+
path: filePath,
167+
limit: 100,
168+
bytes: []byte("content"),
169+
mimeType: "application/octet-stream",
170+
},
171+
{
172+
name: "Offset7",
173+
path: filePath,
174+
offset: 7,
175+
bytes: []byte{},
176+
mimeType: "application/octet-stream",
177+
},
178+
{
179+
name: "Offset100",
180+
path: filePath,
181+
offset: 100,
182+
bytes: []byte{},
183+
mimeType: "application/octet-stream",
184+
},
185+
{
186+
name: "MimeTypePng",
187+
path: imagePath,
188+
bytes: []byte("not really an image"),
189+
mimeType: "image/png",
190+
},
191+
}
192+
193+
for _, tt := range tests {
194+
t.Run(tt.name, func(t *testing.T) {
195+
t.Parallel()
196+
197+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
198+
defer cancel()
199+
200+
reader, mimeType, err := conn.ReadFile(ctx, tt.path, tt.offset, tt.limit)
201+
if tt.errCode != 0 {
202+
require.Error(t, err)
203+
cerr := coderdtest.SDKError(t, err)
204+
require.Contains(t, cerr.Error(), tt.error)
205+
require.Equal(t, tt.errCode, cerr.StatusCode())
206+
} else {
207+
require.NoError(t, err)
208+
defer reader.Close()
209+
bytes, err := io.ReadAll(reader)
210+
require.NoError(t, err)
211+
require.Equal(t, tt.bytes, bytes)
212+
require.Equal(t, tt.mimeType, mimeType)
213+
}
214+
})
215+
}
216+
}

coderd/httpapi/queryparams.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,27 @@ func (p *QueryParamParser) PositiveInt32(vals url.Values, def int32, queryParam
120120
return v
121121
}
122122

123+
// PositiveInt64 function checks if the given value is 64-bit and positive.
124+
func (p *QueryParamParser) PositiveInt64(vals url.Values, def int64, queryParam string) int64 {
125+
v, err := parseQueryParam(p, vals, func(v string) (int64, error) {
126+
intValue, err := strconv.ParseInt(v, 10, 64)
127+
if err != nil {
128+
return 0, err
129+
}
130+
if intValue < 0 {
131+
return 0, xerrors.Errorf("value is negative")
132+
}
133+
return intValue, nil
134+
}, def, queryParam)
135+
if err != nil {
136+
p.Errors = append(p.Errors, codersdk.ValidationError{
137+
Field: queryParam,
138+
Detail: fmt.Sprintf("Query param %q must be a valid 64-bit positive integer: %s", queryParam, err.Error()),
139+
})
140+
}
141+
return v
142+
}
143+
123144
// NullableBoolean will return a null sql value if no input is provided.
124145
// SQLc still uses sql.NullBool rather than the generic type. So converting from
125146
// the generic type is required.

0 commit comments

Comments
 (0)