fix(filesystem): support ranged file reads

docs/configuration/hooks/index.md

@@ -216,7 +216,7 @@ Built-ins are typically zero-config and faster than equivalent shell hooks becau
 | `max_iterations`        | `before_llm_call`                                                                         | `["<N>"]` (required)  | Hard-stops the agent after `N` model calls. Stateless: the runtime supplies the iteration counter on every dispatch.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | `snapshot`              | `session_start`, `turn_start`, `turn_end`, `pre_tool_use`, `post_tool_use`, `session_end` | _none_                | Records filesystem snapshots in a shadow git repo under the Docker Agent data directory. No-op outside git repos; respects the source repo's ignore rules and skips newly-added files larger than 2 MiB.                                                                                                                                                                                                                                                                                                                                                                                                                           |
 | `redact_secrets`        | `pre_tool_use`, `before_llm_call`, `tool_response_transform`                              | _none_                | Scrubs detected secrets (API keys, tokens, private keys, …) out of tool call arguments, outgoing chat content, and tool output. The same builtin handles all three events and dispatches on the event name. Auto-registered on all three events by `redact_secrets: true` on the agent — see [`examples/redact_secrets_hooks.yaml`](https://github.com/docker/docker-agent/blob/main/examples/redact_secrets_hooks.yaml) for the manual wiring.                                                                                                                                                                                     |
-| `limit_large_tool_results` | `tool_response_transform`, `session_end`                                               | _none_                | **Always-on safety hook** — automatically injected by the runtime, no configuration required. When a tool result from the `filesystem`, `shell`, `mcp`, or `a2a` categories exceeds 2,000 lines or 50 KiB, the full payload is written to a per-session temp file and replaced in the conversation with a notice plus a bounded tail (last 2,000 lines, up to 50 KiB). The `session_end` leg deletes the temp directory. Internal toolsets (`memory`, `plan`, `tasks`, `think`, …) are not affected. |
+| `limit_large_tool_results` | `tool_response_transform`, `session_end`                                               | _none_                | **Always-on safety hook** — automatically injected by the runtime, no configuration required. When a tool result from the `filesystem`, `shell`, `mcp`, or `a2a` categories exceeds 2,000 lines or 50 KiB, the full payload is written to a per-session temp file and replaced in the conversation with a notice plus a bounded excerpt (2,000 lines, up to 50 KiB): the tail for most tools, but the head for the built-in filesystem `read_file`, whose notice suggests a follow-up call with `line`/`limit` to continue reading. The `session_end` leg deletes the temp directory. Internal toolsets (`memory`, `plan`, `tasks`, `think`, …) are not affected. |
 | `safer_shell`           | `pre_tool_use`                                                                            | _none_                | **Deprecated compatibility shim.** The runtime now classifies every shell command natively (`safe` / `destructive` / `unknown`) and gates it through the session's [safety mode](../permissions/index.md#safety-modes), so this builtin no longer emits verdicts. Pinned entries keep working as pure labellers that attach classification metadata (`safety_label`, `blast_radius`, `category`, `reason`) to the call. Filters by tool name internally (no-op for non-shell calls). |
 | `unload`                | `on_agent_switch`                                                                         | _none_                | POSTs `{"model": "<id>"}` to each of the previous agent's DMR model endpoints (`/_unload` by default, overridable per-model via `unload_api`) to free the GPU/RAM the just-departing model was holding. Pure HTTP — reads the model snapshot the runtime ships on `on_agent_switch` and depends on no provider-specific runtime state. Non-DMR providers (OpenAI, Anthropic, …) are silently skipped, so cross-provider chains are safe. Errors are logged and swallowed; agent switching never blocks on a slow or unreachable engine (each call has a 10 s timeout). See [`examples/unload_on_switch.yaml`](https://github.com/docker/docker-agent/blob/main/examples/unload_on_switch.yaml). |
 

docs/tools/filesystem/index.md

@@ -17,7 +17,7 @@ The filesystem tool gives agents the ability to explore codebases, read and edit
 
 | Tool                   | Description                                                               |
 | ---------------------- | ------------------------------------------------------------------------- |
-| `read_file`            | Read the complete contents of a file                                      |
+| `read_file`            | Read the contents of a file (whole file, or a line range of a text file)  |
 | `read_multiple_files`  | Read several files in one call (more efficient than multiple `read_file`) |
 | `write_file`           | Create or overwrite a file with new content                               |
 | `edit_file`            | Make line-based edits (find-and-replace) in an existing file              |

pkg/acp/filesystem.go

@@ -187,6 +187,9 @@ func (t *FilesystemToolset) handleReadFile(ctx context.Context, toolCall tools.T
 	if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &args); err != nil {
 		return nil, fmt.Errorf("failed to parse arguments: %w", err)
 	}
+	if err := filesystem.ValidateReadFileRange(args.Line, args.Limit); err != nil {
+		return tools.ResultError(fmt.Sprintf("Error: %s", err)), nil
+	}
 
 	sessionID, ok := getSessionID(ctx)
 	if !ok {
@@ -204,6 +207,8 @@ func (t *FilesystemToolset) handleReadFile(ctx context.Context, toolCall tools.T
 	resp, err := t.agent.conn.ReadTextFile(ctx, acp.ReadTextFileRequest{
 		SessionId: acp.SessionId(sessionID),
 		Path:      resolvedPath,
+		Line:      args.Line,
+		Limit:     args.Limit,
 	})
 	if err != nil {
 		return tools.ResultError(fmt.Sprintf("Error reading file: %s", err)), nil

pkg/acp/filesystem_test.go

@@ -1,13 +1,23 @@
 package acp
 
 import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"runtime"
+	"sync"
 	"testing"
+	"time"
 
+	acpsdk "github.com/coder/acp-go-sdk"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/docker/docker-agent/pkg/tools"
+	"github.com/docker/docker-agent/pkg/tools/builtin/filesystem"
 )
 
 func TestResolvePath(t *testing.T) {
@@ -205,3 +215,186 @@ func TestResolvePath_NonExistentPathWithSymlinkAncestor(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "escapes the working directory")
 }
+
+// readTextFileResponder plays the ACP client side of the connection for
+// fs/read_text_file requests: each decoded request is recorded and answered
+// with the configured content. Any other JSON-RPC request fails the test
+// immediately instead of deadlocking the sender.
+type readTextFileResponder struct {
+	t       *testing.T
+	peer    io.Writer // write half of the connection's inbound peer pipe
+	content string
+
+	mu       sync.Mutex
+	requests []acpsdk.ReadTextFileRequest
+}
+
+func (p *readTextFileResponder) Write(b []byte) (int, error) {
+	var msg struct {
+		ID     json.RawMessage `json:"id"`
+		Method string          `json:"method"`
+		Params json.RawMessage `json:"params"`
+	}
+	if err := json.Unmarshal(b, &msg); err != nil {
+		p.t.Errorf("peer received malformed JSON-RPC message %q: %v", b, err)
+		return 0, err
+	}
+	if len(msg.ID) == 0 || msg.Method != acpsdk.ClientMethodFsReadTextFile {
+		err := fmt.Errorf("peer cannot answer JSON-RPC message %q (id %s)", msg.Method, msg.ID)
+		p.t.Error(err)
+		return 0, err
+	}
+
+	var req acpsdk.ReadTextFileRequest
+	if err := json.Unmarshal(msg.Params, &req); err != nil {
+		p.t.Errorf("peer failed to decode %s params: %v", msg.Method, err)
+		return 0, err
+	}
+	p.mu.Lock()
+	p.requests = append(p.requests, req)
+	p.mu.Unlock()
+
+	response, err := json.Marshal(struct {
+		JSONRPC string          `json:"jsonrpc"`
+		ID      json.RawMessage `json:"id"`
+		Result  any             `json:"result"`
+	}{JSONRPC: "2.0", ID: msg.ID, Result: acpsdk.ReadTextFileResponse{Content: p.content}})
+	if err != nil {
+		return 0, fmt.Errorf("marshal response: %w", err)
+	}
+	if _, err := p.peer.Write(append(response, '\n')); err != nil {
+		return 0, err
+	}
+	return len(b), nil
+}
+
+func (p *readTextFileResponder) recordedRequests() []acpsdk.ReadTextFileRequest {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return append([]acpsdk.ReadTextFileRequest(nil), p.requests...)
+}
+
+// TestFilesystemToolset_ReadFileForwardsLineRange verifies that the ACP
+// read_file override forwards the optional line/limit arguments to the
+// client's fs/read_text_file request and leaves them unset for a path-only
+// call.
+func TestFilesystemToolset_ReadFileForwardsLineRange(t *testing.T) {
+	t.Parallel()
+
+	workingDir := t.TempDir()
+	const sessionID = "read-range-session"
+
+	acpAgent := &Agent{
+		sessions: map[string]*Session{sessionID: {id: sessionID, workingDir: workingDir}},
+		clientFS: acpsdk.FileSystemCapabilities{ReadTextFile: true},
+	}
+
+	peerReader, peerWriter := io.Pipe()
+	responder := &readTextFileResponder{t: t, peer: peerWriter, content: "two\nthree\n"}
+	conn := acpsdk.NewAgentSideConnection(acpAgent, responder, peerReader)
+	conn.SetLogger(slog.New(slog.DiscardHandler))
+	acpAgent.SetAgentConnection(conn)
+	t.Cleanup(func() {
+		_ = peerWriter.Close()
+		select {
+		case <-conn.Done():
+		case <-time.After(5 * time.Second):
+			t.Error("timed out waiting for ACP connection shutdown")
+		}
+	})
+
+	ts := NewFilesystemToolset(acpAgent, workingDir)
+	ctx := withSessionID(t.Context(), sessionID)
+
+	result, err := ts.handleReadFile(ctx, tools.ToolCall{
+		Function: tools.FunctionCall{
+			Name:      filesystem.ToolNameReadFile,
+			Arguments: `{"path": "notes.txt", "line": 2, "limit": 2}`,
+		},
+	}, nil)
+	require.NoError(t, err)
+	require.False(t, result.IsError, result.Output)
+	assert.Equal(t, "two\nthree\n", result.Output)
+
+	result, err = ts.handleReadFile(ctx, tools.ToolCall{
+		Function: tools.FunctionCall{
+			Name:      filesystem.ToolNameReadFile,
+			Arguments: `{"path": "notes.txt"}`,
+		},
+	}, nil)
+	require.NoError(t, err)
+	require.False(t, result.IsError, result.Output)
+
+	reqs := responder.recordedRequests()
+	require.Len(t, reqs, 2)
+
+	ranged := reqs[0]
+	assert.Equal(t, acpsdk.SessionId(sessionID), ranged.SessionId)
+	assert.Equal(t, "notes.txt", filepath.Base(ranged.Path))
+	assert.True(t, filepath.IsAbs(ranged.Path), "ACP read requests must carry absolute paths")
+	require.NotNil(t, ranged.Line)
+	assert.Equal(t, 2, *ranged.Line)
+	require.NotNil(t, ranged.Limit)
+	assert.Equal(t, 2, *ranged.Limit)
+
+	pathOnly := reqs[1]
+	assert.Nil(t, pathOnly.Line, "path-only read must not invent a line")
+	assert.Nil(t, pathOnly.Limit, "path-only read must not invent a limit")
+}
+
+// TestFilesystemToolset_ReadFileRejectsInvalidRange verifies that the ACP
+// read_file override applies the same line/limit contract as the builtin
+// filesystem toolset: explicitly invalid values are rejected with a tool
+// error before any fs/read_text_file request reaches the client.
+func TestFilesystemToolset_ReadFileRejectsInvalidRange(t *testing.T) {
+	t.Parallel()
+
+	workingDir := t.TempDir()
+	const sessionID = "invalid-range-session"
+
+	acpAgent := &Agent{
+		sessions: map[string]*Session{sessionID: {id: sessionID, workingDir: workingDir}},
+		clientFS: acpsdk.FileSystemCapabilities{ReadTextFile: true},
+	}
+
+	peerReader, peerWriter := io.Pipe()
+	responder := &readTextFileResponder{t: t, peer: peerWriter, content: "unreachable"}
+	conn := acpsdk.NewAgentSideConnection(acpAgent, responder, peerReader)
+	conn.SetLogger(slog.New(slog.DiscardHandler))
+	acpAgent.SetAgentConnection(conn)
+	t.Cleanup(func() {
+		_ = peerWriter.Close()
+		select {
+		case <-conn.Done():
+		case <-time.After(5 * time.Second):
+			t.Error("timed out waiting for ACP connection shutdown")
+		}
+	})
+
+	ts := NewFilesystemToolset(acpAgent, workingDir)
+	ctx := withSessionID(t.Context(), sessionID)
+
+	for _, tc := range []struct {
+		name      string
+		arguments string
+		wantErr   string
+	}{
+		{"zero line", `{"path": "notes.txt", "line": 0}`, "invalid line 0"},
+		{"negative line", `{"path": "notes.txt", "line": -3}`, "invalid line -3"},
+		{"zero limit", `{"path": "notes.txt", "limit": 0}`, "invalid limit 0"},
+		{"negative limit", `{"path": "notes.txt", "limit": -1}`, "invalid limit -1"},
+	} {
+		result, err := ts.handleReadFile(ctx, tools.ToolCall{
+			Function: tools.FunctionCall{
+				Name:      filesystem.ToolNameReadFile,
+				Arguments: tc.arguments,
+			},
+		}, nil)
+		require.NoError(t, err, tc.name)
+		require.NotNil(t, result, tc.name)
+		assert.True(t, result.IsError, tc.name)
+		assert.Contains(t, result.Output, tc.wantErr, tc.name)
+	}
+
+	assert.Empty(t, responder.recordedRequests(), "invalid ranges must be rejected before any RPC")
+}