feat(coding-agent/tools): simplified browser screenshot persistence and return paths

- Remove the per-call `save` option from `tab.screenshot()` to simplify usage.
- Update `tab.screenshot()` to return the saved file path as a promise string.
- Configure screenshot persistence to use daemon path or custom `browser.screenshotDir`.
- Add comprehensive tests verifying temp path return and custom directory saving.

packages/coding-agent/CHANGELOG.md

@@ -22,6 +22,8 @@
 
 ### Changed
 
+- `tab.screenshot()` no longer accepts a per-call save path. It saves under `browser.screenshotDir`, or the OS temp directory when unset, and returns the saved path.
+
 - Direct and `xd://` dispatch now share one canonical tool map: `write xd://<tool>` executes any enabled top-level or mounted tool, and `read xd://<tool>` returns its docs, instead of failing when the name was exposed through the other layer. Mounted names are presentation metadata only, so tool replacement and disconnection cannot leave stale device instances; disabled tools remain unreachable, and both `xd://` and Cursor/top-level fallback execution retain the tool's approval and ACP permission gates.
 - Session listing now caches parsed headers keyed on file stat identity (mtime + size), so repeated resume-picker opens and startup scans re-read only changed session files
 - Reduced per-keystroke editor dispatch overhead: keybinding resolution happens once per input chunk and the per-action interception chain is gated behind a single canonical-key set probe

packages/coding-agent/src/prompts/tools/browser.md

@@ -8,6 +8,7 @@ Drives real Chromium tab; full puppeteer access via JS.
 - `tab` helpers (drop to raw puppeteer `page` for anything uncovered):
   Element handles: `tab.ref("e5")` / `tab.id(n)` return a handle you call methods on directly — `(await tab.id(n)).click()`. Handles are NOT selectors: `tab.click`/`type`/`fill`/`waitFor*` take STRING selectors only. Snapshot refs work in any selector slot: `tab.click("e5")` ≡ `tab.click("aria-ref=e5")`.
   Simple: `tab.goto`, `tab.click`, `tab.type`, `tab.fill`, `tab.press`, `tab.scroll`, `tab.scrollIntoView`, `tab.drag`, `tab.uploadFile`, `tab.select`, `tab.screenshot`, `tab.extract`, `tab.evaluate`.
+  Screenshots: `tab.screenshot({ selector?, fullPage?, silent? })` saves to `browser.screenshotDir`, or OS temp when unset, then returns the path. It NEVER accepts a path.
   Waits: `tab.waitFor`, `tab.waitForSelector`, `tab.waitForUrl`, `tab.waitForResponse`, `tab.waitForNavigation`.
   Snapshots: `tab.observe()` → accessibility tree; `tab.ariaSnapshot()` → ARIA YAML with `[ref=eN]`.
 

packages/coding-agent/src/tools/browser.ts

@@ -163,11 +163,11 @@ export class BrowserTool implements AgentTool<typeof browserSchema, BrowserToolD
 			},
 		},
 		{
-			caption: "Screenshot to look at the page — no save path",
+			caption: "Capture a screenshot and return its saved path",
 			call: {
 				action: "run",
 				name: "docs",
-				code: "await tab.screenshot();",
+				code: "return await tab.screenshot();",
 			},
 		},
 		{

packages/coding-agent/src/tools/browser/cmux/cmux-tab.ts

@@ -37,7 +37,6 @@ import type { CmuxSocketClient } from "./socket-client";
 interface ScreenshotOptions {
 	selector?: string;
 	fullPage?: boolean;
-	save?: string;
 	silent?: boolean;
 	encoding?: "base64" | "binary";
 }
@@ -487,7 +486,7 @@ export class CmuxTab {
 		return content;
 	}
 
-	async screenshot(opts: ScreenshotOptions = {}): Promise<ScreenshotResult> {
+	async screenshot(opts: ScreenshotOptions = {}): Promise<string> {
 		const context = this.#requireRunContext("tab.screenshot()");
 		// The cmux daemon's `browser.screenshot` captures the surface viewport
 		// only — it has no element-clip or full-page mode, and Bun.Image cannot
@@ -518,20 +517,16 @@ export class CmuxTab {
 				excludeWebP: context.session.excludeWebP,
 			},
 		);
-		const explicitPath = opts.save ? resolveToCwd(opts.save, context.session.cwd) : undefined;
-		const returnedPath = typeof result.path === "string" && result.path.length > 0 ? result.path : undefined;
-		const saveFullRes = !!(explicitPath || context.session.browserScreenshotDir || returnedPath);
+		const saveFullRes = !!context.session.browserScreenshotDir;
 		const savedBuffer = saveFullRes ? buffer : Buffer.from(resized.buffer);
 		const savedMimeType = saveFullRes ? captureMime : resized.mimeType;
 		const ext = savedMimeType === "image/webp" ? "webp" : savedMimeType === "image/jpeg" ? "jpg" : "png";
-		const dest =
-			explicitPath ??
-			(context.session.browserScreenshotDir
-				? path.join(
-						context.session.browserScreenshotDir,
-						`screenshot-${new Date().toISOString().replace(/[:.]/g, "-").slice(0, -1)}.${ext}`,
-					)
-				: (returnedPath ?? path.join(os.tmpdir(), `omp-sshots-${Snowflake.next()}.${ext}`)));
+		const dest = context.session.browserScreenshotDir
+			? path.join(
+					context.session.browserScreenshotDir,
+					`screenshot-${new Date().toISOString().replace(/[:.]/g, "-").slice(0, -1)}.${ext}`,
+				)
+			: path.join(os.tmpdir(), `omp-sshots-${Snowflake.next()}.${ext}`);
 		await fs.promises.mkdir(path.dirname(dest), { recursive: true });
 		await Bun.write(dest, savedBuffer);
 		const info: ScreenshotResult = {
@@ -556,7 +551,7 @@ export class CmuxTab {
 			context.output.push({ type: "text", text: lines.join("\n") });
 			context.output.push({ type: "image", data: resized.data, mimeType: resized.mimeType });
 		}
-		return info;
+		return dest;
 	}
 
 	async waitForUrl(pattern: string | RegExp, opts?: { timeout?: number }): Promise<string> {

packages/coding-agent/src/tools/browser/tab-worker.ts

@@ -11,7 +11,6 @@ import type {
 	ElementHandle,
 	ElementScreenshotOptions,
 	HTTPResponse,
-	ImageFormat,
 	KeyInput,
 	Page,
 	SerializedAXNode,
@@ -217,7 +216,6 @@ export function resolveWaitTimeout(cellTimeoutMs: number, explicit?: number): nu
 interface ScreenshotOptions {
 	selector?: string;
 	fullPage?: boolean;
-	save?: string;
 	silent?: boolean;
 }
 
@@ -233,7 +231,7 @@ interface TabApi {
 	): Promise<void>;
 	observe(opts?: { includeAll?: boolean; viewportOnly?: boolean }): Promise<Observation>;
 	ariaSnapshot(selector?: string, opts?: AriaSnapshotOptions): Promise<string>;
-	screenshot(opts?: ScreenshotOptions): Promise<ScreenshotResult>;
+	screenshot(opts?: ScreenshotOptions): Promise<string>;
 	extract(format?: ReadableFormat): Promise<string>;
 	click(selector: string): Promise<void>;
 	type(selector: string, text: string): Promise<void>;
@@ -714,19 +712,6 @@ export function describeScreenshot(opts?: ScreenshotOptions): string {
 	return "tab.screenshot()";
 }
 
-/** Map an explicit save path's extension to a puppeteer capture format (default png). */
-export function imageFormatForPath(filePath: string): ImageFormat {
-	switch (path.extname(filePath).toLowerCase()) {
-		case ".webp":
-			return "webp";
-		case ".jpg":
-		case ".jpeg":
-			return "jpeg";
-		default:
-			return "png";
-	}
-}
-
 /** Summarize still-running helpers (oldest first) so a cell timeout names what stalled. */
 export function describeInflight(inflight: Map<number, InflightOp>): string {
 	const now = Date.now();
@@ -1532,7 +1517,7 @@ export class WorkerCore {
 		screenshots: ScreenshotResult[],
 		signal: AbortSignal | undefined,
 		opts: ScreenshotOptions = {},
-	): Promise<ScreenshotResult> {
+	): Promise<string> {
 		const page = this.#requirePage();
 		// Multiple tabs can share one Chromium (sibling headless tabs on a shared
 		// endpoint, cdp/app attach). CDP `Page.captureScreenshot` reads the
@@ -1542,12 +1527,8 @@ export class WorkerCore {
 		// already-active or freshly-closed target never fails the capture.
 		await untilAborted(signal, () => page.bringToFront()).catch(() => undefined);
 		const fullPage = opts.selector ? false : (opts.fullPage ?? false);
-		// An explicit save path picks the full-res capture format: puppeteer encodes
-		// png/jpeg/webp natively, so `save: "shot.webp"` gets real WebP bytes instead
-		// of PNG bytes hiding behind a .webp name. Unknown/missing extensions stay PNG.
-		const explicitPath = opts.save ? resolveToCwd(opts.save, session.cwd) : undefined;
-		const captureType = explicitPath ? imageFormatForPath(explicitPath) : "png";
-		const captureMime = `image/${captureType}` as const;
+		const captureType = "png";
+		const captureMime = "image/png" as const;
 		let buffer: Buffer;
 		if (opts.selector) {
 			const handle =
@@ -1581,20 +1562,16 @@ export class WorkerCore {
 			{ type: "image", data: buffer.toBase64(), mimeType: captureMime },
 			{ maxWidth: 1024, maxHeight: 1024, maxBytes: 150 * 1024, jpegQuality: 70, excludeWebP: session.excludeWebP },
 		);
-		const saveFullRes = !!(explicitPath || session.browserScreenshotDir);
+		const saveFullRes = !!session.browserScreenshotDir;
 		const savedBuffer = saveFullRes ? buffer : resized.buffer;
 		const savedMimeType = saveFullRes ? captureMime : resized.mimeType;
-		// Names must match the bytes we actually write: full-res follows the capture
-		// format, the resized buffer is whichever of PNG/JPEG/WebP encoded smallest.
 		const ext = savedMimeType === "image/webp" ? "webp" : savedMimeType === "image/jpeg" ? "jpg" : "png";
-		const dest =
-			explicitPath ??
-			(session.browserScreenshotDir
-				? path.join(
-						session.browserScreenshotDir,
-						`screenshot-${new Date().toISOString().replace(/[:.]/g, "-").slice(0, -1)}.${ext}`,
-					)
-				: path.join(os.tmpdir(), `omp-sshots-${Snowflake.next()}.${ext}`));
+		const dest = session.browserScreenshotDir
+			? path.join(
+					session.browserScreenshotDir,
+					`screenshot-${new Date().toISOString().replace(/[:.]/g, "-").slice(0, -1)}.${ext}`,
+				)
+			: path.join(os.tmpdir(), `omp-sshots-${Snowflake.next()}.${ext}`);
 		await fs.promises.mkdir(path.dirname(dest), { recursive: true });
 		await Bun.write(dest, savedBuffer);
 		const info: ScreenshotResult = {
@@ -1616,7 +1593,7 @@ export class WorkerCore {
 			output.push({ type: "text", text: lines.join("\n") });
 			output.push({ type: "image", data: resized.data, mimeType: resized.mimeType });
 		}
-		return info;
+		return dest;
 	}
 
 	async #drag(from: DragTarget, to: DragTarget, signal: AbortSignal): Promise<void> {

packages/coding-agent/test/tools/browser-cmux-release-mid-run.test.ts

@@ -29,6 +29,9 @@
  */
 
 import { afterEach, describe, expect, it, spyOn, vi } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
 import type { CmuxKind } from "@oh-my-pi/pi-coding-agent/tools/browser/cmux/rpc";
 import { CmuxSocketClient } from "@oh-my-pi/pi-coding-agent/tools/browser/cmux/socket-client";
 import { acquireBrowser } from "@oh-my-pi/pi-coding-agent/tools/browser/registry";
@@ -48,14 +51,13 @@ function makeKind(socketSuffix: string): CmuxKind {
 	};
 }
 
-function makeSession(cwd: string): ToolSession {
-	// Minimal shape: `runInTab` only reads `cwd`, `settings.get("browser.screenshotDir")`,
-	// and `getActiveModel?.()`. Everything else on `ToolSession` is untouched by the
-	// tab-supervisor flow we exercise.
+function makeSession(cwd: string, screenshotDir?: string): ToolSession {
+	// Minimal shape: `runInTab` reads `cwd`, `settings.get("browser.screenshotDir")`,
+	// and `getActiveModel?.()`. Everything else is untouched by this flow.
 	return {
 		cwd,
 		hasUI: false,
-		settings: { get: () => undefined },
+		settings: { get: (key: string) => (key === "browser.screenshotDir" ? screenshotDir : undefined) },
 		getSessionFile: () => null,
 	} as unknown as ToolSession;
 }
@@ -283,4 +285,99 @@ describe("browser tab-supervisor — cmux tab close mid-run (#4499)", () => {
 			process.removeListener("unhandledRejection", onUnhandled);
 		}
 	});
+
+	it("ignores the daemon screenshot path when no screenshot directory is configured", async () => {
+		spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined);
+		spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined);
+		spyOn(CmuxSocketClient.prototype, "request").mockImplementation(
+			async (method: string): Promise<Record<string, unknown>> => {
+				switch (method) {
+					case "browser.open_split":
+						return { surface_id: "surface-screenshot", url: "about:blank" };
+					case "browser.url.get":
+						return { url: "about:blank" };
+					case "browser.snapshot":
+						return { page: { html: "" } };
+					case "browser.eval":
+						return { value: "" };
+					case "browser.screenshot":
+						return {
+							path: "/workspace/screenshots/daemon-owned.png",
+							png_base64:
+								"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M/wHwAF/gL+4z8ZAAAAAElFTkSuQmCC",
+						};
+					default:
+						return {};
+				}
+			},
+		);
+
+		const browser = await acquireBrowser(makeKind("screenshot-temp"), { cwd: "/tmp" });
+		await acquireTab("screenshot-temp", browser, {
+			timeoutMs: 5_000,
+			ownerSessionId: "session-screenshot-temp",
+		});
+
+		const result = await runInTab("screenshot-temp", {
+			code: "return await tab.screenshot({ silent: true });",
+			timeoutMs: 5_000,
+			session: makeSession("/tmp"),
+		});
+		const savedPath = result.returnValue;
+		expect(typeof savedPath).toBe("string");
+		if (typeof savedPath !== "string") throw new Error("tab.screenshot() did not return a path");
+		expect(path.dirname(savedPath)).toBe(os.tmpdir());
+		expect(savedPath).not.toBe("/workspace/screenshots/daemon-owned.png");
+		expect(await Bun.file(savedPath).exists()).toBe(true);
+		await fs.rm(savedPath);
+	});
+
+	it("saves screenshots under the configured screenshot directory", async () => {
+		spyOn(CmuxSocketClient.prototype, "connect").mockResolvedValue(undefined);
+		spyOn(CmuxSocketClient.prototype, "close").mockImplementation(() => undefined);
+		spyOn(CmuxSocketClient.prototype, "request").mockImplementation(
+			async (method: string): Promise<Record<string, unknown>> => {
+				switch (method) {
+					case "browser.open_split":
+						return { surface_id: "surface-screenshot-configured", url: "about:blank" };
+					case "browser.url.get":
+						return { url: "about:blank" };
+					case "browser.snapshot":
+						return { page: { html: "" } };
+					case "browser.eval":
+						return { value: "" };
+					case "browser.screenshot":
+						return {
+							path: "/workspace/screenshots/daemon-owned.png",
+							png_base64:
+								"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M/wHwAF/gL+4z8ZAAAAAElFTkSuQmCC",
+						};
+					default:
+						return {};
+				}
+			},
+		);
+
+		const screenshotDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-cmux-screenshot-"));
+		try {
+			const browser = await acquireBrowser(makeKind("screenshot-configured"), { cwd: "/tmp" });
+			await acquireTab("screenshot-configured", browser, {
+				timeoutMs: 5_000,
+				ownerSessionId: "session-screenshot-configured",
+			});
+
+			const result = await runInTab("screenshot-configured", {
+				code: "return await tab.screenshot({ silent: true });",
+				timeoutMs: 5_000,
+				session: makeSession("/tmp", screenshotDir),
+			});
+			const savedPath = result.returnValue;
+			expect(typeof savedPath).toBe("string");
+			if (typeof savedPath !== "string") throw new Error("tab.screenshot() did not return a path");
+			expect(path.dirname(savedPath)).toBe(screenshotDir);
+			expect(await Bun.file(savedPath).exists()).toBe(true);
+		} finally {
+			await fs.rm(screenshotDir, { recursive: true, force: true });
+		}
+	});
 });

packages/coding-agent/test/tools/browser-op-tracking.test.ts

@@ -3,7 +3,6 @@ import {
 	describeInflight,
 	describeScreenshot,
 	type InflightOp,
-	imageFormatForPath,
 } from "@oh-my-pi/pi-coding-agent/tools/browser/tab-worker";
 
 describe("browser op tracking — timeout diagnostics", () => {
@@ -37,20 +36,3 @@ describe("browser op tracking — timeout diagnostics", () => {
 		expect(describeInflight(new Map())).toBe("");
 	});
 });
-
-describe("imageFormatForPath — explicit save capture format", () => {
-	it("maps the save path's extension to the matching capture format", () => {
-		expect(imageFormatForPath("/tmp/shot.webp")).toBe("webp");
-		expect(imageFormatForPath("/tmp/shot.WEBP")).toBe("webp");
-		expect(imageFormatForPath("/tmp/shot.jpg")).toBe("jpeg");
-		expect(imageFormatForPath("/tmp/shot.jpeg")).toBe("jpeg");
-		expect(imageFormatForPath("/tmp/shot.png")).toBe("png");
-	});
-
-	it("falls back to png for unknown or missing extensions", () => {
-		expect(imageFormatForPath("/tmp/shot")).toBe("png");
-		expect(imageFormatForPath("/tmp/shot.txt")).toBe("png");
-		// A dotted directory must not leak its "extension" into an extensionless basename.
-		expect(imageFormatForPath("/tmp/v1.2/shot")).toBe("png");
-	});
-});