fix(read): guarded large artifact raw reads

Resolved artifact:// reads to backing files before selector handling, streamed bounded reads, and blocked unbounded raw reads for large artifacts with recovery guidance.

Fixes #4482

packages/coding-agent/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed large `artifact://` reads materializing entire MCP/tool artifacts before selector paging, preventing OOM crashes on unbounded raw reads and surfacing bounded read/search/copy guidance ([#4482](https://github.com/can1357/oh-my-pi/issues/4482)).
+
 ## [16.3.4] - 2026-07-03
 
 ### Fixed

packages/coding-agent/src/internal-urls/artifact-protocol.ts

@@ -15,75 +15,105 @@ import { isEnoent } from "@oh-my-pi/pi-utils";
 import { artifactsDirsFromRegistry } from "./registry-helpers";
 import type { InternalResource, InternalUrl, ProtocolHandler, ResolveContext, UrlCompletion } from "./types";
 
+const MAX_INLINE_ARTIFACT_BYTES = 8 * 1024 * 1024;
+
+/** Filesystem location for a session artifact, resolved without materializing its content. */
+export interface ResolvedArtifactFile {
+	id: string;
+	path: string;
+	size: number;
+}
+
+function parseArtifactId(url: InternalUrl): string {
+	const id = url.rawHost || url.hostname;
+	if (!id) {
+		throw new Error("artifact:// URL requires a numeric ID: artifact://0");
+	}
+	if (!/^\d+$/.test(id)) {
+		throw new Error(`artifact:// ID must be numeric, got: ${id}`);
+	}
+	return id;
+}
+
+/** Resolve an `artifact://` URL to its backing file without reading artifact bytes. */
+export async function resolveArtifactFile(url: InternalUrl, context?: ResolveContext): Promise<ResolvedArtifactFile> {
+	const id = parseArtifactId(url);
+
+	// Artifact ids are per-session counters; in multi-session hosts the same
+	// id exists in several dirs. Pin resolution to the calling session's
+	// artifacts dir first so `artifact://3` means *this* session's #3.
+	const dirs = artifactsDirsFromRegistry();
+	const pinnedDir = context?.localProtocolOptions?.getArtifactsDir?.() ?? null;
+	if (pinnedDir) {
+		const pinnedIndex = dirs.indexOf(pinnedDir);
+		if (pinnedIndex >= 0) dirs.splice(pinnedIndex, 1);
+		dirs.unshift(pinnedDir);
+	}
+
+	if (dirs.length === 0) {
+		throw new Error("No session - artifacts unavailable");
+	}
+
+	let foundPath: string | undefined;
+	let anyDirExists = false;
+	const availableIds = new Set<string>();
+
+	for (const dir of dirs) {
+		let files: string[];
+		try {
+			files = await fs.readdir(dir);
+			anyDirExists = true;
+		} catch (err) {
+			if (isEnoent(err)) continue;
+			throw err;
+		}
+		const match = files.find(f => f.startsWith(`${id}.`));
+		if (match) {
+			foundPath = path.join(dir, match);
+			break;
+		}
+		for (const f of files) {
+			const m = f.match(/^(\d+)\./);
+			if (m) availableIds.add(m[1]);
+		}
+	}
+
+	if (!anyDirExists) {
+		throw new Error("No artifacts directory found");
+	}
+
+	if (!foundPath) {
+		const sorted = [...availableIds].sort((a, b) => Number(a) - Number(b));
+		const availableStr = sorted.length > 0 ? sorted.join(", ") : "none";
+		throw new Error(`Artifact ${id} not found. Available: ${availableStr}`);
+	}
+
+	const stat = await Bun.file(foundPath).stat();
+	if (stat.isDirectory()) {
+		throw new Error(`Artifact ${id} resolved to a directory, not a file`);
+	}
+	return { id, path: foundPath, size: stat.size };
+}
+
 export class ArtifactProtocolHandler implements ProtocolHandler {
 	readonly scheme = "artifact";
 	readonly immutable = true;
 
 	async resolve(url: InternalUrl, context?: ResolveContext): Promise<InternalResource> {
-		const id = url.rawHost || url.hostname;
-		if (!id) {
-			throw new Error("artifact:// URL requires a numeric ID: artifact://0");
-		}
-		if (!/^\d+$/.test(id)) {
-			throw new Error(`artifact:// ID must be numeric, got: ${id}`);
+		const artifact = await resolveArtifactFile(url, context);
+		if (artifact.size > MAX_INLINE_ARTIFACT_BYTES) {
+			throw new Error(
+				`Artifact ${artifact.id} is ${artifact.size} bytes; full internal resolution is blocked. Use read selectors such as artifact://${artifact.id}:1-3000 or artifact://${artifact.id}:raw:1-3000, and use the artifact file path for search/copy workflows: ${artifact.path}`,
+			);
 		}
 
-		// Artifact ids are per-session counters; in multi-session hosts the same
-		// id exists in several dirs. Pin resolution to the calling session's
-		// artifacts dir first so `artifact://3` means *this* session's #3.
-		const dirs = artifactsDirsFromRegistry();
-		const pinnedDir = context?.localProtocolOptions?.getArtifactsDir?.() ?? null;
-		if (pinnedDir) {
-			const pinnedIndex = dirs.indexOf(pinnedDir);
-			if (pinnedIndex >= 0) dirs.splice(pinnedIndex, 1);
-			dirs.unshift(pinnedDir);
-		}
-
-		if (dirs.length === 0) {
-			throw new Error("No session - artifacts unavailable");
-		}
-
-		let foundPath: string | undefined;
-		let anyDirExists = false;
-		const availableIds = new Set<string>();
-
-		for (const dir of dirs) {
-			let files: string[];
-			try {
-				files = await fs.readdir(dir);
-				anyDirExists = true;
-			} catch (err) {
-				if (isEnoent(err)) continue;
-				throw err;
-			}
-			const match = files.find(f => f.startsWith(`${id}.`));
-			if (match) {
-				foundPath = path.join(dir, match);
-				break;
-			}
-			for (const f of files) {
-				const m = f.match(/^(\d+)\./);
-				if (m) availableIds.add(m[1]);
-			}
-		}
-
-		if (!anyDirExists) {
-			throw new Error("No artifacts directory found");
-		}
-
-		if (!foundPath) {
-			const sorted = [...availableIds].sort((a, b) => Number(a) - Number(b));
-			const availableStr = sorted.length > 0 ? sorted.join(", ") : "none";
-			throw new Error(`Artifact ${id} not found. Available: ${availableStr}`);
-		}
-
-		const content = await Bun.file(foundPath).text();
+		const content = await Bun.file(artifact.path).text();
 		return {
 			url: url.href,
 			content,
 			contentType: "text/plain",
-			size: Buffer.byteLength(content, "utf-8"),
-			sourcePath: foundPath,
+			size: artifact.size,
+			sourcePath: artifact.path,
 		};
 	}
 

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

@@ -67,7 +67,7 @@ For `.sqlite`, `.sqlite3`, `.db`, `.db3`:
 
 # Internal URIs
 
-All URI schemes take the same line selectors. `artifact://<id>` recovers full output a bash/eval/tool result spilled or truncated.
+All URI schemes take the same line selectors. `artifact://<id>` recovers spilled output; large artifacts block unbounded `:raw`, so page with `artifact://<id>:N-M` / `artifact://<id>:raw:N-M` and use the reported artifact file path for search/copy workflows.
 
 `ssh://host/<absolute-path>` reads a remote text file (UTF-8, ≤1 MiB) or lists a directory one level deep, on a pre-configured SSH host or `~/.ssh/config` alias; `ssh://host/` lists the remote root and bare `ssh://` lists the configured hosts. Files are also writable via `write` and searchable via `search`; a directory only lists (`search` refuses a directory, `write` refuses to overwrite one). A literal `:`, `?`, or `#` in the remote path must be percent-encoded (`%3A`/`%3F`/`%23`) — a trailing `:sel` is read as a line selector, and `?`/`#` start a URL query/fragment. Requires a POSIX login shell (`sh`/`bash`/`zsh`); a Windows host or a non-POSIX shell (fish, csh/tcsh) is rejected — use the `ssh` tool there.
 

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

@@ -37,6 +37,7 @@ import { normalizeToLF } from "../edit/normalize";
 import { isNotebookPath, readEditableNotebookText } from "../edit/notebook";
 import type { RenderResultOptions } from "../extensibility/custom-tools/types";
 import { InternalUrlRouter, resolveLocalUrlToFile } from "../internal-urls";
+import { type ResolvedArtifactFile, resolveArtifactFile } from "../internal-urls/artifact-protocol";
 import { parseInternalUrl } from "../internal-urls/parse";
 import type { InternalUrl } from "../internal-urls/types";
 import { getLanguageFromPath, type Theme } from "../modes/theme/theme";
@@ -151,6 +152,7 @@ const CONVERTIBLE_EXTENSIONS = new Set([".pdf", ".doc", ".docx", ".ppt", ".pptx"
 
 const MAX_SUMMARY_BYTES = 2 * 1024 * 1024;
 const MAX_SUMMARY_LINES = 20_000;
+const MAX_ARTIFACT_RAW_INLINE_BYTES = DEFAULT_MAX_BYTES;
 /**
  * Per-line column cap for file reads. Lines wider than the value of
  * `tools.outputMaxColumns` are ellipsis-truncated at display time; the file
@@ -1519,6 +1521,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
 		displayMode: { hashLines: boolean; lineNumbers: boolean },
 		suffixResolution: { from: string; to: string } | undefined,
 		signal: AbortSignal | undefined,
+		allowBridge = true,
 	): Promise<{
 		outputText: string;
 		columnTruncated: number;
@@ -1528,7 +1531,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
 		const rawSelector = isRawSelector(parsed);
 
 		// ACP bridge first — the editor's in-memory buffer is source of truth.
-		const bridgePromise = this.#routeReadThroughBridge(absolutePath);
+		const bridgePromise = allowBridge ? this.#routeReadThroughBridge(absolutePath) : undefined;
 		if (bridgePromise !== undefined) {
 			try {
 				const bridgeText = await bridgePromise;
@@ -2802,6 +2805,195 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
 		return toolResult<ReadToolDetails>(details).text(summary).sourcePath(absolutePath).done();
 	}
 
+	#formatArtifactWorkflowNotice(artifact: ResolvedArtifactFile, artifactUrl: string): string {
+		return `Artifact storage: ${artifact.path} (${formatBytes(artifact.size)}). Use ${artifactUrl}:N-M to page, ${artifactUrl}:raw:N-M for verbatim chunks, and the artifact file path for search/copy workflows.`;
+	}
+
+	#formatRawArtifactBlockedNotice(artifact: ResolvedArtifactFile, artifactUrl: string): string {
+		return `Unbounded raw read blocked for ${artifactUrl} (${formatBytes(
+			artifact.size,
+		)}). Reading the whole artifact verbatim can exhaust memory. Use ${artifactUrl}:raw:1-3000 for bounded verbatim chunks, ${artifactUrl}:1-3000 for numbered exploration, and the artifact file path for search/copy workflows: ${artifact.path}`;
+	}
+
+	async #readArtifactFile(
+		url: InternalUrl,
+		parsedSel: ParsedSelector,
+		signal?: AbortSignal,
+	): Promise<AgentToolResult<ReadToolDetails>> {
+		const artifact = await resolveArtifactFile(url, {
+			cwd: this.session.cwd,
+			settings: this.session.settings,
+			signal,
+			localProtocolOptions: this.session.localProtocolOptions,
+			skills: this.session.skills,
+		});
+		const artifactUrl = `artifact://${artifact.id}`;
+		const details: ReadToolDetails = {
+			resolvedPath: artifact.path,
+			contentType: "text/plain",
+		};
+
+		if (parsedSel.kind === "raw" && artifact.size > MAX_ARTIFACT_RAW_INLINE_BYTES) {
+			return toolResult<ReadToolDetails>(details)
+				.text(this.#formatRawArtifactBlockedNotice(artifact, artifactUrl))
+				.sourcePath(artifact.path)
+				.sourceInternal(url.href)
+				.done();
+		}
+
+		const rawSelector = isRawSelector(parsedSel);
+		const displayMode = resolveFileDisplayMode(this.session, { raw: rawSelector, immutable: true });
+		if (isMultiRange(parsedSel) && parsedSel.kind === "lines") {
+			const read = await this.#readLocalFileMultiRange(
+				artifact.path,
+				parsedSel.ranges,
+				artifact.size,
+				parsedSel,
+				displayMode,
+				undefined,
+				signal,
+				false,
+			);
+			if (read.bridgeResult) return read.bridgeResult;
+			if (read.displayContent) details.displayContent = read.displayContent;
+			let text = read.outputText;
+			if (artifact.size > MAX_ARTIFACT_RAW_INLINE_BYTES) {
+				text = text
+					? `${text}\n\n[${this.#formatArtifactWorkflowNotice(artifact, artifactUrl)}]`
+					: this.#formatArtifactWorkflowNotice(artifact, artifactUrl);
+			}
+			const resultBuilder = toolResult<ReadToolDetails>(details)
+				.text(text)
+				.sourcePath(artifact.path)
+				.sourceInternal(url.href);
+			if (read.columnTruncated > 0) resultBuilder.limits({ columnMax: read.columnTruncated });
+			return resultBuilder.done();
+		}
+
+		const { offset, limit } = selToOffsetLimit(parsedSel);
+		const requestedStart = offset ? Math.max(0, offset - 1) : 0;
+		const expandStart = offset !== undefined && offset > 1;
+		const expandEnd = limit !== undefined;
+		const leadingContext = expandStart ? Math.min(requestedStart, RANGE_LEADING_CONTEXT_LINES) : 0;
+		const trailingContext = expandEnd ? RANGE_TRAILING_CONTEXT_LINES : 0;
+		const startLine = requestedStart - leadingContext;
+		const startLineDisplay = startLine + 1;
+		const effectiveLimit = limit ?? this.#defaultLimit;
+		const maxLinesToCollect = Math.min(effectiveLimit + leadingContext + trailingContext, DEFAULT_MAX_LINES);
+		const selectedLineLimit = effectiveLimit + leadingContext + trailingContext;
+		const maxBytesForRead = Math.max(DEFAULT_MAX_BYTES, maxLinesToCollect * 512);
+		const streamResult = await streamLinesFromFile(
+			artifact.path,
+			startLine,
+			maxLinesToCollect,
+			maxBytesForRead,
+			selectedLineLimit,
+			signal,
+			artifact.size > SNAPSHOT_MAX_BYTES,
+		);
+		const {
+			lines: collectedLines,
+			totalFileLines,
+			collectedBytes,
+			stoppedByByteLimit,
+			firstLinePreview,
+			firstLineByteLength,
+			reachedEof,
+		} = streamResult;
+
+		if (requestedStart >= totalFileLines) {
+			const suggestion =
+				totalFileLines === 0
+					? "The artifact is empty."
+					: `Use ${artifactUrl}:1 to read from the start, or ${artifactUrl}:${totalFileLines} to read the last line.`;
+			return toolResult<ReadToolDetails>(details)
+				.text(`Line ${requestedStart + 1} is beyond end of artifact (${totalFileLines} lines total). ${suggestion}`)
+				.sourcePath(artifact.path)
+				.sourceInternal(url.href)
+				.done();
+		}
+
+		const shouldAddLineNumbers = rawSelector ? false : displayMode.hashLines ? false : displayMode.lineNumbers;
+		const selectedContent = collectedLines.join("\n");
+		const totalSelectedLines = totalFileLines - startLine;
+		const wasTruncated = collectedLines.length < totalSelectedLines || stoppedByByteLimit;
+		const firstLineExceedsLimit = firstLineByteLength !== undefined && firstLineByteLength > maxBytesForRead;
+		const truncation: TruncationResult = {
+			content: selectedContent,
+			truncated: wasTruncated,
+			truncatedBy: stoppedByByteLimit ? "bytes" : wasTruncated ? "lines" : undefined,
+			totalLines: totalSelectedLines,
+			totalBytes: collectedBytes,
+			outputLines: collectedLines.length,
+			outputBytes: collectedBytes,
+			lastLinePartial: false,
+			firstLineExceedsLimit,
+		};
+
+		let displayContent: { text: string; startLine: number; lineNumbers?: Array<number | null> } | undefined;
+		const formatText = (text: string, startNum: number): string => {
+			const lineCount = countTextLines(text);
+			displayContent = {
+				text,
+				startLine: startNum,
+				lineNumbers: Array.from({ length: lineCount }, (_, i) => startNum + i),
+			};
+			return formatTextWithMode(text, startNum, false, shouldAddLineNumbers);
+		};
+
+		let outputText: string;
+		let truncationInfo:
+			| { result: TruncationResult; options: { direction: "head"; startLine?: number; totalFileLines?: number } }
+			| undefined;
+		if (truncation.firstLineExceedsLimit) {
+			const firstLineBytes = firstLineByteLength ?? 0;
+			const snippet = firstLinePreview ?? { text: "", bytes: 0 };
+			outputText =
+				snippet.text.length > 0
+					? formatText(snippet.text, startLineDisplay)
+					: `[Line ${startLineDisplay} is ${formatBytes(
+							firstLineBytes,
+						)}, exceeds ${formatBytes(maxBytesForRead)} limit. Unable to display a valid UTF-8 snippet.]`;
+			truncationInfo = {
+				result: truncation,
+				options: {
+					direction: "head",
+					startLine: startLineDisplay,
+					totalFileLines: reachedEof ? totalFileLines : undefined,
+				},
+			};
+		} else {
+			outputText = formatText(truncation.content, startLineDisplay);
+			if (truncation.truncated) {
+				truncationInfo = {
+					result: truncation,
+					options: {
+						direction: "head",
+						startLine: startLineDisplay,
+						totalFileLines: reachedEof ? totalFileLines : undefined,
+					},
+				};
+			} else if (startLine + collectedLines.length < totalFileLines || !reachedEof) {
+				const nextOffset = startLine + collectedLines.length + 1;
+				outputText += reachedEof
+					? `\n\n[${totalFileLines - (startLine + collectedLines.length)} more lines in artifact. Use ${artifactUrl}:${nextOffset} to continue]`
+					: `\n\n[More lines in artifact (${formatBytes(artifact.size)} total; not scanned to EOF). Use ${artifactUrl}:${nextOffset} to continue]`;
+			}
+		}
+
+		if (artifact.size > MAX_ARTIFACT_RAW_INLINE_BYTES) {
+			outputText += `\n\n[${this.#formatArtifactWorkflowNotice(artifact, artifactUrl)}]`;
+		}
+		if (displayContent) details.displayContent = displayContent;
+		if (truncationInfo) details.truncation = truncationInfo.result;
+		const resultBuilder = toolResult<ReadToolDetails>(details)
+			.text(outputText)
+			.sourcePath(artifact.path)
+			.sourceInternal(url.href);
+		if (truncationInfo) resultBuilder.truncation(truncationInfo.result, truncationInfo.options);
+		return resultBuilder.done();
+	}
+
 	/**
 	 * Handle internal URLs (agent://, artifact://, memory://, skill://, rule://, local://, mcp://).
 	 * Supports pagination via offset/limit but rejects them when query extraction is used.
@@ -2829,6 +3021,9 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
 			const hasQueryExtraction = queryParam !== null && queryParam !== "";
 			hasExtraction = hasPathExtraction || hasQueryExtraction;
 		}
+		if (scheme === "artifact") {
+			return this.#readArtifactFile(urlMeta, parsedSel, signal);
+		}
 
 		// local:// files are real on-disk paths. Detect image files and emit a
 		// decoded image block before the text-only resource contract UTF-8

packages/coding-agent/test/tools/read-artifact-large.test.ts

@@ -0,0 +1,93 @@
+import { afterEach, beforeEach, describe, expect, it } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
+import {
+	registerArtifactsDir,
+	resetRegisteredArtifactDirsForTests,
+} from "@oh-my-pi/pi-coding-agent/internal-urls/registry-helpers";
+import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
+import { ReadTool } from "@oh-my-pi/pi-coding-agent/tools/read";
+
+function getTextOutput(result: { content: Array<{ type: string; text?: string }> }): string {
+	return result.content
+		.filter(c => c.type === "text" && typeof c.text === "string")
+		.map(c => c.text as string)
+		.join("\n");
+}
+
+function makeSession(cwd: string): ToolSession {
+	return {
+		cwd,
+		hasUI: false,
+		getSessionFile: () => path.join(cwd, "session.jsonl"),
+		getSessionSpawns: () => "*",
+		getArtifactsDir: () => path.join(cwd, "session"),
+		allocateOutputArtifact: async (toolType: string) => ({
+			id: "a1",
+			path: path.join(cwd, "session", `a1.${toolType}.log`),
+		}),
+		settings: Settings.isolated(),
+	};
+}
+
+function largeArtifactText(): string {
+	return Array.from(
+		{ length: 400 },
+		(_, index) => `line-${String(index + 1).padStart(3, "0")} ${"x".repeat(256)}`,
+	).join("\n");
+}
+
+describe("read tool large artifact handling", () => {
+	let testDir: string;
+	let artifactDir: string;
+	let unregisterArtifactsDir: (() => void) | undefined;
+	let tool: ReadTool;
+
+	beforeEach(async () => {
+		testDir = await fs.mkdtemp(path.join(os.tmpdir(), "read-artifact-large-"));
+		artifactDir = path.join(testDir, "session");
+		await fs.mkdir(artifactDir, { recursive: true });
+		await Bun.write(path.join(artifactDir, "0.mcp.log"), largeArtifactText());
+		resetRegisteredArtifactDirsForTests();
+		unregisterArtifactsDir = registerArtifactsDir(artifactDir);
+		tool = new ReadTool(makeSession(testDir));
+	});
+
+	afterEach(async () => {
+		unregisterArtifactsDir?.();
+		resetRegisteredArtifactDirsForTests();
+		await fs.rm(testDir, { recursive: true, force: true });
+	});
+
+	it("blocks unbounded raw reads and points to bounded artifact workflows", async () => {
+		const result = await tool.execute("call-raw", { path: "artifact://0:raw" });
+		const output = getTextOutput(result);
+
+		expect(output).toContain("Unbounded raw read blocked for artifact://0");
+		expect(output).toContain("artifact://0:raw:1-3000");
+		expect(output).toContain(artifactDir);
+		expect(output).not.toContain("line-001");
+	});
+
+	it("streams bounded artifact reads without materializing the whole artifact", async () => {
+		const result = await tool.execute("call-range", { path: "artifact://0:1-3" });
+		const output = getTextOutput(result);
+
+		expect(output).toContain("line-001");
+		expect(output).toContain("line-003");
+		expect(output).toContain("Artifact storage:");
+		expect(output).toContain("artifact://0:raw:N-M");
+		expect(output).not.toContain("line-400");
+	});
+
+	it("allows bounded raw artifact chunks", async () => {
+		const result = await tool.execute("call-raw-range", { path: "artifact://0:raw:1-2" });
+		const output = getTextOutput(result);
+
+		expect(output).toStartWith("line-001");
+		expect(output).toContain("line-002");
+		expect(output).not.toContain("line-400");
+	});
+});