feat(pi): signal herdr:blocked while a revdiff review owns the terminal

app/plugin_exit_code_test.go

@@ -882,10 +882,12 @@ function fakePi() {
 	const commands = new Map<string, any>();
 	const tools = new Map<string, any>();
 	const sentMessages: string[] = [];
+	const emittedEvents: Array<{ type: string; data: unknown }> = [];
 	return {
 		commands,
 		tools,
 		sentMessages,
+		emittedEvents,
 		registerCommand(name: string, command: any) {
 			commands.set(name, command);
 		},
@@ -895,6 +897,14 @@ function fakePi() {
 		sendUserMessage(message: string) {
 			sentMessages.push(message);
 		},
+		events: {
+			emit(type: string, data: unknown) {
+				emittedEvents.push({ type, data });
+			},
+			on(_type: string, _handler: (data: unknown) => void) {
+				return () => {};
+			},
+		},
 	} as any;
 }
 
@@ -1116,6 +1126,123 @@ async function testArgumentResolution(): Promise<void> {
 	assertArray(shellSplit(shellJoin(roundTrip)), roundTrip, "shellJoin output should shellSplit back to original args");
 }
 
+async function withHerdrEnv<T>(fn: () => Promise<T>): Promise<T> {
+	const old = process.env.HERDR_ENV;
+	process.env.HERDR_ENV = "1";
+	try {
+		return await fn();
+	} finally {
+		if (old === undefined) {
+			delete process.env.HERDR_ENV;
+		} else {
+			process.env.HERDR_ENV = old;
+		}
+	}
+}
+
+async function testHerdrBlockedSignalBracketsReview(): Promise<void> {
+	const tempDir = mkdtempSync(path.join(tmpdir(), "pi-revdiff-blocked-"));
+	const fakeBin = path.join(tempDir, "revdiff");
+	const argFile = path.join(tempDir, "args.txt");
+	writeExecutable(fakeBin, fakeRevdiffScript());
+
+	const oldBin = process.env.REVDIFF_BIN;
+	const oldArgFile = process.env.FAKE_ARG_FILE;
+	process.env.REVDIFF_BIN = fakeBin;
+	process.env.FAKE_ARG_FILE = argFile;
+	try {
+		await withHerdrEnv(async () => {
+			const pi = fakePi();
+			revdiffExtension(pi);
+			await pi.tools.get("revdiff_review").execute("call-1", { args: "--only README.md" }, undefined, undefined, fakeCtx());
+
+			testAssert(pi.emittedEvents.length === 2, "expected exactly one blocked/unblocked pair for a completed review");
+			testAssert(pi.emittedEvents[0].type === "herdr:blocked", "first event should be herdr:blocked");
+			testAssert((pi.emittedEvents[0].data as any).active === true, "review start should signal active: true");
+			testAssert(pi.emittedEvents[1].type === "herdr:blocked", "second event should be herdr:blocked");
+			testAssert((pi.emittedEvents[1].data as any).active === false, "review end should signal active: false");
+		});
+	} finally {
+		if (oldBin === undefined) {
+			delete process.env.REVDIFF_BIN;
+		} else {
+			process.env.REVDIFF_BIN = oldBin;
+		}
+		if (oldArgFile === undefined) {
+			delete process.env.FAKE_ARG_FILE;
+		} else {
+			process.env.FAKE_ARG_FILE = oldArgFile;
+		}
+		rmSync(tempDir, { recursive: true, force: true });
+	}
+}
+
+async function testHerdrBlockedSignalSkippedWithoutHerdrEnv(): Promise<void> {
+	const tempDir = mkdtempSync(path.join(tmpdir(), "pi-revdiff-blocked-off-"));
+	const fakeBin = path.join(tempDir, "revdiff");
+	const argFile = path.join(tempDir, "args.txt");
+	writeExecutable(fakeBin, fakeRevdiffScript());
+
+	const oldBin = process.env.REVDIFF_BIN;
+	const oldArgFile = process.env.FAKE_ARG_FILE;
+	const oldHerdrEnv = process.env.HERDR_ENV;
+	process.env.REVDIFF_BIN = fakeBin;
+	process.env.FAKE_ARG_FILE = argFile;
+	delete process.env.HERDR_ENV;
+	try {
+		const pi = fakePi();
+		revdiffExtension(pi);
+		await pi.tools.get("revdiff_review").execute("call-1", { args: "--only README.md" }, undefined, undefined, fakeCtx());
+		testAssert(pi.emittedEvents.length === 0, "a completed review should emit nothing when HERDR_ENV is unset");
+	} finally {
+		if (oldBin === undefined) {
+			delete process.env.REVDIFF_BIN;
+		} else {
+			process.env.REVDIFF_BIN = oldBin;
+		}
+		if (oldArgFile === undefined) {
+			delete process.env.FAKE_ARG_FILE;
+		} else {
+			process.env.FAKE_ARG_FILE = oldArgFile;
+		}
+		if (oldHerdrEnv === undefined) {
+			delete process.env.HERDR_ENV;
+		} else {
+			process.env.HERDR_ENV = oldHerdrEnv;
+		}
+		rmSync(tempDir, { recursive: true, force: true });
+	}
+}
+
+async function testHerdrBlockedSignalFiresOnceWhenLaunchUnresolved(): Promise<void> {
+	await withHerdrEnv(async () => {
+		const pi = fakePi();
+		revdiffExtension(pi);
+		const result = await pi.tools.get("revdiff_review").execute("call-1", { args: "--output" }, undefined, undefined, fakeCtx());
+
+		testAssert(result.content[0].text === "Could not resolve a revdiff launch target.", "expected the early launch-resolution exit");
+		testAssert(pi.emittedEvents.length === 2, "expected exactly one blocked/unblocked pair even when launch resolution fails");
+		testAssert((pi.emittedEvents[0].data as any).active === true, "unresolved launch should still have signaled active: true first");
+		testAssert((pi.emittedEvents[1].data as any).active === false, "unresolved launch should still signal active: false exactly once");
+	});
+}
+
+async function testHerdrBlockedSignalSkippedForInvalidCwd(): Promise<void> {
+	const tempDir = mkdtempSync(path.join(tmpdir(), "pi-revdiff-blocked-cwd-"));
+	const file = path.join(tempDir, "not-a-dir.txt");
+	testWriteFileSync(file, "not a directory\n");
+	try {
+		await withHerdrEnv(async () => {
+			const pi = fakePi();
+			revdiffExtension(pi);
+			await pi.tools.get("revdiff_review").execute("call-1", { cwd: file }, undefined, undefined, fakeCtx());
+			testAssert(pi.emittedEvents.length === 0, "an unresolved cwd should exit before any blocked signal is emitted");
+		});
+	} finally {
+		rmSync(tempDir, { recursive: true, force: true });
+	}
+}
+
 async function testRefLikePathArgKeepsRef(): Promise<void> {
 	const oldCwd = process.cwd();
 	const repo = initGitRepo();
@@ -1214,6 +1341,10 @@ await testCommandRoutesToSkill();
 await testToolReturnsAnnotations();
 testReviewCwdResolution();
 await testToolRejectsInvalidCwd();
+await testHerdrBlockedSignalBracketsReview();
+await testHerdrBlockedSignalSkippedWithoutHerdrEnv();
+await testHerdrBlockedSignalFiresOnceWhenLaunchUnresolved();
+await testHerdrBlockedSignalSkippedForInvalidCwd();
 await testToolCwdParameter();
 await testSignalTerminatedReviewFails();
 await testArgumentResolution();

plugins/pi/extensions/revdiff.ts

@@ -89,30 +89,58 @@ export default function revdiffExtension(pi: ExtensionAPI): void {
 				return toolTextResult("Could not resolve revdiff working directory.");
 			}
 
-			const launch = await resolveLaunchSpec(params.args?.trim() ?? "", ctx, cwd);
-			if (!launch) {
-				return toolTextResult("Could not resolve a revdiff launch target.");
+			// This call may wait on the user (the ui.select prompt below, or
+			// spawnSync stdio:inherit in runDirectReview) while pi's agent loop is
+			// parked inside this tool call, so terminal multiplexers watching the
+			// agent see "working" even though the session is waiting on the user.
+			// Signal blocked on pi's shared event bus for the whole window;
+			// herdr's omp/pi integration listens for `herdr:blocked` and drives
+			// its sidebar/notifications from it. active:false fires exactly once,
+			// on every exit path.
+			const herdrActive = process.env.HERDR_ENV === "1";
+			if (herdrActive) {
+				pi.events.emit("herdr:blocked", { active: true, label: `revdiff review in ${cwd}` });
 			}
-
-			onUpdate?.({ content: [{ type: "text", text: `Launching revdiff for ${launch.label} in ${cwd}...` }], details: null });
-			const result = await runDirectReview(ctx, launch, cwd);
-			if (!result) {
-				return toolTextResult("revdiff review did not complete.");
-			}
-
-			if (result.annotations.length === 0) {
-				return toolTextResult(`Review complete — no annotations for ${result.label}.`, result);
+			try {
+				const launch = await resolveLaunchSpec(params.args?.trim() ?? "", ctx, cwd);
+				if (!launch) {
+					return toolTextResult("Could not resolve a revdiff launch target.");
+				}
+
+				onUpdate?.({ content: [{ type: "text", text: `Launching revdiff for ${launch.label} in ${cwd}...` }], details: null });
+				if (herdrActive) {
+					// Give the socket write queued by the emit above a couple of
+					// macrotask ticks to run before spawnSync freezes the loop.
+					await flushPendingIOBestEffort();
+				}
+				const result = await runDirectReview(ctx, launch, cwd);
+				if (!result) {
+					return toolTextResult("revdiff review did not complete.");
+				}
+
+				if (result.annotations.length === 0) {
+					return toolTextResult(`Review complete — no annotations for ${result.label}.`, result);
+				}
+
+				const noun = result.annotations.length === 1 ? "annotation" : "annotations";
+				return toolTextResult(
+					[`Captured ${result.annotations.length} ${noun} for ${result.label}.`, "", "Annotations:", result.rawOutput.trim()].join("\n"),
+					result,
+				);
+			} finally {
+				if (herdrActive) {
+					pi.events.emit("herdr:blocked", { active: false });
+				}
 			}
-
-			const noun = result.annotations.length === 1 ? "annotation" : "annotations";
-			return toolTextResult(
-				[`Captured ${result.annotations.length} ${noun} for ${result.label}.`, "", "Annotations:", result.rawOutput.trim()].join("\n"),
-				result,
-			);
 		},
 	});
 }
 
+async function flushPendingIOBestEffort(): Promise<void> {
+	await new Promise<void>((resolve) => setImmediate(resolve));
+	await new Promise<void>((resolve) => setImmediate(resolve));
+}
+
 function toolTextResult(content: string, details?: ReviewResult) {
 	return { content: [{ type: "text" as const, text: content }], details: details ?? null };
 }