feat(coding-agent/tools): implemented pdf page screenshot rendering
- Add `renderPdfPageScreenshot` to render PDF pages via headless Chromium. - Update `ReadTool` to intercept legacy PDF image paths and return page screenshots. - Ensure daemon clients are closed on read command exit.
This commit is contained in:
@@ -5,15 +5,12 @@
|
||||
### Changed
|
||||
|
||||
- Replaced the MuPDF-WASM PDF document backend with `pdf-inspector` through `@oh-my-pi/pi-natives`, preserving cached text conversion and PDF line selectors while reporting pages that need OCR.
|
||||
- Removed `read <pdf>:` image listings and `read <pdf>:<image>.png` extraction because `pdf-inspector` does not rasterize pages; these reads now direct users to the Puppeteer browser tool for rendering or to read the PDF path for extracted text.
|
||||
- Restored `read <pdf>:` and `read <pdf>:<image>.png` page rendering by automatically capturing PDF pages through the headless Chromium browser tool.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed Streamable HTTP MCP sessions being invalidated by opening the optional GET SSE stream before sending `notifications/initialized`, which prevented Figma Dev Mode MCP from connecting ([#8514](https://github.com/can1357/oh-my-pi/issues/8514)).
|
||||
### Fixed
|
||||
|
||||
- Fixed the `/hotkeys` table describing Ctrl+D (`app.exit`) as "Exit (when editor is empty)" when it actually exits unconditionally and saves the current prompt as a resumable draft ([#8530](https://github.com/can1357/oh-my-pi/issues/8530)).
|
||||
### Fixed
|
||||
|
||||
- Fixed Ctrl+G external editors failing to launch on Windows because Bun re-quoted the embedded `cmd.exe /c` command line ([#8544](https://github.com/can1357/oh-my-pi/issues/8544)).
|
||||
|
||||
## [17.3.3] - 2026-08-14
|
||||
|
||||
@@ -10,6 +10,7 @@ import chalk from "@oh-my-pi/pi-utils/chalk";
|
||||
import { Settings } from "../config/settings";
|
||||
import { extractUriScheme } from "../internal-urls/parse";
|
||||
import { InternalUrlRouter } from "../internal-urls/router";
|
||||
import { closeDaemonClients } from "../launch/client";
|
||||
import { discoverAndLoadMCPTools } from "../mcp/loader";
|
||||
import { MCPManager } from "../mcp/manager";
|
||||
import { discoverAuthStorage } from "../session/auth-broker-config";
|
||||
@@ -93,6 +94,7 @@ export async function runReadCommand(cmd: ReadCommandArgs): Promise<void> {
|
||||
if (MCPManager.instance() === mcpManager) MCPManager.setInstance(undefined);
|
||||
}
|
||||
authStorage?.close();
|
||||
await closeDaemonClients();
|
||||
}
|
||||
|
||||
if (failed) process.exit(1);
|
||||
|
||||
@@ -1,13 +1,135 @@
|
||||
const PDF_IMAGE_MEMBER_RE = /^(.*\.pdf):(.*)$/i;
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { untilAborted } from "@oh-my-pi/pi-utils";
|
||||
import type { ToolSession } from "../sdk";
|
||||
import type { BrowserHandle } from "./browser/registry";
|
||||
import type { ScreenshotResult } from "./browser/tab-protocol";
|
||||
import { ToolAbortError, ToolError } from "./tool-errors";
|
||||
|
||||
/** Parse a former PDF image-member read without claiming normal selectors. */
|
||||
export function splitUnsupportedPdfImageReadPath(readPath: string): { pdfPath: string } | null {
|
||||
const PDF_IMAGE_MEMBER_RE = /^(.*\.pdf):(.*)$/i;
|
||||
const PDF_PAGE_MEMBER_RE = /^(?:p|page[-_]?)(\d+)(?:[-_].*)?\.png$/i;
|
||||
const PDF_RENDER_TIMEOUT_MS = 30_000;
|
||||
|
||||
// Chromium's PDF plugin paints in an out-of-process frame after navigation has
|
||||
// completed. Wait for document dimensions, then cross compositor boundaries
|
||||
// before capturing; otherwise the screenshot can contain only the viewer shell.
|
||||
const PDF_SCREENSHOT_CODE = `
|
||||
let viewerFrame;
|
||||
await wait(async () => {
|
||||
for (const frame of page.frames()) {
|
||||
try {
|
||||
const loaded = await frame.evaluate(() => {
|
||||
const viewer = document.querySelector("pdf-viewer");
|
||||
const toolbar = viewer?.shadowRoot?.querySelector("viewer-toolbar");
|
||||
const pageLength = toolbar
|
||||
?.shadowRoot?.querySelector("viewer-page-selector")
|
||||
?.shadowRoot?.querySelector("#pagelength")
|
||||
?.textContent;
|
||||
if (Number(pageLength) > 0 && !toolbar?.hasAttribute("loading_")) return true;
|
||||
|
||||
const plugin = document.querySelector('embed[type="application/x-google-chrome-pdf"]');
|
||||
const sizer = document.querySelector("#sizer");
|
||||
return plugin !== null && sizer !== null && sizer.clientWidth > 0 && sizer.clientHeight > 0;
|
||||
});
|
||||
if (loaded) {
|
||||
viewerFrame = frame;
|
||||
return true;
|
||||
}
|
||||
} catch {}
|
||||
}
|
||||
return false;
|
||||
});
|
||||
await page.screenshot({ type: "png" });
|
||||
await viewerFrame.evaluate(() => {
|
||||
const { promise, resolve } = Promise.withResolvers();
|
||||
requestAnimationFrame(() =>
|
||||
requestAnimationFrame(() =>
|
||||
requestAnimationFrame(() => requestAnimationFrame(resolve)),
|
||||
),
|
||||
);
|
||||
return promise;
|
||||
});
|
||||
return await tab.screenshot({ fullPage: true, silent: true });
|
||||
`;
|
||||
|
||||
/** A legacy PDF image-member path interpreted as a page screenshot request. */
|
||||
export interface PdfImageReadTarget {
|
||||
/** PDF path before the member delimiter. */
|
||||
pdfPath: string;
|
||||
/** Original member text after the delimiter. */
|
||||
member: string;
|
||||
/** One-indexed page inferred from names such as `p2-img0.png`; defaults to page 1. */
|
||||
page: number;
|
||||
}
|
||||
|
||||
/** Parse a former PDF image-member path as a Chromium page screenshot request. */
|
||||
export function splitPdfImageReadPath(readPath: string): PdfImageReadTarget | null {
|
||||
const match = PDF_IMAGE_MEMBER_RE.exec(readPath);
|
||||
const pdfPath = match?.[1];
|
||||
return pdfPath ? { pdfPath } : null;
|
||||
const member = match?.[2];
|
||||
if (!pdfPath || member === undefined) return null;
|
||||
const pageText = PDF_PAGE_MEMBER_RE.exec(member)?.[1];
|
||||
const parsedPage = pageText === undefined ? 1 : Number(pageText);
|
||||
const page = Number.isSafeInteger(parsedPage) && parsedPage > 0 ? parsedPage : 1;
|
||||
return { pdfPath, member, page };
|
||||
}
|
||||
|
||||
/** Explain how to render a PDF now that the text backend has no rasterizer. */
|
||||
export function pdfImageRenderingUnsupportedMessage(pdfPath: string): string {
|
||||
return `pdf-inspector cannot render PDF images. Use the Puppeteer browser tool to render '${pdfPath}', or read '${pdfPath}' for extracted text.`;
|
||||
/** Render one PDF page through the browser tool's shared headless Chromium. */
|
||||
export async function renderPdfPageScreenshot(
|
||||
session: ToolSession,
|
||||
absolutePdfPath: string,
|
||||
page: number,
|
||||
signal?: AbortSignal,
|
||||
): Promise<ScreenshotResult> {
|
||||
const [{ acquireBrowser, holdBrowser, releaseBrowser }, { acquireTab, releaseTab, runInTab }] = await Promise.all([
|
||||
import("./browser/registry"),
|
||||
import("./browser/tab-supervisor"),
|
||||
]);
|
||||
const timeoutSignal = AbortSignal.timeout(PDF_RENDER_TIMEOUT_MS);
|
||||
const renderSignal = signal ? AbortSignal.any([signal, timeoutSignal]) : timeoutSignal;
|
||||
const tabName = `read-pdf-${Bun.randomUUIDv7()}`;
|
||||
const url = pathToFileURL(absolutePdfPath);
|
||||
url.hash = `page=${page}&toolbar=0&navpanes=0&view=Fit`;
|
||||
|
||||
let browserLease = false;
|
||||
let tabOpened = false;
|
||||
let browser: BrowserHandle | undefined;
|
||||
try {
|
||||
const acquiredBrowser = await untilAborted(renderSignal, () =>
|
||||
acquireBrowser({ kind: "headless", headless: true }, { cwd: session.cwd, signal: renderSignal }),
|
||||
);
|
||||
browser = acquiredBrowser;
|
||||
holdBrowser(acquiredBrowser);
|
||||
browserLease = true;
|
||||
await untilAborted(renderSignal, () =>
|
||||
acquireTab(tabName, acquiredBrowser, {
|
||||
url: url.href,
|
||||
waitUntil: "load",
|
||||
timeoutMs: PDF_RENDER_TIMEOUT_MS,
|
||||
signal: renderSignal,
|
||||
ownerSessionId: session.getSessionId?.() ?? undefined,
|
||||
}),
|
||||
);
|
||||
tabOpened = true;
|
||||
await releaseBrowser(acquiredBrowser, { kill: false });
|
||||
browserLease = false;
|
||||
|
||||
const result = await runInTab(tabName, {
|
||||
code: PDF_SCREENSHOT_CODE,
|
||||
timeoutMs: PDF_RENDER_TIMEOUT_MS,
|
||||
signal: renderSignal,
|
||||
session,
|
||||
});
|
||||
const screenshot = result.screenshots.at(-1);
|
||||
if (!screenshot) throw new ToolError(`Chromium did not capture PDF page ${page}.`);
|
||||
return screenshot;
|
||||
} catch (error) {
|
||||
if (signal?.aborted) throw new ToolAbortError();
|
||||
if (timeoutSignal.aborted) {
|
||||
throw new ToolError(`Timed out rendering PDF page ${page} in Chromium.`);
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
if (tabOpened) await releaseTab(tabName, { kill: false });
|
||||
if (browserLease && browser) await releaseBrowser(browser, { kill: false });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,7 +100,7 @@ import {
|
||||
isRemoteMountPath,
|
||||
type SuffixMatchCache,
|
||||
} from "./read-path-resolution";
|
||||
import { pdfImageRenderingUnsupportedMessage, splitUnsupportedPdfImageReadPath } from "./read-pdf";
|
||||
import { type PdfImageReadTarget, renderPdfPageScreenshot, splitPdfImageReadPath } from "./read-pdf";
|
||||
import { isMultiRange, isRawSelector, type ParsedSelector, parseSel, selToOffsetLimit } from "./read-selector";
|
||||
import { readSqlite, resolveSqliteReadPath } from "./read-sqlite";
|
||||
import { isProseSummaryPath, renderSummary, routeReadThroughBridge, trySummarize } from "./read-summary";
|
||||
@@ -398,8 +398,13 @@ type ReadParams = ReadToolInput;
|
||||
*/
|
||||
export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
readonly name = "read";
|
||||
readonly approval = (args: unknown): ToolTier =>
|
||||
pathTargetsSsh(String((args as { path?: unknown }).path ?? "")) ? "exec" : "read";
|
||||
readonly approval = (args: unknown): ToolTier => {
|
||||
let readPath = "";
|
||||
if (args && typeof args === "object" && "path" in args) readPath = String(args.path ?? "");
|
||||
if (pathTargetsSsh(readPath)) return "exec";
|
||||
const target = splitPathAndSel(readPath);
|
||||
return target.sel === undefined && splitPdfImageReadPath(readPath) ? "exec" : "read";
|
||||
};
|
||||
readonly label = "Read";
|
||||
readonly loadMode = "essential";
|
||||
description: string;
|
||||
@@ -546,6 +551,40 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
return toolResult<ReadToolDetails>({ notes, displayReadTargets }).content(content).done();
|
||||
}
|
||||
|
||||
async #readPdfPageScreenshot(options: {
|
||||
readPath: string;
|
||||
absolutePdfPath: string;
|
||||
page: number;
|
||||
pdfFileSize: number;
|
||||
suffixResolution?: { from: string; to: string };
|
||||
signal?: AbortSignal;
|
||||
}): Promise<AgentToolResult<ReadToolDetails>> {
|
||||
const { readPath, absolutePdfPath, page, pdfFileSize, suffixResolution, signal } = options;
|
||||
const screenshot = await renderPdfPageScreenshot(this.session, absolutePdfPath, page, signal);
|
||||
const screenshotFile = Bun.file(screenshot.dest);
|
||||
const screenshotMetadata = await readImageMetadata(screenshot.dest);
|
||||
const loaded = await this.#loadImageContent({
|
||||
readPath,
|
||||
absolutePath: screenshot.dest,
|
||||
mimeType: screenshot.mimeType,
|
||||
imageMetadata: screenshotMetadata,
|
||||
fileSize: screenshotFile.size,
|
||||
});
|
||||
if (suffixResolution) {
|
||||
const firstText = loaded.content.find((entry): entry is TextContent => entry.type === "text");
|
||||
if (firstText) firstText.text = prependSuffixResolutionNotice(firstText.text, suffixResolution);
|
||||
}
|
||||
const image = loaded.content.find((entry): entry is ImageContent => entry.type === "image");
|
||||
const details: ReadToolDetails = {
|
||||
...loaded.details,
|
||||
resolvedPath: absolutePdfPath,
|
||||
contentType: image?.mimeType ?? screenshot.mimeType,
|
||||
fileSize: pdfFileSize,
|
||||
suffixResolution,
|
||||
};
|
||||
return toolResult(details).content(loaded.content).sourcePath(loaded.sourcePath).done();
|
||||
}
|
||||
|
||||
/**
|
||||
* Build content blocks for an on-disk image file: an `inspect_image`
|
||||
* metadata note when inspection is active, otherwise the decoded image
|
||||
@@ -903,6 +942,8 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
? readPath.includes(":") && (await probeLiteralPathExists(readPath, this.session.cwd)) !== "missing"
|
||||
: literalSplit.sel === undefined && splitPathAndSel(readPath).sel !== undefined;
|
||||
|
||||
let pdfImageRead: PdfImageReadTarget | null = null;
|
||||
|
||||
if (!rawPathIsLiteral) {
|
||||
const archivePath = await resolveArchiveReadPath(this.session, readPath, suffixCache, signal);
|
||||
if (archivePath) {
|
||||
@@ -925,14 +966,14 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
return readSqlite(sqlitePath, signal);
|
||||
}
|
||||
|
||||
const unsupportedPdfImageRead =
|
||||
literalSplit.sel === undefined ? splitUnsupportedPdfImageReadPath(readPath) : null;
|
||||
if (unsupportedPdfImageRead && (await probeLiteralPathExists(readPath, this.session.cwd)) === "missing") {
|
||||
throw new ToolError(pdfImageRenderingUnsupportedMessage(unsupportedPdfImageRead.pdfPath));
|
||||
}
|
||||
const pdfCandidate = literalSplit.sel === undefined ? splitPdfImageReadPath(readPath) : null;
|
||||
pdfImageRead =
|
||||
pdfCandidate && (await probeLiteralPathExists(readPath, this.session.cwd)) === "missing"
|
||||
? pdfCandidate
|
||||
: null;
|
||||
}
|
||||
|
||||
const localTarget = literalSplit;
|
||||
const localTarget = pdfImageRead ? { path: pdfImageRead.pdfPath, sel: undefined } : literalSplit;
|
||||
const localReadPath = localTarget.path;
|
||||
const parsed = parseSel(localTarget.sel);
|
||||
|
||||
@@ -1011,6 +1052,17 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
|
||||
return this.#readFileConflicts(absolutePath, suffixResolution, signal);
|
||||
}
|
||||
|
||||
if (pdfImageRead) {
|
||||
return this.#readPdfPageScreenshot({
|
||||
readPath,
|
||||
absolutePdfPath: absolutePath,
|
||||
page: pdfImageRead.page,
|
||||
pdfFileSize: fileSize,
|
||||
suffixResolution,
|
||||
signal,
|
||||
});
|
||||
}
|
||||
|
||||
const imageMetadata = await readImageMetadata(absolutePath);
|
||||
const mimeType = imageMetadata?.mimeType;
|
||||
const ext = path.extname(absolutePath).toLowerCase();
|
||||
|
||||
@@ -6,16 +6,22 @@ import type { AgentToolResult } from "@oh-my-pi/pi-agent-core";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { ReadTool, type ReadToolDetails } from "@oh-my-pi/pi-coding-agent/tools/read";
|
||||
import * as pdfRead from "@oh-my-pi/pi-coding-agent/tools/read-pdf";
|
||||
import * as markit from "@oh-my-pi/pi-coding-agent/utils/markit";
|
||||
import { removeWithRetries } from "@oh-my-pi/pi-utils";
|
||||
|
||||
const ONE_PX_PNG = Buffer.from(
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC",
|
||||
"base64",
|
||||
);
|
||||
|
||||
function makeSession(cwd: string): ToolSession {
|
||||
return {
|
||||
cwd,
|
||||
hasUI: false,
|
||||
getSessionFile: () => null,
|
||||
getSessionSpawns: () => "*",
|
||||
settings: Settings.isolated({ "images.autoResize": false }),
|
||||
settings: Settings.isolated({ "images.autoResize": false, "inspect_image.mode": "off" }),
|
||||
} as ToolSession;
|
||||
}
|
||||
|
||||
@@ -26,14 +32,17 @@ function textOf(result: AgentToolResult<ReadToolDetails>): string {
|
||||
.join("\n");
|
||||
}
|
||||
|
||||
describe("read unsupported PDF image members", () => {
|
||||
describe("read PDF page screenshots", () => {
|
||||
let testDir: string;
|
||||
let pdfPath: string;
|
||||
let screenshotPath: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
testDir = await fs.mkdtemp(path.join(os.tmpdir(), "read-pdf-image-unsupported-"));
|
||||
testDir = await fs.mkdtemp(path.join(os.tmpdir(), "read-pdf-page-"));
|
||||
pdfPath = path.join(testDir, "doc.pdf");
|
||||
await fs.writeFile(pdfPath, `%PDF-stub-${testDir}`);
|
||||
screenshotPath = path.join(testDir, "rendered.png");
|
||||
await fs.writeFile(screenshotPath, ONE_PX_PNG);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
@@ -41,21 +50,28 @@ describe("read unsupported PDF image members", () => {
|
||||
await removeWithRetries(testDir);
|
||||
});
|
||||
|
||||
it("directs former image listing and PNG member reads to browser rendering", async () => {
|
||||
it("renders former image-member reads through Chromium", async () => {
|
||||
const render = vi.spyOn(pdfRead, "renderPdfPageScreenshot").mockResolvedValue({
|
||||
dest: screenshotPath,
|
||||
mimeType: "image/png",
|
||||
bytes: ONE_PX_PNG.byteLength,
|
||||
width: 1,
|
||||
height: 1,
|
||||
});
|
||||
const tool = new ReadTool(makeSession(testDir));
|
||||
|
||||
for (const readPath of [`${pdfPath}:`, `${pdfPath}:p1-img0.png`]) {
|
||||
try {
|
||||
await tool.execute("read-pdf-image", { path: readPath });
|
||||
throw new Error("Expected the PDF image read to fail");
|
||||
} catch (error) {
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
const message = (error as Error).message;
|
||||
expect(message).toContain("pdf-inspector cannot render PDF images");
|
||||
expect(message).toContain("Puppeteer browser tool");
|
||||
expect(message).toContain(`read '${pdfPath}' for extracted text`);
|
||||
}
|
||||
for (const [readPath, page] of [
|
||||
[`${pdfPath}:`, 1],
|
||||
[`${pdfPath}:p2-img0.png`, 2],
|
||||
] as const) {
|
||||
const result = await tool.execute("read-pdf-image", { path: readPath });
|
||||
expect(result.content.some(entry => entry.type === "image" && entry.mimeType === "image/png")).toBe(true);
|
||||
expect(textOf(result)).toContain("Read image file [image/png]");
|
||||
expect(result.details?.resolvedPath).toBe(pdfPath);
|
||||
expect(render).toHaveBeenLastCalledWith(expect.anything(), pdfPath, page, undefined);
|
||||
}
|
||||
expect(tool.approval({ path: `${pdfPath}:p1-img0.png` })).toBe("exec");
|
||||
expect(tool.approval({ path: `${pdfPath}:2-2` })).toBe("read");
|
||||
});
|
||||
|
||||
it("preserves a literal filename that looks like a PDF image listing", async () => {
|
||||
|
||||
Reference in New Issue
Block a user