mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
feat: add line-based read_file tool with safety limits (#22400)
## Summary Adds a new line-based file reading endpoint to the workspace agent, replacing the unbounded byte-based approach for the `read_file` chat tool and `coder_workspace_read_file` MCP tool. **Problem**: The current `read_file` tool returns the entire file contents with no limits, which can blow up LLM context windows and cause OOM issues with large files. **Solution**: Inspired by [`coder/mux`](https://github.com/coder/mux) and [`openai/codex`](https://github.com/openai/codex), implement a line-based reader with safety limits. ## Changes ### Agent (`agent/agentfiles/`) - New `/read-file-lines` endpoint with `HandleReadFileLines` handler - Line-based `offset` (1-based line number, default: 1) and `limit` (line count, default: 2000) - Safety constants: | Constant | Value | Purpose | |---|---|---| | `MaxFileSize` | 1 MB | Reject files larger than this at stat | | `MaxLineBytes` | 1,024 | Per-line truncation with `... [truncated]` marker | | `MaxResponseLines` | 2,000 | Max lines per response | | `MaxResponseBytes` | 32 KB | Max total response size | | `DefaultLineLimit` | 2,000 | Default when no limit specified | - Line numbering format: `1\tcontent` (tab-separated) - Structured JSON response: `{ success, file_size, total_lines, lines_read, content, error }` - Hard errors when limits exceeded — tells the LLM to use `offset`/`limit` - Existing byte-based `/read-file` endpoint preserved (used by `instruction.go`) ### SDK (`codersdk/workspacesdk/`) - `ReadFileLinesResponse` type added - `ReadFileLines` method added to `AgentConn` interface - Mock regenerated ### Chat tool (`coderd/chatd/chattool/`) - `read_file` tool now uses `conn.ReadFileLines()` instead of `conn.ReadFile()` - Updated tool description to document line-based parameters - Response includes `file_size`, `total_lines`, `lines_read` metadata ### MCP tool (`codersdk/toolsdk/`) - `coder_workspace_read_file` updated to use line-based reading - Schema descriptions updated for line-based offset/limit - Removed `maxFileLimit` constant (agent handles limits now) ### Tests - 13 new test cases for `TestReadFileLines`: - Path validation (empty, relative, non-existent, directory, no permissions) - Empty file handling - Basic read, offset, limit, offset+limit combinations - Offset beyond file length - Long line truncation (>1024 bytes) - Large file rejection (>1MB) - All existing tests pass unchanged ## Design decisions | Decision | Rationale | |---|---| | Line-based, not byte-based | Both coder/mux and openai/codex use line-based — matches how LLMs reason about code | | Default limit of 2000 | Matches codex; prevents accidental full-file dumps while being generous | | 32 KB response cap | Compromise between mux (16 KB) and codex (no cap) | | 1024 byte/line truncation with marker | More generous than codex (500), marker helps LLM know data is missing | | Hard errors on overflow | Matches mux; forces LLM to paginate rather than getting partial data | | Preserve byte-based endpoint | `instruction.go` needs raw byte access for AGENTS.md |
This commit is contained in:
@@ -63,6 +63,7 @@ type AgentConn interface {
|
||||
RecreateDevcontainer(ctx context.Context, devcontainerID string) (codersdk.Response, error)
|
||||
LS(ctx context.Context, path string, req LSRequest) (LSResponse, error)
|
||||
ReadFile(ctx context.Context, path string, offset, limit int64) (io.ReadCloser, string, error)
|
||||
ReadFileLines(ctx context.Context, path string, offset, limit int64, limits ReadFileLinesLimits) (ReadFileLinesResponse, error)
|
||||
WriteFile(ctx context.Context, path string, reader io.Reader) error
|
||||
EditFiles(ctx context.Context, edits FileEditRequest) error
|
||||
SSH(ctx context.Context) (*gonet.TCPConn, error)
|
||||
@@ -551,6 +552,31 @@ func (c *agentConn) LS(ctx context.Context, path string, req LSRequest) (LSRespo
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// ReadFileLines reads a file with line-based offset and limit, returning
|
||||
// line-numbered content with safety limits.
|
||||
func (c *agentConn) ReadFileLines(ctx context.Context, path string, offset, limit int64, limits ReadFileLinesLimits) (ReadFileLinesResponse, error) {
|
||||
ctx, span := tracing.StartSpan(ctx)
|
||||
defer span.End()
|
||||
|
||||
res, err := c.apiRequest(ctx, http.MethodGet, fmt.Sprintf(
|
||||
"/api/v0/read-file-lines?path=%s&offset=%d&limit=%d&max_file_size=%d&max_line_bytes=%d&max_response_lines=%d&max_response_bytes=%d",
|
||||
path, offset, limit, limits.MaxFileSize, limits.MaxLineBytes, limits.MaxResponseLines, limits.MaxResponseBytes,
|
||||
), nil)
|
||||
if err != nil {
|
||||
return ReadFileLinesResponse{}, xerrors.Errorf("do request: %w", err)
|
||||
}
|
||||
defer res.Body.Close()
|
||||
if res.StatusCode != http.StatusOK {
|
||||
return ReadFileLinesResponse{}, codersdk.ReadBodyAsError(res)
|
||||
}
|
||||
|
||||
var resp ReadFileLinesResponse
|
||||
if err := json.NewDecoder(res.Body).Decode(&resp); err != nil {
|
||||
return ReadFileLinesResponse{}, xerrors.Errorf("decode response: %w", err)
|
||||
}
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
// ReadFile reads from a file from the workspace, returning a file reader and
|
||||
// the mime type.
|
||||
func (c *agentConn) ReadFile(ctx context.Context, path string, offset, limit int64) (io.ReadCloser, string, error) {
|
||||
@@ -596,6 +622,51 @@ func (c *agentConn) WriteFile(ctx context.Context, path string, reader io.Reader
|
||||
return nil
|
||||
}
|
||||
|
||||
// ReadFileLinesResponse is the response from the line-based file reader.
|
||||
type ReadFileLinesResponse struct {
|
||||
Success bool `json:"success"`
|
||||
FileSize int64 `json:"file_size,omitempty"`
|
||||
TotalLines int `json:"total_lines,omitempty"`
|
||||
LinesRead int `json:"lines_read,omitempty"`
|
||||
Content string `json:"content,omitempty"`
|
||||
Error string `json:"error,omitempty"`
|
||||
}
|
||||
|
||||
// ReadFileLinesLimits contains configurable safety limits for the line-based
|
||||
// file reader. These are sent as query parameters so callers can tune them
|
||||
// without requiring an agent redeployment.
|
||||
type ReadFileLinesLimits struct {
|
||||
// MaxFileSize is the maximum file size (in bytes) that will be opened.
|
||||
MaxFileSize int64
|
||||
// MaxLineBytes is the per-line byte cap before truncation.
|
||||
MaxLineBytes int
|
||||
// MaxResponseLines is the maximum number of lines in a single response.
|
||||
MaxResponseLines int
|
||||
// MaxResponseBytes is the maximum total bytes of formatted output.
|
||||
MaxResponseBytes int
|
||||
}
|
||||
|
||||
const (
|
||||
// DefaultMaxFileSize is the default maximum file size (1 MB).
|
||||
DefaultMaxFileSize int64 = 1 << 20
|
||||
// DefaultMaxLineBytes is the default per-line truncation threshold.
|
||||
DefaultMaxLineBytes int64 = 1024
|
||||
// DefaultMaxResponseLines is the default max lines per response.
|
||||
DefaultMaxResponseLines int64 = 2000
|
||||
// DefaultMaxResponseBytes is the default max response size (32 KB).
|
||||
DefaultMaxResponseBytes int64 = 32768
|
||||
)
|
||||
|
||||
// DefaultReadFileLinesLimits returns the default limits.
|
||||
func DefaultReadFileLinesLimits() ReadFileLinesLimits {
|
||||
return ReadFileLinesLimits{
|
||||
MaxFileSize: DefaultMaxFileSize,
|
||||
MaxLineBytes: int(DefaultMaxLineBytes),
|
||||
MaxResponseLines: int(DefaultMaxResponseLines),
|
||||
MaxResponseBytes: int(DefaultMaxResponseBytes),
|
||||
}
|
||||
}
|
||||
|
||||
type FileEdit struct {
|
||||
Search string `json:"search"`
|
||||
Replace string `json:"replace"`
|
||||
|
||||
@@ -291,6 +291,21 @@ func (mr *MockAgentConnMockRecorder) ReadFile(ctx, path, offset, limit any) *gom
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadFile", reflect.TypeOf((*MockAgentConn)(nil).ReadFile), ctx, path, offset, limit)
|
||||
}
|
||||
|
||||
// ReadFileLines mocks base method.
|
||||
func (m *MockAgentConn) ReadFileLines(ctx context.Context, path string, offset, limit int64, limits workspacesdk.ReadFileLinesLimits) (workspacesdk.ReadFileLinesResponse, error) {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "ReadFileLines", ctx, path, offset, limit, limits)
|
||||
ret0, _ := ret[0].(workspacesdk.ReadFileLinesResponse)
|
||||
ret1, _ := ret[1].(error)
|
||||
return ret0, ret1
|
||||
}
|
||||
|
||||
// ReadFileLines indicates an expected call of ReadFileLines.
|
||||
func (mr *MockAgentConnMockRecorder) ReadFileLines(ctx, path, offset, limit, limits any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadFileLines", reflect.TypeOf((*MockAgentConn)(nil).ReadFileLines), ctx, path, offset, limit, limits)
|
||||
}
|
||||
|
||||
// ReconnectingPTY mocks base method.
|
||||
func (m *MockAgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string, initOpts ...workspacesdk.AgentReconnectingPTYInitOption) (net.Conn, error) {
|
||||
m.ctrl.T.Helper()
|
||||
|
||||
Reference in New Issue
Block a user