Merge PR #8414: fix(models): isolate ambient hooks from catalog listing (@starlink-awaken)

packages/coding-agent/src/cli/models-cli.ts

@@ -304,7 +304,7 @@ export async function runModelsListing(options: RunModelsListingOptions): Promis
 		cwd,
 		eventBus,
 		disableExtensionDiscovery ? undefined : disabledExtensionIds,
-		{ ambient: !disableExtensionDiscovery },
+		{ ambient: !disableExtensionDiscovery, includeAmbientHooks: false },
 	);
 	const extensionRunner =
 		extensionsResult.extensions.length > 0

packages/coding-agent/src/extensibility/extensions/loader.ts

@@ -613,6 +613,8 @@ async function discoverHooksInPackageRoot(root: string): Promise<string[]> {
 export interface DiscoverExtensionPathOptions {
 	/** Include ambient native extensions, hooks, and installed plugins. */
 	ambient?: boolean;
+	/** Include ambient hook factories. Disable for read-only catalog commands. */
+	includeAmbientHooks?: boolean;
 }
 
 export async function discoverExtensionPaths(
@@ -664,7 +666,7 @@ export async function discoverExtensionPaths(
 	// runner, which owns the current runtime event bus. Non-ambient discovery
 	// scans only this invocation's configured package roots; it must not consult
 	// settings, installed packages, or process-global CLI injection state.
-	if (ambient) {
+	if (ambient && options.includeAmbientHooks !== false) {
 		const hooks = await loadCapability<Hook>(hookCapability.id, loadOptions);
 		for (const hookPath of hooks.items
 			.map(hook => hook.path)

packages/coding-agent/test/extensions-discovery.test.ts

@@ -698,6 +698,28 @@ describe("extensions discovery", () => {
 		expect(loadedHook?.handlers.has("tool_call")).toBe(true);
 	});
 
+	it("can exclude ambient hooks without disabling native provider extensions", async () => {
+		const hookDir = path.join(getProjectAgentDir(tempDir.path()), "hooks", "pre");
+		fs.mkdirSync(hookDir, { recursive: true });
+		const hookPath = path.join(hookDir, "models-poison.ts");
+		fs.writeFileSync(
+			hookPath,
+			`export default function(pi) {
+				pi.on("tool_call", async () => ({ block: true, reason: "blocked by hook" }));
+			}`,
+		);
+		const nativeExtensionPath = path.join(extensionsDir, "provider.ts");
+		fs.writeFileSync(nativeExtensionPath, extensionCode);
+
+		const paths = await discoverExtensionPaths([], tempDir.path(), undefined, {
+			ambient: true,
+			includeAmbientHooks: false,
+		});
+
+		expect(paths).toContain(nativeExtensionPath);
+		expect(paths).not.toContain(hookPath);
+	});
+
 	it("keeps discovered hooks separate from disabled extension-module ids", async () => {
 		const extensionPath = path.join(extensionsDir, "guard.ts");
 		fs.writeFileSync(extensionPath, extensionCode);

packages/coding-agent/test/issue-905-repro.test.ts

@@ -14,15 +14,17 @@
 
 import { afterAll, beforeAll, expect, test } from "bun:test";
 import * as fs from "node:fs/promises";
+import * as path from "node:path";
 import { AuthStorage } from "@oh-my-pi/pi-ai";
 import { runModelsListing } from "@oh-my-pi/pi-coding-agent/cli/models-cli";
 import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
-import { TempDir } from "@oh-my-pi/pi-utils";
+import { getProjectAgentDir, TempDir } from "@oh-my-pi/pi-utils";
 
 let tmp: TempDir;
 let extPath: string;
 let explicitPackagePath: string;
 let ambientExtPath: string;
+let ambientHookMarkerPath: string;
 let dbPath: string;
 let shutdownExtPath: string;
 let shutdownPath: string;
@@ -64,7 +66,16 @@ beforeAll(async () => {
 	);
 	explicitPackagePath = tmp.join("explicit-package");
 	ambientExtPath = tmp.join("ambient.ts");
+	ambientHookMarkerPath = tmp.join("ambient-hook-loaded");
 	await fs.mkdir(tmp.join("explicit-package", "src"), { recursive: true });
+	const hookDir = path.join(getProjectAgentDir(tmp.path()), "hooks", "pre");
+	await fs.mkdir(hookDir, { recursive: true });
+	await fs.writeFile(
+		path.join(hookDir, "models-poison.ts"),
+		`await Bun.write(${JSON.stringify(ambientHookMarkerPath)}, "loaded");
+export default function () {}
+`,
+	);
 	await fs.writeFile(
 		tmp.join("explicit-package", "package.json"),
 		JSON.stringify({ name: "explicit-package", omp: { extensions: ["./src/main.ts"] } }),
@@ -147,6 +158,37 @@ test("omp models surfaces extension-registered providers (issue #905)", async ()
 	}
 });
 
+test("omp models does not execute ambient hooks while retaining explicit providers", async () => {
+	const authStorage = await AuthStorage.create(":memory:");
+	try {
+		const modelRegistry = new ModelRegistry(authStorage);
+		const captured: string[] = [];
+		const originalWrite = process.stdout.write.bind(process.stdout);
+		process.stdout.write = ((chunk: string | Uint8Array) => {
+			captured.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"));
+			return true;
+		}) as typeof process.stdout.write;
+
+		try {
+			await runModelsListing({
+				modelRegistry,
+				cwd: tmp.path(),
+				action: "ls",
+				additionalExtensionPaths: [extPath],
+			});
+		} finally {
+			process.stdout.write = originalWrite;
+		}
+
+		const output = captured.join("");
+		expect(output).toContain("test-gw");
+		expect(output).toContain("test-model");
+		expect(await Bun.file(ambientHookMarkerPath).exists()).toBe(false);
+	} finally {
+		authStorage.close();
+	}
+});
+
 test("omp models emits extension shutdown after listing (issue #6297)", async () => {
 	const authStorage = await AuthStorage.create(":memory:");
 	try {