From c9789de2bdebfe3595166b8e678ccac84c5b3806 Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Mon, 10 Aug 2026 19:14:09 +0200 Subject: [PATCH] toolsrv: fix ReadAt offset for request-response files After Write(), the file offset is at the end of the written data. For request-response files (like proc/new), we need to read the result from offset 0, not from the current offset. Changed CallTool() to use ReadAt(buf, 0) instead of io.ReadAll() which uses Read() and inherits the wrong offset. Same fix was already applied to token reading in Dial(). Added TestIntegration_ToolExecution to verify the full tool execution path works end-to-end. --- toolsrv/client9p.go | 19 +++++++++++--- toolsrv/integration_test.go | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/toolsrv/client9p.go b/toolsrv/client9p.go index 5ddb8f0..f632d59 100644 --- a/toolsrv/client9p.go +++ b/toolsrv/client9p.go @@ -148,9 +148,22 @@ func (c *Conn) CallTool(ctx context.Context, name string, args json.RawMessage) return nil, fmt.Errorf("write: %w", err) } - result, err := io.ReadAll(fid) - if err != nil { - return nil, fmt.Errorf("read: %w", err) + // Read result from offset 0 (Write advances the file offset, but + // for request-response files we need to read the result from the start) + var result []byte + buf := make([]byte, 8192) + for offset := int64(0); ; { + n, err := fid.ReadAt(buf, offset) + if n > 0 { + result = append(result, buf[:n]...) + offset += int64(n) + } + if err == io.EOF || n == 0 { + break + } + if err != nil { + return nil, fmt.Errorf("read: %w", err) + } } return json.RawMessage(result), nil diff --git a/toolsrv/integration_test.go b/toolsrv/integration_test.go index 4b543f8..4a248fc 100644 --- a/toolsrv/integration_test.go +++ b/toolsrv/integration_test.go @@ -6,10 +6,12 @@ package toolsrv import ( "context" + "encoding/json" "fmt" "os" "os/exec" "path/filepath" + "strings" "testing" "time" ) @@ -208,6 +210,53 @@ func TestIntegration_BasicOperations(t *testing.T) { t.Logf("ListTools returned %d tools", len(tools)) } +func TestIntegration_ToolExecution(t *testing.T) { + socketPath, cleanup := startTestServer(t) + defer cleanup() + + conn, err := Dial(socketPath, "test-secret-exec") + if err != nil { + t.Fatalf("Dial failed: %v", err) + } + defer conn.Close() + + // Load the shell tool + if err := conn.LoadTool("shell"); err != nil { + t.Fatalf("LoadTool failed: %v", err) + } + + // Verify it's loaded + tools, err := conn.ListTools() + if err != nil { + t.Fatalf("ListTools failed: %v", err) + } + found := false + for _, tool := range tools { + if tool.Name == "shell" { + found = true + break + } + } + if !found { + t.Fatal("shell tool not found after loading") + } + + // Execute a tool + ctx := context.Background() + args := json.RawMessage(`{"cmd": "echo hello from test"}`) + result, err := conn.CallTool(ctx, "shell", args) + if err != nil { + t.Fatalf("CallTool failed: %v", err) + } + + // Result should be JSON containing the output + resultStr := string(result) + if !strings.Contains(resultStr, "hello from test") { + t.Errorf("unexpected result: %s", resultStr) + } + t.Logf("Tool result: %s", resultStr) +} + func TestIntegration_ProcessKeeperReconnect(t *testing.T) { socketPath, cleanup := startTestServer(t) defer cleanup()