fix(coding-agent): applied immutable metadata to internal URL resolution

- Added immutable metadata to protocol handlers and had the router copy each handler's setting onto resolved internal resources. - Updated read and search tools to honor `resource.immutable`, and made search suppress hashline anchors only for immutable source paths while preserving them for mutable files. - Expanded internal URL tests to cover mutable local resources and mixed immutable/mutable search input hashline behavior.
2026-05-10 12:46:05 +02:00
parent 32e1b41889
commit 8cc1aba65f
15 changed files with 93 additions and 8 deletions
@@ -42,6 +42,7 @@ async function listAvailableOutputs(artifactsDir: string): Promise<string[]> {
 */
 export class AgentProtocolHandler implements ProtocolHandler {
 	readonly scheme = "agent";
+	readonly immutable = true;

 	constructor(private readonly options: AgentProtocolOptions) {}

@@ -44,6 +44,7 @@ async function listAvailableArtifacts(artifactsDir: string): Promise<string[]> {
 */
 export class ArtifactProtocolHandler implements ProtocolHandler {
 	readonly scheme = "artifact";
+	readonly immutable = true;

 	constructor(private readonly options: ArtifactProtocolOptions) {}

@@ -24,6 +24,7 @@ function normalizeJobId(url: InternalUrl): string {

 export class JobsProtocolHandler implements ProtocolHandler {
 	readonly scheme = "jobs";
+	readonly immutable = true;

 	constructor(private readonly options: JobsProtocolOptions) {}

@@ -141,6 +141,7 @@ export function resolveLocalUrlToPath(input: string | InternalUrl, options: Loca
 */
 export class LocalProtocolHandler implements ProtocolHandler {
 	readonly scheme = "local";
+	readonly immutable = false;

 	constructor(private readonly options: LocalProtocolOptions) {}

@@ -106,6 +106,7 @@ function formatAvailableResources(mcpManager: MCPManager): string {
 */
 export class McpProtocolHandler implements ProtocolHandler {
 	readonly scheme = "mcp";
+	readonly immutable = true;

 	constructor(private readonly options: McpProtocolOptions) {}

@@ -70,6 +70,7 @@ export function resolveMemoryUrlToPath(url: InternalUrl, memoryRoot: string): st
 */
 export class MemoryProtocolHandler implements ProtocolHandler {
 	readonly scheme = "memory";
+	readonly immutable = true;

 	constructor(private readonly options: MemoryProtocolOptions) {}

@@ -18,6 +18,7 @@ import type { InternalResource, InternalUrl, ProtocolHandler } from "./types";
 */
 export class PiProtocolHandler implements ProtocolHandler {
 	readonly scheme = "pi";
+	readonly immutable = true;

 	async resolve(url: InternalUrl): Promise<InternalResource> {
 		// Extract filename from host + path
@@ -49,6 +49,7 @@ export class InternalUrlRouter {
 			throw new Error(`Unknown protocol: ${scheme}://\nSupported: ${available || "none"}`);
 		}

-		return handler.resolve(parsed as InternalUrl);
+		const resource = await handler.resolve(parsed as InternalUrl);
+		return { ...resource, immutable: resource.immutable ?? handler.immutable };
 	}
 }
@@ -23,6 +23,7 @@ export interface RuleProtocolOptions {
 */
 export class RuleProtocolHandler implements ProtocolHandler {
 	readonly scheme = "rule";
+	readonly immutable = true;

 	constructor(private readonly options: RuleProtocolOptions) {}

@@ -48,6 +48,7 @@ export function validateRelativePath(relativePath: string): void {
 */
 export class SkillProtocolHandler implements ProtocolHandler {
 	readonly scheme = "skill";
+	readonly immutable = true;

 	constructor(private readonly options: SkillProtocolOptions) {}

@@ -6,7 +6,9 @@
 */

 /**
- * Resolved internal resource returned by protocol handlers.
+ * Raw resource payload returned by protocol handlers. The `immutable` flag is
+ * applied by the router from {@link ProtocolHandler.immutable}, so handlers do
+ * not need to set it themselves.
 */
 export interface InternalResource {
 	/** Canonical URL that was resolved */
@@ -21,6 +23,13 @@ export interface InternalResource {
 	sourcePath?: string;
 	/** Additional notes about resolution */
 	notes?: string[];
+	/**
+	 * True when the resolved content cannot be edited by the agent (e.g. sealed
+	 * artifacts, harness docs, machine-generated memory summaries). Hashline
+	 * anchors and similar edit affordances are suppressed for immutable
+	 * resources. Mutable resources (e.g. local://) behave like editable files.
+	 */
+	immutable?: boolean;
 }

 /**
@@ -44,7 +53,14 @@ export interface ProtocolHandler {
 	/** The scheme this handler processes (without trailing ://) */
 	readonly scheme: string;
 	/**
-	 * Resolve an internal URL to its content.
+	 * Whether resources produced by this handler are immutable (cannot be
+	 * edited by the agent). When true, callers suppress hashline anchors and
+	 * other edit affordances. When false, resources behave like editable files.
+	 */
+	readonly immutable: boolean;
+	/**
+	 * Resolve an internal URL to its content. The router stamps the
+	 * {@link InternalResource.immutable} flag from {@link ProtocolHandler.immutable}.
 	 * @param url Parsed URL object
 	 * @throws Error with user-friendly message if resolution fails
 	 */
@@ -1590,7 +1590,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
 			sourceInternal: url,
 			entityLabel: "resource",
 			ignoreResultLimits: scheme === "skill",
-			immutable: true,
+			immutable: resource.immutable,
 			raw: options?.raw,
 		});
 	}
@@ -121,7 +121,6 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
 			const patternHasNewline = normalizedPattern.includes("\n") || normalizedPattern.includes("\\n");
 			const effectiveMultiline = patternHasNewline;

-			let hasImmutableInput = false;
 			const formatScopePath = (targetPath: string): string => formatPathRelativeToCwd(targetPath, this.session.cwd);
 			let searchPath: string;
 			let scopePath: string;
@@ -134,12 +133,15 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
 			}
 			const internalRouter = this.session.internalRouter;
 			const resolvedPathInputs: string[] = [];
+			// Absolute filesystem paths whose source is immutable (e.g. artifact://,
+			// pi://, skill://). Hashline anchors are suppressed for these on a
+			// per-file basis, leaving editable mixed-in files untouched.
+			const immutableSourcePaths = new Set<string>();
 			for (const rawPath of rawPaths) {
 				if (!internalRouter?.canHandle(rawPath)) {
 					resolvedPathInputs.push(rawPath);
 					continue;
 				}
-				hasImmutableInput = true;
 				if (hasGlobPathChars(rawPath)) {
 					throw new ToolError(`Glob patterns are not supported for internal URLs: ${rawPath}`);
 				}
@@ -147,9 +149,13 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
 				if (!resource.sourcePath) {
 					throw new ToolError(`Cannot search internal URL without a backing file: ${rawPath}`);
 				}
+				if (resource.immutable) {
+					immutableSourcePaths.add(path.resolve(resource.sourcePath));
+				}
 				resolvedPathInputs.push(resource.sourcePath);
 			}
-			const useHashLines = resolveFileDisplayMode(this.session, { immutable: hasImmutableInput }).hashLines;
+			const baseDisplayMode = resolveFileDisplayMode(this.session);
+			const immutableDisplayMode = resolveFileDisplayMode(this.session, { immutable: true });
 			// Tolerate missing entries in a multi-path call: skip ones whose base
 			// directory is gone, and only error if every entry is missing. Single
 			// missing path keeps the original ENOENT semantics.
@@ -338,6 +344,10 @@ export class SearchTool implements AgentTool<typeof searchSchema, SearchToolDeta
 				const modelOut: string[] = [];
 				const displayOut: string[] = [];
 				const fileMatches = matchesByFile.get(relativePath) ?? [];
+				const absoluteFilePath = path.resolve(this.session.cwd, relativePath);
+				const useHashLines = immutableSourcePaths.has(absoluteFilePath)
+					? immutableDisplayMode.hashLines
+					: baseDisplayMode.hashLines;
 				const lineNumberWidth = fileMatches.reduce((width, match) => {
 					let nextWidth = Math.max(width, String(match.lineNumber).length);
 					for (const ctx of match.contextBefore ?? []) {
@@ -24,7 +24,7 @@ function createInternalRouter(resources: Record<string, { sourcePath?: string; e
 	canHandle: (input: string) => boolean;
 	resolve: (
 		input: string,
-	) => Promise<{ url: string; content: string; contentType: "text/plain"; sourcePath?: string }>;
+	) => Promise<{ url: string; content: string; contentType: "text/plain"; sourcePath?: string; immutable: boolean }>;
 } {
 	return {
 		canHandle: input => /^(agent|artifact|plan|memory|rule):\/\//.test(input),
@@ -41,6 +41,7 @@ function createInternalRouter(resources: Record<string, { sourcePath?: string; e
 				content: "",
 				contentType: "text/plain",
 				sourcePath: entry.sourcePath,
+				immutable: true,
 			};
 		},
 	};
@@ -4,6 +4,7 @@ import * as os from "node:os";
 import * as path from "node:path";
 import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
 import { ArtifactProtocolHandler } from "@oh-my-pi/pi-coding-agent/internal-urls/artifact-protocol";
+import { LocalProtocolHandler } from "@oh-my-pi/pi-coding-agent/internal-urls/local-protocol";
 import { InternalUrlRouter } from "@oh-my-pi/pi-coding-agent/internal-urls/router";
 import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
 import { SearchTool } from "@oh-my-pi/pi-coding-agent/tools/search";
@@ -87,6 +88,7 @@ describe("SearchTool internal URL resolution", () => {
 		const router = new InternalUrlRouter();
 		router.register({
 			scheme: "agent",
+			immutable: true,
 			async resolve() {
 				return {
 					url: "agent://0",
@@ -154,6 +156,52 @@ describe("SearchTool internal URL resolution", () => {
 		expect(text).not.toMatch(/^\*?\s*\d+[a-z]{2}\|/m);
 	});

+	it("keeps hashline anchors when searching mutable local:// sources", async () => {
+		const localRoot = path.join(artifactsDir, "local");
+		await fs.mkdir(localRoot, { recursive: true });
+		await Bun.write(path.join(localRoot, "plan.md"), "alpha line\nbeta needle line\ngamma line\n");
+
+		const router = new InternalUrlRouter();
+		router.register(
+			new LocalProtocolHandler({
+				getArtifactsDir: () => artifactsDir,
+				getSessionId: () => "session",
+			}),
+		);
+		const session = createSession({ internalRouter: router, hasEditTool: true });
+		const tool = new SearchTool(session);
+
+		const result = await tool.execute("test-call", {
+			pattern: "needle",
+			paths: ["local://plan.md"],
+		});
+
+		const text = getResultText(result);
+		expect(text).toContain("needle");
+		// Hashline anchor (LINE+ID|content) is kept for mutable local:// sources
+		expect(text).toMatch(/^\*?\s*\d+[a-z]{2}\|/m);
+	});
+
+	it("keeps hashlines on mutable files when mixed with immutable artifact:// inputs", async () => {
+		const content = "alpha line\nbeta needle line\ngamma line\n";
+		await Bun.write(path.join(artifactsDir, "11.bash.log"), content);
+		await Bun.write(path.join(tmpDir, "mixed.txt"), "mixed needle line\n");
+
+		const router = createRouterWithArtifacts();
+		const session = createSession({ internalRouter: router, hasEditTool: true });
+		const tool = new SearchTool(session);
+
+		const result = await tool.execute("test-call", {
+			pattern: "needle",
+			paths: ["artifact://11", "mixed.txt"],
+		});
+
+		const text = getResultText(result);
+		expect(text).toContain("needle");
+		// Mutable mixed.txt keeps hashlines somewhere in the output
+		expect(text).toMatch(/^\*?\s*\d+[a-z]{2}\|.*mixed needle/m);
+	});
+
 	it("throws on nonexistent artifact ID", async () => {
 		const router = createRouterWithArtifacts();
 		const session = createSession({ internalRouter: router });