diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9e18a14fd..99a443f06 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 :` image listings and `read :.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 :` and `read :.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 diff --git a/packages/coding-agent/src/cli/read-cli.ts b/packages/coding-agent/src/cli/read-cli.ts index c6fd24f38..83cf28e49 100644 --- a/packages/coding-agent/src/cli/read-cli.ts +++ b/packages/coding-agent/src/cli/read-cli.ts @@ -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 { if (MCPManager.instance() === mcpManager) MCPManager.setInstance(undefined); } authStorage?.close(); + await closeDaemonClients(); } if (failed) process.exit(1); diff --git a/packages/coding-agent/src/tools/read-pdf.ts b/packages/coding-agent/src/tools/read-pdf.ts index ece7cc873..eb6d69f29 100644 --- a/packages/coding-agent/src/tools/read-pdf.ts +++ b/packages/coding-agent/src/tools/read-pdf.ts @@ -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 { + 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 }); + } } diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index aa49839ae..dba46c9b9 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -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 { 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 { return toolResult({ notes, displayReadTargets }).content(content).done(); } + async #readPdfPageScreenshot(options: { + readPath: string; + absolutePdfPath: string; + page: number; + pdfFileSize: number; + suffixResolution?: { from: string; to: string }; + signal?: AbortSignal; + }): Promise> { + 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 { ? 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 { 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 { 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(); diff --git a/packages/coding-agent/test/tools/read-pdf-rendering.test.ts b/packages/coding-agent/test/tools/read-pdf-rendering.test.ts index a1728ebe8..ea8dee7bf 100644 --- a/packages/coding-agent/test/tools/read-pdf-rendering.test.ts +++ b/packages/coding-agent/test/tools/read-pdf-rendering.test.ts @@ -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): 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 () => {