merged PR #5462: fix(coding-agent): resolve agent:// slash form for nested subagent output

# Conflicts: # packages/coding-agent/src/prompts/system/system-prompt.md
2026-07-16 03:41:51 +02:00
parent 8ff821515d 8c6b2fb450
commit 0533ec005f
5 changed files with 147 additions and 41 deletions
@@ -68,7 +68,7 @@ Settled response (`async.enabled=false`, no job manager, every item's agent `blo

 Artifacts and side channels:
 - Every subagent with an artifacts dir writes `<id>.md`; `agent://<id>` resolves to that file.
- If the output file is JSON, `agent://<id>/<path>` and `agent://<id>?q=<query>` perform JSON extraction.
+- A subagent's own children are dot-qualified (`<id>.<child>`); `agent://<id>/<child>` reads that nested output. When the path names no nested output and the file is JSON, `agent://<id>/<path>` and `agent://<id>?q=<query>` perform JSON extraction.
 - Each subagent gets `<id>.jsonl` session history when the parent persists artifacts; `history://<id>` renders it as a concise transcript (works for live and parked agents).
 - Isolated patch mode writes `<id>.patch` before merge.

@@ -288,6 +288,7 @@
 - Fixed the Model Hub role-assignment strip hiding the selected chip once the row overflowed; the strip now scrolls horizontally, truncating passed chips behind a leading ellipsis so the selection (plus one chip of lookahead) stays visible.
 - Fixed mouse hover and clicks in the /models Roles view landing one row above the pointer (the row mapping subtracted the status row twice).
 - Fixed model search keeping the most-recently-used model on top of the results: match quality now ranks first (an exact `gpt-5.5` beats the active `gpt-5.6-sol`), with MRU order only breaking ties between equally good matches.
+- Fixed the `agent://<parent>/<child>` slash form failing to resolve a nested subagent's output: the path segment was always treated as a jq JSON-extraction key against `<parent>.md`, so a precise-planner reading its own scout child (`agent://Plan/Scout`) got `Not found`. The slash is now a hierarchy separator first (`agent://Parent/Child` → `Parent.Child.md`), falling back to JSON extraction only when no nested output matches the path. ([#5238](https://github.com/can1357/oh-my-pi/issues/5238))

 ## [16.4.5] - 2026-07-11

@@ -66,3 +66,74 @@ it("agent:// resolves a depth-2 subagent's .md output while its session is live
 	const resource = await new AgentProtocolHandler().resolve(new URL(`agent://${grandchildId}`) as never);
 	expect(resource.content).toBe("full report content");
 });
+
+it("agent:// slash form resolves a nested subagent child (hierarchy separator)", async () => {
+	const root = tempDir.path();
+	const rootSessionFile = path.join(root, "slash-session.jsonl");
+	const rootArtifactsDir = rootSessionFile.slice(0, -6);
+	await fs.mkdir(rootArtifactsDir, { recursive: true });
+	const sharedArtifactManager = new ArtifactManager(rootArtifactsDir);
+
+	// Parent subagent adopts the root ArtifactManager; its own children are
+	// written one level deeper under its sessionFile-derived dir, dot-qualified.
+	const parentSessionFile = path.join(rootArtifactsDir, "Parent.jsonl");
+	const parentOwnDir = parentSessionFile.slice(0, -6);
+	await fs.mkdir(parentOwnDir, { recursive: true });
+	await fs.writeFile(path.join(parentOwnDir, "Parent.Child.md"), "child capsule");
+
+	const fakeSession = {
+		sessionManager: { getArtifactsDir: () => sharedArtifactManager.dir },
+	} as unknown as AgentSession;
+	const registry = AgentRegistry.global();
+	registry.register({
+		id: "Main",
+		displayName: "main",
+		kind: "main",
+		session: fakeSession,
+		sessionFile: rootSessionFile,
+	});
+	registry.register({
+		id: "Parent",
+		displayName: "sub",
+		kind: "sub",
+		parentId: "Main",
+		session: fakeSession,
+		sessionFile: parentSessionFile,
+	});
+
+	const handler = new AgentProtocolHandler();
+	// Slash form is a hierarchy hop, not a jq extraction.
+	const slash = await handler.resolve(new URL("agent://Parent/Child") as never);
+	expect(slash.content).toBe("child capsule");
+	expect(slash.contentType).toBe("text/markdown");
+	// The canonical dotted id resolves to the same output.
+	const dotted = await handler.resolve(new URL("agent://Parent.Child") as never);
+	expect(dotted.content).toBe("child capsule");
+});
+
+it("agent:// path form falls back to JSON extraction when no nested output matches", async () => {
+	const root = tempDir.path();
+	const rootSessionFile = path.join(root, "json-session.jsonl");
+	const rootArtifactsDir = rootSessionFile.slice(0, -6);
+	await fs.mkdir(rootArtifactsDir, { recursive: true });
+	const sharedArtifactManager = new ArtifactManager(rootArtifactsDir);
+	await fs.writeFile(path.join(rootArtifactsDir, "Worker.md"), JSON.stringify({ result: { ok: true } }));
+
+	const fakeSession = {
+		sessionManager: { getArtifactsDir: () => sharedArtifactManager.dir },
+	} as unknown as AgentSession;
+	const registry = AgentRegistry.global();
+	registry.register({
+		id: "Main",
+		displayName: "main",
+		kind: "main",
+		session: fakeSession,
+		sessionFile: rootSessionFile,
+	});
+
+	const handler = new AgentProtocolHandler();
+	// `result` names no nested output, so the path extracts JSON from Worker.md.
+	const extracted = await handler.resolve(new URL("agent://Worker/result") as never);
+	expect(extracted.contentType).toBe("application/json");
+	expect(JSON.parse(extracted.content)).toEqual({ ok: true });
+});
@@ -8,7 +8,11 @@
 *
 * URL forms:
 * - agent://<id> - Full output content
- * - agent://<id>/<path> - JSON extraction via path form
+ * - agent://<id>/<child> - Nested subagent output (hierarchy separator; the
+ *   registry allocates a subagent's own children as dot-qualified ids, so
+ *   `agent://Parent/Child` resolves `Parent.Child.md`)
+ * - agent://<id>/<path> - JSON extraction via path form (fallback when no
+ *   nested output matches the path)
 * - agent://<id>?q=<query> - JSON extraction via query form
 */
 import * as fs from "node:fs/promises";
@@ -44,65 +48,56 @@ export class AgentProtocolHandler implements ProtocolHandler {
 		}

 		const dirs = artifactsDirsFromRegistry();
-
 		if (dirs.length === 0) {
 			throw new Error("No session - agent outputs unavailable");
 		}

-		let foundPath: string | undefined;
-		let anyDirExists = false;
-		const availableIds = new Set<string>();
-
-		for (const dir of dirs) {
+		// A subagent allocates its own children as dot-qualified ids
+		// (`Parent.Child`), so the slash path form is first tried as a hierarchy
+		// separator: `agent://Parent/Child` resolves `Parent.Child.md`. Only when
+		// no such nested output exists does the path fall back to jq-style JSON
+		// extraction on `<outputId>.md`. Query form (`?q=`) is always extraction.
+		const pathSegments = hasPathExtraction ? urlPath.split("/").filter(Boolean) : [];
+		const decodedSegments = pathSegments.map(segment => {
 			try {
-				await fs.stat(dir);
-				anyDirExists = true;
-			} catch (err) {
-				if (isEnoent(err)) continue;
-				throw err;
+				return decodeURIComponent(segment);
+			} catch {
+				return segment;
 			}
-			const candidate = path.join(dir, `${outputId}.md`);
-			try {
-				await fs.stat(candidate);
-				foundPath = candidate;
-				break;
-			} catch (err) {
-				if (!isEnoent(err)) throw err;
-				try {
-					const files = await fs.readdir(dir);
-					for (const f of files) {
-						if (f.endsWith(".md")) availableIds.add(f.replace(/\.md$/, ""));
-					}
-				} catch {
-					// Listing failures are non-fatal; continue searching.
-				}
-			}
-		}
+		});
+		const nestedId =
+			decodedSegments.length > 0 && decodedSegments.every(segment => !segment.includes("."))
+				? [outputId, ...decodedSegments].join(".")
+				: undefined;

-		if (!anyDirExists) {
+		const scan = await this.#findOutput(dirs, nestedId ? [nestedId, outputId] : [outputId]);
+		if (!scan.anyDirExists) {
 			throw new Error("No artifacts directory found");
 		}
-
-		if (!foundPath) {
-			const availableStr = availableIds.size > 0 ? [...availableIds].join(", ") : "none";
-			throw new Error(`Not found: ${outputId}\nAvailable: ${availableStr}`);
+		if (!scan.foundPath) {
+			const target = nestedId ?? outputId;
+			const availableStr = scan.availableIds.size > 0 ? [...scan.availableIds].join(", ") : "none";
+			throw new Error(`Not found: ${target}\nAvailable: ${availableStr}`);
 		}

-		const rawContent = await Bun.file(foundPath).text();
+		const rawContent = await Bun.file(scan.foundPath).text();
 		const notes: string[] = [];
 		let content = rawContent;
 		let contentType: InternalResource["contentType"] = "text/markdown";

-		if (hasPathExtraction || hasQueryExtraction) {
+		// Extraction applies only when the URL did NOT resolve to a nested output
+		// (a slash that named a real child is a hierarchy hop, not a jq path).
+		const extract = hasQueryExtraction || (hasPathExtraction && scan.matchedId !== nestedId);
+		if (extract) {
 			let jsonValue: unknown;
 			try {
 				jsonValue = JSON.parse(rawContent);
 			} catch (err) {
 				const message = err instanceof Error ? err.message : String(err);
-				throw new Error(`Output ${outputId} is not valid JSON: ${message}`);
+				throw new Error(`Output ${scan.matchedId} is not valid JSON: ${message}`);
 			}

-			const query = hasPathExtraction ? pathToQuery(urlPath) : queryParam!;
+			const query = hasQueryExtraction ? queryParam! : pathToQuery(urlPath);
 			if (query) {
 				const extracted = applyQuery(jsonValue, query);
 				try {
@@ -122,11 +117,50 @@ export class AgentProtocolHandler implements ProtocolHandler {
 			content,
 			contentType,
 			size: Buffer.byteLength(content, "utf-8"),
-			sourcePath: foundPath,
+			sourcePath: scan.foundPath,
 			notes,
 		};
 	}

+	/**
+	 * Scan every registered artifacts dir for the first `<id>.md` among
+	 * `candidateIds` (tried in order, so a hierarchy match wins over the base
+	 * id). Returns the resolved path and the id it matched, plus the set of
+	 * available ids gathered from the scanned dirs for the not-found message.
+	 */
+	async #findOutput(
+		dirs: string[],
+		candidateIds: string[],
+	): Promise<{ foundPath?: string; matchedId?: string; anyDirExists: boolean; availableIds: Set<string> }> {
+		// Build a full id→path map across every registered dir before picking, so
+		// candidate priority is global: a nested id in a deeper dir must win over
+		// the base id even when the base id's dir is scanned first.
+		const byId = new Map<string, string>();
+		let anyDirExists = false;
+		for (const dir of dirs) {
+			let files: string[];
+			try {
+				files = await fs.readdir(dir);
+			} catch (err) {
+				if (isEnoent(err)) continue;
+				throw err;
+			}
+			anyDirExists = true;
+			for (const f of files) {
+				if (!f.endsWith(".md")) continue;
+				const id = f.slice(0, -3);
+				if (!byId.has(id)) byId.set(id, path.join(dir, f));
+			}
+		}
+		for (const id of candidateIds) {
+			const foundPath = byId.get(id);
+			if (foundPath) {
+				return { foundPath, matchedId: id, anyDirExists, availableIds: new Set(byId.keys()) };
+			}
+		}
+		return { anyDirExists, availableIds: new Set(byId.keys()) };
+	}
+
 	async complete(): Promise<UrlCompletion[]> {
 		const ids = new Set<string>();
 		for (const dir of artifactsDirsFromRegistry()) {
@@ -56,7 +56,7 @@ Special URLs for internal resources; with most FS/bash tools they auto-resolve t
  {{#if hasMemoryRoot}}
 - `memory://root`: project memory summary
  {{/if}}
- `agent://<id>`: agent output artifact; `/<path>` extracts a JSON field
+- `agent://<id>`: agent output artifact; `/<child>` reads a nested subagent's output, else `/<path>` extracts a JSON field
 - `history://<id>`: read-only markdown transcript of an agent (live, parked, or released); bare `history://` lists all agents. Serves any agent whose session file persists on disk, not just registered peers.
 - `artifact://<id>`: artifact content
 - `local://<name>.md`: plan artifacts or shared content for subagents