fix(read): treat utf-8 pdf names as text and quote pdftotext (#1238)

* fix(read): lead file-inspection diagnosis with the cause

Collapsed tool rows clip to 72 characters, so a long path buried the
gap. The first sentence now names missing extractor, permission
boundary, or malformed file, and the message uses the call path.

* fix(read): treat utf-8 pdf names as text and quote pdftotext

A .pdf suffix without %PDF magic is only malformed when the first chunk
is binary. extractor_ready quotes the path so it pastes into bash.

* test(read): expect quoted call-path in extractor_ready diagnosis

Author: TheGreatAxios

src/agent/tool-aliases.ts

@@ -141,7 +141,7 @@ export function withAuthzParityDefinitions<
 
 const SHELL_WRAPPERS = new Set(["bash", "sh", "zsh"]);
 
-function shellQuote(arg: string): string {
+export function shellQuote(arg: string): string {
   if (/^[A-Za-z0-9_\-./:=@%]+$/.test(arg)) return arg;
   return `'${arg.replace(/'/g, `'\\''`)}'`;
 }

src/plugins/file-inspection-diagnosis.test.ts

@@ -17,14 +17,17 @@ describe("PDF sniffing", () => {
     ).toBe("pdf");
   });
 
-  test("a .pdf name without magic is malformed, not a capability miss", () => {
+  test("a .pdf name with NUL and no magic is malformed; a text .pdf is not refused", () => {
     expect(pathLooksLikePdf("report.PDF")).toBe(true);
     expect(
       inspectionKindFromFirstChunk("report.pdf", Buffer.from("not-a-pdf\0")),
     ).toBe("pdf-malformed");
+  });
+
+  test("plain UTF-8 named .pdf without magic is not an inspection block", () => {
     expect(
       inspectionKindFromFirstChunk("report.pdf", Buffer.from("plain text")),
-    ).toBe("pdf-malformed");
+    ).toBeUndefined();
   });
 
   test("NUL without PDF markers is ordinary binary", () => {
@@ -51,7 +54,7 @@ describe("diagnoseBlockedFileInspection", () => {
     expect(result.code).toBe("missing_extractor");
     expect(result.message).toContain("PDF");
     expect(result.message).toContain(PDF_EXTRACTOR_BIN);
-    expect(result.message).toContain("missing");
+    expect(result.message).toContain("Missing PDF extractor");
     expect(result.message.toLowerCase()).toContain("poppler");
     expect(result.message).toContain("not a malformed file");
     expect(result.message).not.toContain("permission boundary");
@@ -76,7 +79,7 @@ describe("diagnoseBlockedFileInspection", () => {
     });
     expect(result.code).toBe("permission_boundary");
     expect(result.message).toContain("installed");
-    expect(result.message).toContain("permission boundary");
+    expect(result.message).toContain("Permission boundary");
     expect(result.message).toContain("run_shell");
     expect(result.message).toContain("not a malformed file");
     expect(result.message.toLowerCase()).not.toContain("brew install");
@@ -94,6 +97,18 @@ describe("diagnoseBlockedFileInspection", () => {
     expect(result.message).toContain(`${PDF_EXTRACTOR_BIN} ${PATH} -`);
   });
 
+  test("extractor_ready quotes a path that would break a pasted bash command", () => {
+    const spaced = "/tmp/my report.pdf";
+    const result = diagnoseBlockedFileInspection({
+      path: spaced,
+      kind: "pdf",
+      extractorAvailable: true,
+      canExecuteHostCommands: true,
+    });
+    expect(result.code).toBe("extractor_ready");
+    expect(result.message).toContain(`${PDF_EXTRACTOR_BIN} '${spaced}' -`);
+  });
+
   test("malformed PDF is not blamed on tooling or permission", () => {
     const result = diagnoseBlockedFileInspection({
       path: PATH,
@@ -102,7 +117,7 @@ describe("diagnoseBlockedFileInspection", () => {
       canExecuteHostCommands: false,
     });
     expect(result.code).toBe("malformed");
-    expect(result.message).toContain("not a valid PDF");
+    expect(result.message).toContain("Malformed PDF");
     expect(result.message).toContain("not a missing extractor");
     expect(result.message).not.toContain("brew install");
     expect(result.message).not.toContain("grant this worker");
@@ -116,7 +131,7 @@ describe("diagnoseBlockedFileInspection", () => {
       canExecuteHostCommands: true,
     });
     expect(result.code).toBe("unreadable");
-    expect(result.message).toContain("unreadable");
+    expect(result.message).toContain("Unreadable file");
     expect(result.message).toContain("filesystem permission");
     expect(result.message).not.toContain("grant this worker");
     expect(result.message.toLowerCase()).not.toContain("poppler");
@@ -131,6 +146,34 @@ describe("diagnoseBlockedFileInspection", () => {
     });
     expect(result.code).toBe("binary");
     expect(result.message).toContain("refusing to read binary file");
-    expect(result.message).toContain("not a missing-tool");
+    expect(result.message).toContain("Not a missing-tool");
+  });
+
+  test("the cause leads so a 72-character collapsed preview still names the gap", () => {
+    const missing = diagnoseBlockedFileInspection({
+      path: "/very/long/workspace/path/to/a/nested/report.pdf",
+      kind: "pdf",
+      extractorAvailable: false,
+      canExecuteHostCommands: true,
+    });
+    expect(missing.message.slice(0, 72).toLowerCase()).toContain("extractor");
+
+    const permission = diagnoseBlockedFileInspection({
+      path: "/very/long/workspace/path/to/a/nested/report.pdf",
+      kind: "pdf",
+      extractorAvailable: true,
+      canExecuteHostCommands: false,
+    });
+    expect(permission.message.slice(0, 72).toLowerCase()).toContain(
+      "permission",
+    );
+
+    const malformed = diagnoseBlockedFileInspection({
+      path: "/very/long/workspace/path/to/a/nested/report.pdf",
+      kind: "pdf-malformed",
+      extractorAvailable: false,
+      canExecuteHostCommands: false,
+    });
+    expect(malformed.message.slice(0, 72).toLowerCase()).toContain("malformed");
   });
 });

src/plugins/file-inspection-diagnosis.ts

@@ -1,4 +1,5 @@
 import { extname } from "node:path";
+import { shellQuote } from "../agent/tool-aliases.js";
 
 /** Host binary that extracts text from a PDF (poppler). */
 export const PDF_EXTRACTOR_BIN = "pdftotext";
@@ -45,16 +46,18 @@ export function pathLooksLikePdf(filePath: string): boolean {
 
 /**
  * Classify the first streamed chunk. PDF magic wins even without a .pdf
- * suffix. A .pdf name without magic is malformed, whether or not the chunk
- * looks binary. NUL without PDF markers is an ordinary binary file.
+ * suffix. A .pdf name with NUL but no magic is malformed. A .pdf that is
+ * otherwise valid UTF-8 is left as a normal text read so a misnamed text
+ * file is not a new refusal.
  */
 export function inspectionKindFromFirstChunk(
   filePath: string,
   chunk: Uint8Array,
 ): FileInspectionKind | undefined {
   if (chunkLooksLikePdf(chunk)) return "pdf";
-  if (pathLooksLikePdf(filePath)) return "pdf-malformed";
-  if (chunk.includes(0)) return "binary";
+  if (chunk.includes(0)) {
+    return pathLooksLikePdf(filePath) ? "pdf-malformed" : "binary";
+  }
   return undefined;
 }
 
@@ -76,25 +79,25 @@ export function diagnoseBlockedFileInspection(
     return {
       code: "unreadable",
       message:
-        `Cannot inspect ${path}: the file is unreadable (filesystem permission denied), not a missing extractor or worker-tool gap. ` +
-        `Check ownership and mode, or copy the file into a readable workspace path.`,
+        `Unreadable file (filesystem permission denied), not a missing extractor or worker-tool gap. ` +
+        `Check ownership and mode, or copy it into a readable workspace path. File: ${path}`,
     };
   }
 
   if (input.kind === "pdf-malformed") {
     return {
       code: "malformed",
       message:
-        `Cannot inspect ${path}: the file is not a valid PDF (malformed or unreadable bytes), not a missing extractor or permission boundary. ` +
-        `Open it in a PDF reader or re-export it, then retry.`,
+        `Malformed PDF, not a missing extractor or permission boundary. ` +
+        `Open it in a PDF reader or re-export it. File: ${path}`,
     };
   }
 
   if (input.kind === "binary") {
     return {
       code: "binary",
       message:
-        `refusing to read binary file: ${path}. This is a binary file, not a missing-tool or permission failure. ` +
+        `refusing to read binary file: ${path}. Not a missing-tool or permission failure. ` +
         `Use a format-specific extractor if you need text from it.`,
     };
   }
@@ -104,24 +107,22 @@ export function diagnoseBlockedFileInspection(
     return {
       code: "missing_extractor",
       message:
-        `Cannot inspect PDF ${path}: ${extractor} is not installed (missing PDF-extraction capability), not a malformed file. ` +
-        `Install poppler so ${extractor} is on PATH (for example \`brew install poppler\` or \`apt-get install poppler-utils\`), then retry.`,
+        `Missing PDF extractor: ${extractor} is not on PATH (capability gap), not a malformed file. ` +
+        `Install poppler (\`brew install poppler\` or \`apt-get install poppler-utils\`), then retry. File: ${path}`,
     };
   }
 
   if (!input.canExecuteHostCommands) {
     return {
       code: "permission_boundary",
       message:
-        `Cannot inspect PDF ${path}: ${extractor} is installed, but this worker cannot execute it (run_shell is not mounted — a permission boundary, not a malformed file). ` +
-        `Re-dispatch to a director that mounts run_shell, or grant this worker run_shell, then retry.`,
+        `Permission boundary: ${extractor} is installed but this worker cannot execute it (run_shell is not mounted), not a malformed file. ` +
+        `Re-dispatch to a director that mounts run_shell, or grant this worker run_shell. File: ${path}`,
     };
   }
 
   return {
     code: "extractor_ready",
-    message:
-      `Cannot inspect PDF ${path} as text via read_file. ${extractor} is installed and this session can run host commands. ` +
-      `Run \`${extractor} ${path} -\` via bash to extract text.`,
+    message: `read_file cannot decode PDFs. ${extractor} is installed — run \`${extractor} ${shellQuote(path)} -\` via bash.`,
   };
 }

src/plugins/read-file-guard-plugin.test.ts

@@ -126,7 +126,7 @@ describe("readFileBounded", () => {
     );
     expect(isError).toBe(true);
     expect(content).toContain("binary");
-    expect(content).toContain("not a missing-tool");
+    expect(content).toContain("Not a missing-tool");
   });
 
   test("a PDF without pdftotext names the missing extractor, not a malformed file", async () => {
@@ -140,7 +140,7 @@ describe("readFileBounded", () => {
     );
     expect(isError).toBe(true);
     expect(content).toContain("pdftotext");
-    expect(content).toContain("missing");
+    expect(content).toContain("Missing PDF extractor");
     expect(content).toContain("not a malformed file");
     expect(content).not.toContain("permission boundary");
   });
@@ -159,7 +159,7 @@ describe("readFileBounded", () => {
     );
     expect(isError).toBe(true);
     expect(content).toContain("installed");
-    expect(content).toContain("permission boundary");
+    expect(content).toContain("Permission boundary");
     expect(content).toContain("run_shell");
     expect(content).toContain("not a malformed file");
   });
@@ -174,11 +174,27 @@ describe("readFileBounded", () => {
       { whichExtractor: () => null, canExecuteHostCommands: () => false },
     );
     expect(isError).toBe(true);
-    expect(content).toContain("not a valid PDF");
+    expect(content).toContain("Malformed PDF");
     expect(content).toContain("not a missing extractor");
     expect(content).not.toContain("brew install");
   });
 
+  test("UTF-8 .pdf without magic still reads as text", async () => {
+    const p = await fixture("utf8.pdf", "plain utf-8 notes\nsecond line");
+    const { content, isError } = await readFileBounded(
+      p,
+      0,
+      2000,
+      neverAbort(),
+      { whichExtractor: () => null, canExecuteHostCommands: () => false },
+    );
+    expect(isError).toBeUndefined();
+    expect(content).toContain("plain utf-8 notes");
+    expect(content).toContain("second line");
+    expect(content).not.toContain("not a valid PDF");
+    expect(content).not.toContain("malformed");
+  });
+
   test("a NUL deep in an otherwise-valid file does not discard streamed content", async () => {
     // Enough valid text (>64KB) to guarantee the NUL lands in a later chunk.
     const head = Array.from(
@@ -807,8 +823,9 @@ describe("readFileGuardPlugin", () => {
     );
     expect(missingResult.isError).toBe(true);
     expect(String(missingResult.content)).toContain("pdftotext");
-    expect(String(missingResult.content)).toContain("missing");
+    expect(String(missingResult.content)).toContain("Missing PDF extractor");
     expect(String(missingResult.content)).toContain("not a malformed file");
+    expect(String(missingResult.content)).toContain("flow.pdf");
 
     const blocked = readFileGuardPlugin(dir, {
       whichExtractor: () => "/opt/homebrew/bin/pdftotext",
@@ -820,10 +837,41 @@ describe("readFileGuardPlugin", () => {
     );
     expect(blockedResult.isError).toBe(true);
     expect(String(blockedResult.content)).toContain("installed");
-    expect(String(blockedResult.content)).toContain("permission boundary");
+    expect(String(blockedResult.content)).toContain("Permission boundary");
     expect(String(blockedResult.content)).toContain("run_shell");
     expect(String(blockedResult.content)).not.toContain("brew install");
   });
+
+  test("UTF-8 .pdf without magic still reads as text through the plugin", async () => {
+    await fixture("notes.pdf", "hello from a misnamed text file");
+    const result = await run({
+      id: "utf8-pdf",
+      name: "read_file",
+      arguments: { path: "notes.pdf" },
+    });
+    expect(result.isError).toBeFalsy();
+    expect(result.content).toContain("hello from a misnamed text file");
+    expect(String(result.content)).not.toContain("not a valid PDF");
+    expect(String(result.content)).not.toContain("malformed");
+  });
+
+  test("extractor_ready quotes a PDF path that contains spaces", async () => {
+    await fixture("my report.pdf", Buffer.from("%PDF-1.4\n"));
+    const plugin = readFileGuardPlugin(dir, {
+      whichExtractor: () => "/usr/bin/pdftotext",
+      canExecuteHostCommands: () => true,
+    });
+    const result = await defined(plugin.middleware)(fallback)(
+      {
+        id: "quoted-pdf",
+        name: "read_file",
+        arguments: { path: "my report.pdf" },
+      },
+      neverAbort(),
+    );
+    expect(result.isError).toBe(true);
+    expect(String(result.content)).toContain(`pdftotext 'my report.pdf' -`);
+  });
 });
 
 describe("CL-8980 single-way path+offset resume", () => {

src/plugins/read-file-guard-plugin.ts

@@ -60,6 +60,8 @@ export interface ReadFileGuardPluginOptions {
    * Workers flip this after the capability filter; omitted means yes.
    */
   canExecuteHostCommands?: () => boolean;
+  /** Model-facing path for diagnosis text (the call argument, not the absolute). */
+  displayPath?: string;
 }
 
 // A truncated read tells the model to continue with the same path and the
@@ -366,23 +368,24 @@ export function readFileBounded(
   signal: AbortSignal,
   inspection?: Pick<
     ReadFileGuardPluginOptions,
-    "whichExtractor" | "canExecuteHostCommands"
+    "whichExtractor" | "canExecuteHostCommands" | "displayPath"
   >,
 ): Promise<BoundedRead> {
+  const labeledPath = inspection?.displayPath ?? absolutePath;
   return readStreamBounded(
     createReadStream(absolutePath),
     absolutePath,
     offset,
     limit,
     signal,
     {
-      mapStreamError: (err) => mapFilesystemStreamError(absolutePath, err),
+      mapStreamError: (err) => mapFilesystemStreamError(labeledPath, err),
       windowHugeLines: true,
       diagnoseFirstChunk: (chunk) => {
         const kind = inspectionKindFromFirstChunk(absolutePath, chunk);
         if (kind === undefined) return undefined;
         return diagnoseBlockedFileInspection({
-          path: absolutePath,
+          path: labeledPath,
           kind,
           extractorAvailable: resolveExtractorProbe(inspection?.whichExtractor),
           canExecuteHostCommands:
@@ -537,7 +540,7 @@ export function readFileGuardPlugin(
           return {
             callId: call.id,
             content: diagnoseBlockedFileInspection({
-              path: rawPath,
+              path: absolutePath,
               kind: "unreadable",
               extractorAvailable: false,
               canExecuteHostCommands: false,
@@ -549,13 +552,10 @@ export function readFileGuardPlugin(
       }
 
       try {
-        const res = await readFileBounded(
-          absolutePath,
-          offset,
-          limit,
-          signal,
-          inspection,
-        );
+        const res = await readFileBounded(absolutePath, offset, limit, signal, {
+          ...inspection,
+          displayPath: rawPath,
+        });
         return res.isError
           ? { callId: call.id, content: res.content, isError: true }
           : { callId: call.id, content: res.content };