feat(core): improve read tool parity (#39126)

packages/cli/src/acp/tool.ts

@@ -44,6 +44,7 @@ export function toLocations(toolName: string, input: ToolInput, cwd?: string): T
       return workdir ? [{ path: workdir }] : []
     }
     case "read":
+      return locationFrom(input.path)
     case "edit":
     case "write":
     case "patch":

packages/cli/test/acp/tool.test.ts

@@ -28,7 +28,7 @@ describe("acp tools", () => {
   })
 
   test("extracts file locations from tool input", () => {
-    expect(toLocations("read", { filePath: "/tmp/a.ts" })).toEqual([{ path: "/tmp/a.ts" }])
+    expect(toLocations("read", { path: "/tmp/a.ts" })).toEqual([{ path: "/tmp/a.ts" }])
     expect(toLocations("edit", { filePath: "/tmp/b.ts" })).toEqual([{ path: "/tmp/b.ts" }])
     expect(toLocations("write", { filePath: "/tmp/c.ts" })).toEqual([{ path: "/tmp/c.ts" }])
     expect(toLocations("grep", { path: "/repo/src" })).toEqual([{ path: "/repo/src" }])
@@ -43,7 +43,7 @@ describe("acp tools", () => {
     ])
     expect(toLocations("bash", { command: "pwd", workdir: "/abs/dir" }, "/workspace")).toEqual([{ path: "/abs/dir" }])
     expect(toLocations("bash", { command: "printf hello" })).toEqual([])
-    expect(toLocations("read", { path: "/tmp/missing-file-path.ts" })).toEqual([])
+    expect(toLocations("read", { path: "/tmp/missing-file-path.ts" })).toEqual([{ path: "/tmp/missing-file-path.ts" }])
   })
 
   test("builds completed content with text and image attachments", () => {
@@ -205,12 +205,13 @@ describe("acp tools", () => {
       runningToolUpdate({
         toolCallId: "call",
         toolName: "read",
-        state: { input: { filePath: "/tmp/a" } },
+        state: { input: { path: "/tmp/a" } },
         content: [{ type: "text", text: "done" }],
       }),
     ).toMatchObject({
       toolCallId: "call",
       status: "in_progress",
+      locations: [{ path: "/tmp/a" }],
       content: [{ type: "content", content: { type: "text", text: "done" } }],
     })
   })
@@ -273,7 +274,7 @@ describe("acp tools", () => {
       errorToolUpdate({
         toolCallId: "call",
         toolName: "read",
-        input: { filePath: "/tmp/a" },
+        input: { path: "/tmp/a" },
         content: [{ type: "text", text: "partial output" }],
         metadata: { path: "/tmp/a" },
         error: "failed",
@@ -284,7 +285,7 @@ describe("acp tools", () => {
       kind: "read",
       title: "read",
       locations: [{ path: "/tmp/a" }],
-      rawInput: { filePath: "/tmp/a" },
+      rawInput: { path: "/tmp/a" },
       content: [
         { type: "content", content: { type: "text", text: "partial output" } },
         { type: "content", content: { type: "text", text: "failed" } },

packages/core/src/location-mutation.ts

@@ -120,6 +120,8 @@ const layer = Layer.effect(
 
     const resolve = Effect.fn("LocationMutation.resolve")(function* (input: ResolveInput) {
       const absolute = path.resolve(location.directory, input.path)
+      // External access follows the requested path boundary. Symlinks reached through an
+      // internal path intentionally retain internal permission semantics after canonicalization.
       const lexicallyInternal = FSUtil.contains(location.directory, absolute)
 
       const resolved = yield* resolvePath(absolute)

packages/core/src/tool/plugin/read.ts

@@ -14,7 +14,7 @@ import { ReadToolFileSystem } from "../read-filesystem"
 
 export const name = "read"
 const FILENAME = "AGENTS.md"
-const SUPPORTED_IMAGE_MIMES = new Set(["image/jpeg", "image/png", "image/gif", "image/webp"])
+const SUPPORTED_MEDIA_MIMES = new Set(["image/jpeg", "image/png", "image/gif", "image/webp", "application/pdf"])
 const LocationInput = Schema.Struct({
   path: Schema.String,
   offset: ReadToolFileSystem.PageInput.fields.offset.annotate({
@@ -110,7 +110,7 @@ export const Plugin = {
                   Effect.catch(() => Effect.void),
                   Effect.catchDefect(() => Effect.void),
                 )
-                if (content.type === "file" && content.encoding === "base64" && !SUPPORTED_IMAGE_MIMES.has(content.mime))
+                if (content.type === "file" && content.encoding === "base64" && !SUPPORTED_MEDIA_MIMES.has(content.mime))
                   return yield* Effect.fail(new ReadToolFileSystem.BinaryFileError({ resource }))
                 return content
               }).pipe(
@@ -119,9 +119,13 @@ export const Plugin = {
                   // unresized copy in model text.
                   const content =
                     output.type === "file" && output.encoding === "base64"
-                      ? SUPPORTED_IMAGE_MIMES.has(output.mime)
+                      ? SUPPORTED_MEDIA_MIMES.has(output.mime)
                         ? ([
-                            { type: "text", text: "Image read successfully" },
+                            {
+                              type: "text",
+                              text:
+                                output.mime === "application/pdf" ? "PDF read successfully" : "Image read successfully",
+                            },
                             {
                               type: "file",
                               uri: `data:${output.mime};base64,${output.content}`,
@@ -136,7 +140,10 @@ export const Plugin = {
                 Effect.mapError((error) => {
                   const message =
                     error instanceof ReadToolFileSystem.BinaryFileError ||
-                    error instanceof ReadToolFileSystem.MediaIngestLimitError
+                    error instanceof ReadToolFileSystem.MediaIngestLimitError ||
+                    error instanceof ReadToolFileSystem.MalformedUtf8Error ||
+                    error instanceof ReadToolFileSystem.OffsetOutOfRangeError ||
+                    error instanceof ReadToolFileSystem.PathKindError
                       ? error.message
                       : `Unable to read ${input.path}`
                   return new ToolFailure({ message, error })

packages/core/src/tool/read-filesystem.ts

@@ -140,12 +140,13 @@ const extensions = new Set([
   ".pyo",
 ])
 const startsWith = (bytes: Uint8Array, prefix: number[]) => prefix.every((value, index) => bytes[index] === value)
-const imageMime = (bytes: Uint8Array) => {
+const mediaMime = (bytes: Uint8Array) => {
   if (startsWith(bytes, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])) return "image/png"
   if (startsWith(bytes, [0xff, 0xd8, 0xff])) return "image/jpeg"
   if (startsWith(bytes, [0x47, 0x49, 0x46, 0x38])) return "image/gif"
   if (startsWith(bytes, [0x52, 0x49, 0x46, 0x46]) && startsWith(bytes.subarray(8), [0x57, 0x45, 0x42, 0x50]))
     return "image/webp"
+  if (startsWith(bytes, [0x25, 0x50, 0x44, 0x46, 0x2d])) return "application/pdf"
 }
 const binary = (resource: string, bytes: Uint8Array) => {
   if (extensions.has(path.extname(resource).toLowerCase())) return true
@@ -191,7 +192,7 @@ export const read = Effect.fn("ReadTool.read")(function* (
         yield* file.readAlloc(Math.min(64 * 1024, Number(info.size) || 4 * 1024)),
         () => new Uint8Array(),
       )
-      const mime = imageMime(first)
+      const mime = mediaMime(first)
       if (mime) {
         if (info.size > MAX_MEDIA_INGEST_BYTES)
           return yield* Effect.fail(new MediaIngestLimitError({ resource, maximumBytes: MAX_MEDIA_INGEST_BYTES }))
@@ -217,7 +218,7 @@ export const read = Effect.fn("ReadTool.read")(function* (
           mime,
         }
       }
-      if (startsWith(first, [0x25, 0x50, 0x44, 0x46]) || extensions.has(path.extname(resource).toLowerCase()))
+      if (extensions.has(path.extname(resource).toLowerCase()))
         return yield* Effect.fail(new BinaryFileError({ resource }))
       const paged = info.size > MAX_READ_BYTES || page.offset !== undefined || page.limit !== undefined
       if (!paged) {

packages/core/test/tool-read-filesystem.test.ts

@@ -115,4 +115,21 @@ describe("ReadToolFileSystem", () => {
       )
     }),
   )
+
+  it.effect("reads PDFs as bounded media", () =>
+    Effect.gen(function* () {
+      const { fs, files, directory } = yield* fixture
+      const file = path.join(directory, "document.pdf")
+      yield* files.writeFileString(file, "%PDF-1.7\ncontent")
+
+      const result = yield* ReadToolFileSystem.read(fs, file, "document.pdf")
+
+      expect(result).toMatchObject({
+        type: "file",
+        content: Buffer.from("%PDF-1.7\ncontent").toString("base64"),
+        encoding: "base64",
+        mime: "application/pdf",
+      })
+    }),
+  )
 })

packages/core/test/tool-read.test.ts

@@ -540,6 +540,35 @@ describe("ReadTool", () => {
     }),
   )
 
+  it.effect("returns PDFs as native media", () =>
+    Effect.gen(function* () {
+      const pdf = "JVBERi0xLjcK"
+      readResult = {
+        type: "file",
+        uri: "file:///document.pdf",
+        name: "document.pdf",
+        content: pdf,
+        encoding: "base64",
+        mime: "application/pdf",
+      }
+      const registry = yield* Tool.Service
+
+      expect(
+        yield* executeTool(registry, {
+          sessionID,
+          ...toolIdentity,
+          call: { type: "tool-call", id: "call-pdf", name: "read", input: { path: "document.pdf" } },
+        }),
+      ).toMatchObject({
+        status: "completed",
+        content: [
+          { type: "text", text: "PDF read successfully" },
+          { type: "file", uri: `data:application/pdf;base64,${pdf}`, mime: "application/pdf", name: "document.pdf" },
+        ],
+      })
+    }),
+  )
+
   it.effect("returns expected filesystem failures to the model", () =>
     Effect.gen(function* () {
       readFailure = new ReadToolFileSystem.BinaryFileError({ resource: "archive.dat" })
@@ -563,6 +592,32 @@ describe("ReadTool", () => {
     }),
   )
 
+  it.effect("preserves actionable read failure messages", () =>
+    Effect.gen(function* () {
+      const registry = yield* Tool.Service
+      for (const [error, message] of [
+        [
+          new ReadToolFileSystem.MalformedUtf8Error({ resource: "invalid.txt" }),
+          "File is not valid UTF-8: invalid.txt",
+        ],
+        [new ReadToolFileSystem.OffsetOutOfRangeError({ offset: 10 }), "Offset 10 is out of range"],
+        [
+          new ReadToolFileSystem.PathKindError({ resource: "socket", expected: "a file" }),
+          "Path is not a file: socket",
+        ],
+      ] as const) {
+        readFailure = error
+        expect(
+          yield* executeTool(registry, {
+            sessionID,
+            ...toolIdentity,
+            call: { type: "tool-call", id: `call-${error._tag}`, name: "read", input: { path: "target" } },
+          }),
+        ).toEqual({ status: "error", error: { type: "unknown", message } })
+      }
+    }),
+  )
+
   it.effect("preserves unexpected filesystem defects", () =>
     Effect.gen(function* () {
       resolveFailure = new Error("unexpected")