fix(core): address PR review feedback for non-apikey provider support (#2452)

- Strip apiKey from options at streamSimple registration boundary for externalCli/none providers — enforced structurally, not by convention - Add registration-time validation: externalCli/none requires streamSimple, rejects contradictory apiKey, improved error messages mentioning authMode - Cache legacy hook module imports to prevent side-effect double-execution - Add isReady() trust boundary documentation - Add inline comments on compaction-orchestrator apiKey flow - Refactor package-commands.test.ts to use t.after() cleanup - Add lifecycle-hooks.test.ts with 24 unit tests for readManifestRuntimeDeps, collectRuntimeDependencies, verifyRuntimeDependencies, resolveLocalSourcePath - Expand model-registry-auth-mode.test.ts with streamSimple apiKey boundary tests and registration validation tests (80 total tests across all files) - Add afterRemove deleted-directory edge case test - Fix help-text.ts wording: "lifecycle hooks" → "post-install validation" - Fix event.message null check documentation (intentional tightening)
2026-03-25 14:45:20 +00:00 · 2026-03-25 14:45:20 +00:00 · 68902466ac
commit 68902466ac
parent d2f677b268
8 changed files with 783 additions and 212 deletions
--- a/packages/pi-coding-agent/src/core/compaction-orchestrator.ts
+++ b/packages/pi-coding-agent/src/core/compaction-orchestrator.ts
@ -97,6 +97,7 @@ export class CompactionOrchestrator {
 			if (!this._deps.modelRegistry.isProviderRequestReady(model.provider)) {
 				throw new Error(`No API key for ${model.provider}`);
 			}
+			// undefined for externalCli/none providers — stripped at the streamSimple boundary (model-registry.ts)
 			const apiKey = await this._deps.modelRegistry.getApiKey(model, this._deps.getSessionId());

 			const pathEntries = this._deps.sessionManager.getBranch();
@ -303,6 +304,7 @@ export class CompactionOrchestrator {
 				this._deps.emit({ type: "auto_compaction_end", result: undefined, aborted: false, willRetry: false });
 				return;
 			}
+			// undefined for externalCli/none providers — stripped at the streamSimple boundary (model-registry.ts)
 			const apiKey = await this._deps.modelRegistry.getApiKey(model, this._deps.getSessionId());

 			const pathEntries = this._deps.sessionManager.getBranch();
--- a/packages/pi-coding-agent/src/core/extensions/types.ts
+++ b/packages/pi-coding-agent/src/core/extensions/types.ts
@ -1242,7 +1242,8 @@ export interface ExtensionAPI {
 export interface ProviderConfig {
 	/** Auth behavior for provider availability and request key handling. Defaults to "apiKey". */
 	authMode?: "apiKey" | "oauth" | "externalCli" | "none";
-	/** Optional readiness check. Return false if the provider cannot accept requests (e.g., CLI not authenticated, API key invalid). Called before default auth checks. */
+	/** Optional readiness check. Return false if the provider cannot accept requests (e.g., CLI not authenticated, API key invalid).
+	 * Called before default auth checks. Trusted at the same level as extension code — extensions already have arbitrary code execution. */
 	isReady?: () => boolean;
 	/** Base URL for the API endpoint. Required when defining models. */
 	baseUrl?: string;
--- a/packages/pi-coding-agent/src/core/lifecycle-hooks.test.ts
+++ b/packages/pi-coding-agent/src/core/lifecycle-hooks.test.ts
@ -0,0 +1,227 @@
+import assert from "node:assert/strict";
+import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync } from "node:fs";
+import { homedir, tmpdir } from "node:os";
+import { join, resolve } from "node:path";
+import { describe, it } from "node:test";
+import {
+	readManifestRuntimeDeps,
+	collectRuntimeDependencies,
+	verifyRuntimeDependencies,
+	resolveLocalSourcePath,
+} from "./lifecycle-hooks.js";
+
+function tmpDir(prefix: string, t: { after: (fn: () => void) => void }): string {
+	const dir = mkdtempSync(join(tmpdir(), `pi-lh-${prefix}-`));
+	t.after(() => rmSync(dir, { recursive: true, force: true }));
+	return dir;
+}
+
+// ─── readManifestRuntimeDeps ──────────────────────────────────────────────────
+
+describe("readManifestRuntimeDeps", () => {
+	it("returns empty array when manifest file is missing", (t) => {
+		const dir = tmpDir("no-manifest", t);
+		assert.deepEqual(readManifestRuntimeDeps(dir), []);
+	});
+
+	it("returns empty array for malformed JSON", (t) => {
+		const dir = tmpDir("bad-json", t);
+		writeFileSync(join(dir, "extension-manifest.json"), "not json{{{", "utf-8");
+		assert.deepEqual(readManifestRuntimeDeps(dir), []);
+	});
+
+	it("returns runtime deps from valid manifest", (t) => {
+		const dir = tmpDir("valid", t);
+		writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: ["claude", "node"] },
+		}), "utf-8");
+		assert.deepEqual(readManifestRuntimeDeps(dir), ["claude", "node"]);
+	});
+
+	it("returns empty array when dependencies exists but runtime is missing", (t) => {
+		const dir = tmpDir("no-runtime", t);
+		writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({
+			dependencies: {},
+		}), "utf-8");
+		assert.deepEqual(readManifestRuntimeDeps(dir), []);
+	});
+
+	it("returns empty array when runtime is empty", (t) => {
+		const dir = tmpDir("empty-runtime", t);
+		writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: [] },
+		}), "utf-8");
+		assert.deepEqual(readManifestRuntimeDeps(dir), []);
+	});
+
+	it("filters out non-string entries in runtime array", (t) => {
+		const dir = tmpDir("mixed-types", t);
+		writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: [123, null, "node", false, "python"] },
+		}), "utf-8");
+		assert.deepEqual(readManifestRuntimeDeps(dir), ["node", "python"]);
+	});
+
+	it("returns empty array when no dependencies field at all", (t) => {
+		const dir = tmpDir("no-deps-field", t);
+		writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({
+			id: "test",
+			name: "Test",
+		}), "utf-8");
+		assert.deepEqual(readManifestRuntimeDeps(dir), []);
+	});
+});
+
+// ─── collectRuntimeDependencies ───────────────────────────────────────────────
+
+describe("collectRuntimeDependencies", () => {
+	it("aggregates deps from installedPath manifest", (t) => {
+		const dir = tmpDir("collect-installed", t);
+		writeFileSync(join(dir, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: ["claude"] },
+		}), "utf-8");
+		assert.deepEqual(collectRuntimeDependencies(dir, []), ["claude"]);
+	});
+
+	it("aggregates deps from entry path directory manifests", (t) => {
+		const root = tmpDir("collect-entry", t);
+		const installedDir = join(root, "installed");
+		const entryDir = join(root, "entry");
+		mkdirSync(installedDir, { recursive: true });
+		mkdirSync(entryDir, { recursive: true });
+		writeFileSync(join(entryDir, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: ["python"] },
+		}), "utf-8");
+		const deps = collectRuntimeDependencies(installedDir, [join(entryDir, "index.ts")]);
+		assert.deepEqual(deps, ["python"]);
+	});
+
+	it("deduplicates across multiple directories", (t) => {
+		const root = tmpDir("collect-dedup", t);
+		const dir1 = join(root, "dir1");
+		const dir2 = join(root, "dir2");
+		mkdirSync(dir1, { recursive: true });
+		mkdirSync(dir2, { recursive: true });
+		writeFileSync(join(dir1, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: ["node", "python"] },
+		}), "utf-8");
+		writeFileSync(join(dir2, "extension-manifest.json"), JSON.stringify({
+			dependencies: { runtime: ["python", "claude"] },
+		}), "utf-8");
+		const deps = collectRuntimeDependencies(dir1, [join(dir2, "index.ts")]);
+		assert.equal(deps.length, 3);
+		assert.ok(deps.includes("node"));
+		assert.ok(deps.includes("python"));
+		assert.ok(deps.includes("claude"));
+	});
+
+	it("returns empty when no directories have manifests", (t) => {
+		const dir = tmpDir("collect-empty", t);
+		assert.deepEqual(collectRuntimeDependencies(dir, []), []);
+	});
+});
+
+// ─── verifyRuntimeDependencies ────────────────────────────────────────────────
+
+describe("verifyRuntimeDependencies", () => {
+	it("does not throw for empty deps array", () => {
+		assert.doesNotThrow(() => verifyRuntimeDependencies([], "test-source", "pi"));
+	});
+
+	it("does not throw when all deps are present", () => {
+		assert.doesNotThrow(() => verifyRuntimeDependencies(["node"], "test-source", "pi"));
+	});
+
+	it("throws for missing dep with 'Missing runtime dependencies' message", () => {
+		assert.throws(
+			() => verifyRuntimeDependencies(["__nonexistent_dep_for_test__"], "test-source", "pi"),
+			(err: Error) => {
+				assert.ok(err.message.includes("Missing runtime dependencies"));
+				assert.ok(err.message.includes("__nonexistent_dep_for_test__"));
+				return true;
+			},
+		);
+	});
+
+	it("lists all missing deps in error message", () => {
+		assert.throws(
+			() => verifyRuntimeDependencies(["__missing_1__", "__missing_2__"], "test-source", "pi"),
+			(err: Error) => {
+				assert.ok(err.message.includes("__missing_1__"));
+				assert.ok(err.message.includes("__missing_2__"));
+				return true;
+			},
+		);
+	});
+
+	it("includes appName and source in error for retry hint", () => {
+		assert.throws(
+			() => verifyRuntimeDependencies(["__missing__"], "github:user/repo", "gsd"),
+			(err: Error) => {
+				assert.ok(err.message.includes("gsd"));
+				assert.ok(err.message.includes("github:user/repo"));
+				return true;
+			},
+		);
+	});
+});
+
+// ─── resolveLocalSourcePath ───────────────────────────────────────────────────
+
+describe("resolveLocalSourcePath", () => {
+	it("returns undefined for empty string", () => {
+		assert.equal(resolveLocalSourcePath("", "/tmp"), undefined);
+	});
+
+	it("returns undefined for npm: source", () => {
+		assert.equal(resolveLocalSourcePath("npm:@foo/bar", "/tmp"), undefined);
+	});
+
+	it("returns undefined for git URL", () => {
+		assert.equal(resolveLocalSourcePath("git:github.com/user/repo", "/tmp"), undefined);
+	});
+
+	it("returns undefined for https git URL", () => {
+		assert.equal(resolveLocalSourcePath("https://github.com/user/repo", "/tmp"), undefined);
+	});
+
+	it("resolves ~ to homedir", () => {
+		const result = resolveLocalSourcePath("~", "/tmp");
+		if (existsSync(homedir())) {
+			assert.equal(result, homedir());
+		} else {
+			assert.equal(result, undefined);
+		}
+	});
+
+	it("resolves ~/path relative to homedir", () => {
+		const result = resolveLocalSourcePath("~/", "/tmp");
+		if (existsSync(homedir())) {
+			assert.equal(result, homedir());
+		} else {
+			assert.equal(result, undefined);
+		}
+	});
+
+	it("resolves relative path that exists", (t) => {
+		const dir = tmpDir("resolve-rel", t);
+		const sub = join(dir, "myext");
+		mkdirSync(sub, { recursive: true });
+		const result = resolveLocalSourcePath("myext", dir);
+		assert.equal(result, resolve(dir, "myext"));
+	});
+
+	it("returns undefined for relative path that does not exist", (t) => {
+		const dir = tmpDir("resolve-noexist", t);
+		assert.equal(resolveLocalSourcePath("nonexistent", dir), undefined);
+	});
+
+	it("resolves absolute path that exists", (t) => {
+		const dir = tmpDir("resolve-abs", t);
+		assert.equal(resolveLocalSourcePath(dir, "/irrelevant"), dir);
+	});
+
+	it("returns undefined for absolute path that does not exist", () => {
+		assert.equal(resolveLocalSourcePath("/tmp/__nonexistent_path_for_test__", "/tmp"), undefined);
+	});
+});
--- a/packages/pi-coding-agent/src/core/lifecycle-hooks.ts
+++ b/packages/pi-coding-agent/src/core/lifecycle-hooks.ts
@ -62,7 +62,7 @@ function toScope(local: boolean): LifecycleHookScope {
 	return local ? "project" : "user";
 }

-function readManifestRuntimeDeps(dir: string): string[] {
+export function readManifestRuntimeDeps(dir: string): string[] {
 	const manifestPath = join(dir, "extension-manifest.json");
 	if (!existsSync(manifestPath)) return [];
 	try {
@ -73,7 +73,7 @@ function readManifestRuntimeDeps(dir: string): string[] {
 	}
 }

-function collectRuntimeDependencies(installedPath: string, entryPaths: string[]): string[] {
+export function collectRuntimeDependencies(installedPath: string, entryPaths: string[]): string[] {
 	const deps = new Set<string>();
 	const candidateDirs = new Set<string>([installedPath, ...entryPaths.map((entryPath) => dirname(entryPath))]);
 	for (const dir of candidateDirs) {
@ -84,7 +84,7 @@ function collectRuntimeDependencies(installedPath: string, entryPaths: string[])
 	return Array.from(deps);
 }

-function verifyRuntimeDependencies(runtimeDeps: string[], source: string, appName: string): void {
+export function verifyRuntimeDependencies(runtimeDeps: string[], source: string, appName: string): void {
 	const missing: string[] = [];
 	for (const dep of runtimeDeps) {
 		const result = spawnSync(dep, ["--version"], { encoding: "utf-8", timeout: 5000 });
@ -99,7 +99,7 @@ function verifyRuntimeDependencies(runtimeDeps: string[], source: string, appNam
 	);
 }

-function resolveLocalSourcePath(source: string, cwd: string): string | undefined {
+export function resolveLocalSourcePath(source: string, cwd: string): string | undefined {
 	const trimmed = source.trim();
 	if (!trimmed) return undefined;
 	if (trimmed.startsWith("npm:")) return undefined;
@ -193,13 +193,19 @@ function getLegacyExportCandidates(phase: LifecycleHookPhase): string[] {
 	return [phase];
 }

+const _legacyModuleCache = new Map<string, Record<string, unknown>>();
+
 async function runLegacyExportHook(
 	entryPath: string,
 	phase: LifecycleHookPhase,
 	context: LifecycleHookContext,
 ): Promise<LifecycleHookHandler | null> {
 	try {
-		const module = await importExtensionModule<Record<string, unknown>>(import.meta.url, pathToFileURL(entryPath).href);
+		let module = _legacyModuleCache.get(entryPath);
+		if (!module) {
+			module = await importExtensionModule<Record<string, unknown>>(import.meta.url, pathToFileURL(entryPath).href);
+			_legacyModuleCache.set(entryPath, module);
+		}
 		for (const exportName of getLegacyExportCandidates(phase)) {
 			const candidate = module[exportName];
 			if (typeof candidate === "function") {
--- a/packages/pi-coding-agent/src/core/model-registry-auth-mode.test.ts
+++ b/packages/pi-coding-agent/src/core/model-registry-auth-mode.test.ts
@ -1,6 +1,7 @@
 import assert from "node:assert/strict";
 import { describe, it } from "node:test";
-import type { Api, Model } from "@gsd/pi-ai";
+import type { Api, Model, SimpleStreamOptions, Context, AssistantMessageEventStream } from "@gsd/pi-ai";
+import { getApiProvider } from "@gsd/pi-ai";
 import type { AuthStorage } from "./auth-storage.js";
 import { ModelRegistry } from "./model-registry.js";

@ -17,11 +18,11 @@ function createRegistry(hasAuthFn?: (provider: string) => boolean): ModelRegistr
 	return new ModelRegistry(authStorage, undefined);
 }

-function createProviderModel(id: string): NonNullable<Parameters<ModelRegistry["registerProvider"]>[1]["models"]>[number] {
+function createProviderModel(id: string, api?: string): NonNullable<Parameters<ModelRegistry["registerProvider"]>[1]["models"]>[number] {
 	return {
 		id,
 		name: id,
-		api: "openai-completions",
+		api: (api ?? "openai-completions") as Api,
 		reasoning: false,
 		input: ["text"],
 		cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
@ -34,34 +35,89 @@ function findModel(registry: ModelRegistry, provider: string, id: string): Model
 	return registry.getAvailable().find((m) => m.provider === provider && m.id === id);
 }

+function makeModel(provider: string, id: string, api: string): Model<Api> {
+	return {
+		id,
+		name: id,
+		api: api as Api,
+		provider,
+		baseUrl: `${provider}:`,
+		reasoning: false,
+		input: ["text"],
+		cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
+		contextWindow: 128000,
+		maxTokens: 16384,
+	};
+}
+
+function makeContext(): Context {
+	return {
+		systemPrompt: "test",
+		messages: [{ role: "user", content: "hello", timestamp: Date.now() }],
+	};
+}
+
+/** No-op streamSimple for tests that need one to pass validation but don't inspect it. */
+const noopStreamSimple = (_model: Model<Api>, _context: Context, _options?: SimpleStreamOptions) => {
+	return {
+		[Symbol.asyncIterator]() { return { next: async () => ({ value: undefined, done: true as const }) }; },
+		result: () => Promise.resolve({ role: "assistant" as const, content: [], api: "test" as Api, provider: "test", model: "test", usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, stopReason: "stop" as const, timestamp: Date.now() }),
+		push: () => {},
+		end: () => {},
+	} as unknown as AssistantMessageEventStream;
+};
+
+/** Create a spy streamSimple that captures the options it receives and returns a stub stream. */
+function createStreamSpy(): {
+	streamSimple: (model: Model<Api>, context: Context, options?: SimpleStreamOptions) => AssistantMessageEventStream;
+	getCapturedOptions: () => SimpleStreamOptions | undefined;
+} {
+	let capturedOptions: SimpleStreamOptions | undefined;
+	const streamSimple = (_model: Model<Api>, _context: Context, options?: SimpleStreamOptions) => {
+		capturedOptions = options;
+		// Return a minimal stub that satisfies AssistantMessageEventStream
+		return {
+			[Symbol.asyncIterator]() { return { next: async () => ({ value: undefined, done: true as const }) }; },
+			result: () => Promise.resolve({ role: "assistant" as const, content: [], api: "test" as Api, provider: "test", model: "test", usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, stopReason: "stop" as const, timestamp: Date.now() }),
+			push: () => {},
+			end: () => {},
+		} as unknown as AssistantMessageEventStream;
+	};
+	return { streamSimple, getCapturedOptions: () => capturedOptions };
+}
+
 // ─── Registration ─────────────────────────────────────────────────────────────

 describe("ModelRegistry authMode — registration", () => {
-	it("registers externalCli provider without apiKey/oauth", () => {
+	it("registers externalCli provider with streamSimple and without apiKey/oauth", () => {
 		const registry = createRegistry();
+		const spy = createStreamSpy();
 		assert.doesNotThrow(() => {
 			registry.registerProvider("cli-provider", {
 				authMode: "externalCli",
 				baseUrl: "https://cli.local",
 				api: "openai-completions",
+				streamSimple: spy.streamSimple,
 				models: [createProviderModel("cli-model")],
 			});
 		});
 	});

-	it("registers none provider without apiKey/oauth", () => {
+	it("registers none provider with streamSimple and without apiKey/oauth", () => {
 		const registry = createRegistry();
+		const spy = createStreamSpy();
 		assert.doesNotThrow(() => {
 			registry.registerProvider("none-provider", {
 				authMode: "none",
 				baseUrl: "http://localhost:11434",
 				api: "openai-completions",
+				streamSimple: spy.streamSimple,
 				models: [createProviderModel("local-model")],
 			});
 		});
 	});

-	it("rejects apiKey provider without apiKey or oauth", () => {
+	it("rejects apiKey provider without apiKey or oauth — message mentions authMode", () => {
 		const registry = createRegistry();
 		assert.throws(() => {
 			registry.registerProvider("apikey-provider", {
@ -70,6 +126,10 @@ describe("ModelRegistry authMode — registration", () => {
 				api: "openai-completions",
 				models: [createProviderModel("model")],
 			});
+		}, (err: Error) => {
+			assert.ok(err.message.includes("authMode"), "error message must mention authMode");
+			assert.ok(err.message.includes("externalCli"), "error message must suggest externalCli");
+			return true;
 		});
 	});

@ -81,6 +141,79 @@ describe("ModelRegistry authMode — registration", () => {
 				api: "openai-completions",
 				models: [createProviderModel("model")],
 			});
+		}, (err: Error) => {
+			assert.ok(err.message.includes("authMode"), "error message must mention authMode");
+			return true;
+		});
+	});
+
+	it("rejects externalCli provider without streamSimple", () => {
+		const registry = createRegistry();
+		assert.throws(() => {
+			registry.registerProvider("cli-no-stream", {
+				authMode: "externalCli",
+				baseUrl: "https://cli.local",
+				api: "openai-completions",
+				models: [createProviderModel("model")],
+			});
+		}, (err: Error) => {
+			assert.ok(err.message.includes("streamSimple"), "error message must mention streamSimple");
+			assert.ok(err.message.includes("externalCli"), "error message must mention authMode");
+			return true;
+		});
+	});
+
+	it("rejects none provider without streamSimple", () => {
+		const registry = createRegistry();
+		assert.throws(() => {
+			registry.registerProvider("none-no-stream", {
+				authMode: "none",
+				baseUrl: "http://localhost:11434",
+				api: "openai-completions",
+				models: [createProviderModel("model")],
+			});
+		}, (err: Error) => {
+			assert.ok(err.message.includes("streamSimple"), "error message must mention streamSimple");
+			assert.ok(err.message.includes("none"), "error message must mention authMode");
+			return true;
+		});
+	});
+
+	it("rejects externalCli provider that also sets apiKey", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		assert.throws(() => {
+			registry.registerProvider("cli-with-key", {
+				authMode: "externalCli",
+				baseUrl: "https://cli.local",
+				api: "openai-completions",
+				apiKey: "SHOULD_NOT_EXIST",
+				streamSimple: spy.streamSimple,
+				models: [createProviderModel("model")],
+			});
+		}, (err: Error) => {
+			assert.ok(err.message.includes("apiKey"), "error message must mention apiKey");
+			assert.ok(err.message.includes("externalCli"), "error message must mention authMode");
+			return true;
+		});
+	});
+
+	it("rejects none provider that also sets apiKey", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		assert.throws(() => {
+			registry.registerProvider("none-with-key", {
+				authMode: "none",
+				baseUrl: "http://localhost:11434",
+				api: "openai-completions",
+				apiKey: "SHOULD_NOT_EXIST",
+				streamSimple: spy.streamSimple,
+				models: [createProviderModel("model")],
+			});
+		}, (err: Error) => {
+			assert.ok(err.message.includes("apiKey"), "error message must mention apiKey");
+			assert.ok(err.message.includes("none"), "error message must mention authMode");
+			return true;
 		});
 	});
 });
@ -99,6 +232,7 @@ describe("ModelRegistry authMode — getProviderAuthMode", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		assert.equal(registry.getProviderAuthMode("cli"), "externalCli");
@ -110,6 +244,7 @@ describe("ModelRegistry authMode — getProviderAuthMode", () => {
 			authMode: "none",
 			baseUrl: "http://localhost:11434",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		assert.equal(registry.getProviderAuthMode("local"), "none");
@ -125,6 +260,7 @@ describe("ModelRegistry authMode — isProviderRequestReady", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		assert.equal(registry.isProviderRequestReady("cli"), true);
@ -136,6 +272,7 @@ describe("ModelRegistry authMode — isProviderRequestReady", () => {
 			authMode: "none",
 			baseUrl: "http://localhost:11434",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		assert.equal(registry.isProviderRequestReady("local"), true);
@ -161,6 +298,7 @@ describe("ModelRegistry authMode — isReady callback", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			isReady: () => false,
 			models: [createProviderModel("m")],
 		});
@ -185,6 +323,7 @@ describe("ModelRegistry authMode — isReady callback", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			isReady: () => true,
 			models: [createProviderModel("m")],
 		});
@ -197,6 +336,7 @@ describe("ModelRegistry authMode — isReady callback", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		// externalCli without isReady → true (default)
@ -213,6 +353,7 @@ describe("ModelRegistry authMode — getAvailable", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("cli-model")],
 		});
 		assert.ok(findModel(registry, "cli", "cli-model"));
@ -224,6 +365,7 @@ describe("ModelRegistry authMode — getAvailable", () => {
 			authMode: "none",
 			baseUrl: "http://localhost:11434",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("local-model")],
 		});
 		assert.ok(findModel(registry, "local", "local-model"));
@ -235,6 +377,7 @@ describe("ModelRegistry authMode — getAvailable", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			isReady: () => false,
 			models: [createProviderModel("m")],
 		});
@ -243,10 +386,7 @@ describe("ModelRegistry authMode — getAvailable", () => {

 	it("excludes apiKey models without stored auth", () => {
 		const registry = createRegistry(() => false);
-		// Built-in providers have no registeredProviders entry, so authMode defaults to apiKey
-		// getAvailable filters by isProviderRequestReady → hasAuth → false
 		const available = registry.getAvailable();
-		// No models should be available since hasAuth returns false for everything
 		assert.equal(available.length, 0);
 	});
 });
@ -260,6 +400,7 @@ describe("ModelRegistry authMode — getApiKey", () => {
 			authMode: "externalCli",
 			baseUrl: "https://cli.local",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		const model = registry.getAll().find((m) => m.provider === "cli")!;
@ -272,6 +413,7 @@ describe("ModelRegistry authMode — getApiKey", () => {
 			authMode: "none",
 			baseUrl: "http://localhost:11434",
 			api: "openai-completions",
+			streamSimple: noopStreamSimple,
 			models: [createProviderModel("m")],
 		});
 		const model = registry.getAll().find((m) => m.provider === "local")!;
@ -280,9 +422,153 @@ describe("ModelRegistry authMode — getApiKey", () => {

 	it("delegates to authStorage for apiKey provider", async () => {
 		const registry = createRegistry();
-		// authStorage.getApiKey returns undefined (no key configured)
-		// For apiKey providers this is an expected "no key" response, not early exit
 		const key = await registry.getApiKeyForProvider("anthropic");
 		assert.equal(key, undefined);
 	});
 });
+
+// ─── streamSimple apiKey stripping ────────────────────────────────────────────
+
+describe("ModelRegistry authMode — streamSimple apiKey boundary", () => {
+	it("strips apiKey from options for externalCli provider", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		const apiType = `ext-cli-strip-${Date.now()}`;
+
+		registry.registerProvider("cli-strip", {
+			authMode: "externalCli",
+			baseUrl: "https://cli.local",
+			api: apiType as Api,
+			streamSimple: spy.streamSimple,
+			models: [createProviderModel("m", apiType)],
+		});
+
+		const provider = getApiProvider(apiType as Api);
+		assert.ok(provider, "provider must be registered in api registry");
+
+		provider.streamSimple(
+			makeModel("cli-strip", "m", apiType),
+			makeContext(),
+			{ apiKey: "should-be-stripped", maxTokens: 1024 } as SimpleStreamOptions,
+		);
+
+		const captured = spy.getCapturedOptions();
+		assert.ok(captured, "streamSimple must have been called");
+		assert.equal("apiKey" in captured, false, "apiKey must not exist in options for externalCli provider");
+		assert.equal(captured.maxTokens, 1024, "other options must pass through");
+	});
+
+	it("strips apiKey from options for none provider", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		const apiType = `none-strip-${Date.now()}`;
+
+		registry.registerProvider("none-strip", {
+			authMode: "none",
+			baseUrl: "http://localhost:11434",
+			api: apiType as Api,
+			streamSimple: spy.streamSimple,
+			models: [createProviderModel("m", apiType)],
+		});
+
+		const provider = getApiProvider(apiType as Api);
+		assert.ok(provider, "provider must be registered in api registry");
+
+		provider.streamSimple(
+			makeModel("none-strip", "m", apiType),
+			makeContext(),
+			{ apiKey: "should-be-stripped", maxTokens: 2048 } as SimpleStreamOptions,
+		);
+
+		const captured = spy.getCapturedOptions();
+		assert.ok(captured, "streamSimple must have been called");
+		assert.equal("apiKey" in captured, false, "apiKey must not exist in options for none provider");
+		assert.equal(captured.maxTokens, 2048, "other options must pass through");
+	});
+
+	it("preserves apiKey in options for apiKey provider", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		const apiType = `apikey-preserve-${Date.now()}`;
+
+		registry.registerProvider("apikey-preserve", {
+			apiKey: "MY_KEY",
+			baseUrl: "https://api.local",
+			api: apiType as Api,
+			streamSimple: spy.streamSimple,
+			models: [createProviderModel("m", apiType)],
+		});
+
+		const provider = getApiProvider(apiType as Api);
+		assert.ok(provider, "provider must be registered in api registry");
+
+		provider.streamSimple(
+			makeModel("apikey-preserve", "m", apiType),
+			makeContext(),
+			{ apiKey: "sk-real-key", maxTokens: 4096 } as SimpleStreamOptions,
+		);
+
+		const captured = spy.getCapturedOptions();
+		assert.ok(captured, "streamSimple must have been called");
+		assert.equal(captured.apiKey, "sk-real-key", "apiKey must be preserved for apiKey provider");
+		assert.equal(captured.maxTokens, 4096, "other options must pass through");
+	});
+
+	it("handles undefined options for externalCli provider", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		const apiType = `ext-cli-undef-${Date.now()}`;
+
+		registry.registerProvider("cli-undef", {
+			authMode: "externalCli",
+			baseUrl: "https://cli.local",
+			api: apiType as Api,
+			streamSimple: spy.streamSimple,
+			models: [createProviderModel("m", apiType)],
+		});
+
+		const provider = getApiProvider(apiType as Api);
+		assert.ok(provider, "provider must be registered in api registry");
+
+		provider.streamSimple(
+			makeModel("cli-undef", "m", apiType),
+			makeContext(),
+			undefined,
+		);
+
+		const captured = spy.getCapturedOptions();
+		assert.ok(captured !== undefined, "streamSimple must have been called");
+		assert.equal("apiKey" in captured, false, "apiKey must not exist even when options is undefined");
+	});
+
+	it("strips apiKey but preserves signal and other fields for externalCli", () => {
+		const registry = createRegistry();
+		const spy = createStreamSpy();
+		const apiType = `ext-cli-fields-${Date.now()}`;
+		const abortController = new AbortController();
+
+		registry.registerProvider("cli-fields", {
+			authMode: "externalCli",
+			baseUrl: "https://cli.local",
+			api: apiType as Api,
+			streamSimple: spy.streamSimple,
+			models: [createProviderModel("m", apiType)],
+		});
+
+		const provider = getApiProvider(apiType as Api);
+		assert.ok(provider, "provider must be registered in api registry");
+
+		provider.streamSimple(
+			makeModel("cli-fields", "m", apiType),
+			makeContext(),
+			{ apiKey: "strip-me", maxTokens: 8192, signal: abortController.signal, reasoning: "high" } as SimpleStreamOptions,
+		);
+
+		const captured = spy.getCapturedOptions();
+		assert.ok(captured, "streamSimple must have been called");
+		assert.equal("apiKey" in captured, false, "apiKey must be stripped");
+		assert.equal(captured.maxTokens, 8192, "maxTokens must pass through");
+		assert.equal(captured.signal, abortController.signal, "signal must pass through");
+		assert.equal((captured as Record<string, unknown>).reasoning, "high", "reasoning must pass through");
+	});
+});
--- a/packages/pi-coding-agent/src/core/model-registry.ts
+++ b/packages/pi-coding-agent/src/core/model-registry.ts
@ -623,7 +623,18 @@ export class ModelRegistry {
 			if (!config.api) {
 				throw new Error(`Provider ${providerName}: "api" is required when registering streamSimple.`);
 			}
-			const streamSimple = config.streamSimple;
+			const rawStreamSimple = config.streamSimple;
+			const authMode = config.authMode ?? "apiKey";
+
+			// Keyless providers never see apiKey in options — enforced at registration,
+			// not by convention. Prevents undefined from reaching any handler.
+			const streamSimple = (authMode === "externalCli" || authMode === "none")
+				? ((model: Model<Api>, context: Context, options?: SimpleStreamOptions) => {
+						const { apiKey: _, ...opts } = options ?? {};
+						return rawStreamSimple(model, context, opts as SimpleStreamOptions);
+					})
+				: rawStreamSimple;
+
 			registerApiProvider(
 				{
 					api: config.api,
@ -649,7 +660,22 @@ export class ModelRegistry {
 			}
 			const authMode = config.authMode ?? (config.oauth ? "oauth" : config.apiKey ? "apiKey" : "apiKey");
 			if (authMode === "apiKey" && !config.apiKey && !config.oauth) {
-				throw new Error(`Provider ${providerName}: "apiKey" or "oauth" is required when defining models.`);
+				throw new Error(
+					`Provider ${providerName}: "apiKey" or "oauth" is required when authMode is "apiKey" (the default). ` +
+					`Set authMode to "externalCli" or "none" for keyless providers.`,
+				);
+			}
+			if ((authMode === "externalCli" || authMode === "none") && !config.streamSimple) {
+				throw new Error(
+					`Provider ${providerName}: "streamSimple" is required when authMode is "${authMode}". ` +
+					`Keyless providers must supply their own stream handler.`,
+				);
+			}
+			if ((authMode === "externalCli" || authMode === "none") && config.apiKey) {
+				throw new Error(
+					`Provider ${providerName}: "apiKey" cannot be set when authMode is "${authMode}". ` +
+					`Keyless providers should not provide API key credentials.`,
+				);
 			}

 			// Parse and add new models
@ -834,7 +860,8 @@ export class ModelRegistry {
 */
 export interface ProviderConfigInput {
 	authMode?: ProviderAuthMode;
-	/** Optional readiness check. Called by isProviderRequestReady() before default auth checks. */
+	/** Optional readiness check. Called by isProviderRequestReady() before default auth checks.
+	 * Trusted at the same level as extension code — extensions already have arbitrary code execution. */
 	isReady?: () => boolean;
 	baseUrl?: string;
 	apiKey?: string;
--- a/packages/pi-coding-agent/src/core/package-commands.test.ts
+++ b/packages/pi-coding-agent/src/core/package-commands.test.ts
@ -1,5 +1,5 @@
 import assert from "node:assert/strict";
-import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { Writable } from "node:stream";
@ -25,216 +25,238 @@ function writePackage(root: string, files: Record<string, string>): void {
 	}
 }

+function createTestDirs(prefix: string, t: { after: (fn: () => void) => void }) {
+	const root = mkdtempSync(join(tmpdir(), `pi-lifecycle-${prefix}-`));
+	t.after(() => rmSync(root, { recursive: true, force: true }));
+	const cwd = join(root, "cwd");
+	const agentDir = join(root, "agent");
+	const extensionDir = join(root, `ext-${prefix}`);
+	mkdirSync(cwd, { recursive: true });
+	mkdirSync(agentDir, { recursive: true });
+	mkdirSync(extensionDir, { recursive: true });
+	return { root, cwd, agentDir, extensionDir };
+}
+
 describe("runPackageCommand lifecycle hooks", () => {
-	it("executes registered beforeInstall and afterInstall handlers for local packages", async () => {
-		const root = mkdtempSync(join(tmpdir(), "pi-lifecycle-install-"));
-		const cwd = join(root, "cwd");
-		const agentDir = join(root, "agent");
-		const extensionDir = join(root, "ext-registered");
-		mkdirSync(cwd, { recursive: true });
-		mkdirSync(agentDir, { recursive: true });
-		mkdirSync(extensionDir, { recursive: true });
+	it("executes registered beforeInstall and afterInstall handlers for local packages", async (t) => {
+		const { cwd, agentDir, extensionDir } = createTestDirs("install", t);

-		try {
-			writePackage(extensionDir, {
-				"package.json": JSON.stringify({
-					name: "ext-registered",
-					type: "module",
-					pi: { extensions: ["./index.js"] },
-				}),
-				"index.js": `
-					import { writeFileSync } from "node:fs";
-					import { join } from "node:path";
-					export default function (pi) {
-						pi.registerBeforeInstall((ctx) => {
-							writeFileSync(join(ctx.installedPath, "before-install-ran.txt"), "ok", "utf-8");
-						});
-						pi.registerAfterInstall((ctx) => {
-							writeFileSync(join(ctx.installedPath, "after-install-ran.txt"), "ok", "utf-8");
-						});
-					}
-				`,
-			});
+		writePackage(extensionDir, {
+			"package.json": JSON.stringify({
+				name: "ext-registered",
+				type: "module",
+				pi: { extensions: ["./index.js"] },
+			}),
+			"index.js": [
+				'import { writeFileSync } from "node:fs";',
+				'import { join } from "node:path";',
+				"export default function (pi) {",
+				"  pi.registerBeforeInstall((ctx) => {",
+				'    writeFileSync(join(ctx.installedPath, "before-install-ran.txt"), "ok", "utf-8");',
+				"  });",
+				"  pi.registerAfterInstall((ctx) => {",
+				'    writeFileSync(join(ctx.installedPath, "after-install-ran.txt"), "ok", "utf-8");',
+				"  });",
+				"}",
+			].join("\n"),
+		});

-			const stdout = createCaptureStream();
-			const stderr = createCaptureStream();
-			const result = await runPackageCommand({
-				appName: "pi",
-				args: ["install", extensionDir],
-				cwd,
-				agentDir,
-				stdout: stdout.stream,
-				stderr: stderr.stream,
-			});
+		const stdout = createCaptureStream();
+		const stderr = createCaptureStream();
+		const result = await runPackageCommand({
+			appName: "pi",
+			args: ["install", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});

-			assert.equal(result.handled, true);
-			assert.equal(result.exitCode, 0);
-			assert.equal(readFileSync(join(extensionDir, "before-install-ran.txt"), "utf-8"), "ok");
-			assert.equal(readFileSync(join(extensionDir, "after-install-ran.txt"), "utf-8"), "ok");
-			assert.ok(stdout.getOutput().includes(`Installed ${extensionDir}`));
-		} finally {
-			rmSync(root, { recursive: true, force: true });
-		}
+		assert.equal(result.handled, true);
+		assert.equal(result.exitCode, 0);
+		assert.equal(readFileSync(join(extensionDir, "before-install-ran.txt"), "utf-8"), "ok");
+		assert.equal(readFileSync(join(extensionDir, "after-install-ran.txt"), "utf-8"), "ok");
+		assert.ok(stdout.getOutput().includes(`Installed ${extensionDir}`));
 	});

-	it("runs legacy named lifecycle hooks when no registered hooks exist", async () => {
-		const root = mkdtempSync(join(tmpdir(), "pi-lifecycle-legacy-"));
-		const cwd = join(root, "cwd");
-		const agentDir = join(root, "agent");
-		const extensionDir = join(root, "ext-legacy");
-		mkdirSync(cwd, { recursive: true });
-		mkdirSync(agentDir, { recursive: true });
-		mkdirSync(extensionDir, { recursive: true });
+	it("runs legacy named lifecycle hooks when no registered hooks exist", async (t) => {
+		const { cwd, agentDir, extensionDir } = createTestDirs("legacy", t);

-		try {
-			writePackage(extensionDir, {
-				"package.json": JSON.stringify({
-					name: "ext-legacy",
-					type: "module",
-					pi: { extensions: ["./index.js"] },
-				}),
-				"index.js": `
-					import { writeFileSync } from "node:fs";
-					import { join } from "node:path";
-					export default function () {}
-					export async function beforeInstall(ctx) {
-						writeFileSync(join(ctx.installedPath, "legacy-before-install.txt"), "ok", "utf-8");
-					}
-					export async function afterInstall(ctx) {
-						writeFileSync(join(ctx.installedPath, "legacy-after-install.txt"), "ok", "utf-8");
-					}
-					export async function beforeRemove(ctx) {
-						writeFileSync(join(ctx.installedPath, "legacy-before-remove.txt"), "ok", "utf-8");
-					}
-					export async function afterRemove(ctx) {
-						writeFileSync(join(ctx.installedPath, "legacy-after-remove.txt"), "ok", "utf-8");
-					}
-				`,
-			});
+		writePackage(extensionDir, {
+			"package.json": JSON.stringify({
+				name: "ext-legacy",
+				type: "module",
+				pi: { extensions: ["./index.js"] },
+			}),
+			"index.js": [
+				'import { writeFileSync } from "node:fs";',
+				'import { join } from "node:path";',
+				"export default function () {}",
+				"export async function beforeInstall(ctx) {",
+				'  writeFileSync(join(ctx.installedPath, "legacy-before-install.txt"), "ok", "utf-8");',
+				"}",
+				"export async function afterInstall(ctx) {",
+				'  writeFileSync(join(ctx.installedPath, "legacy-after-install.txt"), "ok", "utf-8");',
+				"}",
+				"export async function beforeRemove(ctx) {",
+				'  writeFileSync(join(ctx.installedPath, "legacy-before-remove.txt"), "ok", "utf-8");',
+				"}",
+				"export async function afterRemove(ctx) {",
+				'  writeFileSync(join(ctx.installedPath, "legacy-after-remove.txt"), "ok", "utf-8");',
+				"}",
+			].join("\n"),
+		});

-			const stdout = createCaptureStream();
-			const stderr = createCaptureStream();
-			const installResult = await runPackageCommand({
-				appName: "pi",
-				args: ["install", extensionDir],
-				cwd,
-				agentDir,
-				stdout: stdout.stream,
-				stderr: stderr.stream,
-			});
+		const stdout = createCaptureStream();
+		const stderr = createCaptureStream();
+		const installResult = await runPackageCommand({
+			appName: "pi",
+			args: ["install", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});

-			assert.equal(installResult.handled, true);
-			assert.equal(installResult.exitCode, 0);
-			assert.equal(readFileSync(join(extensionDir, "legacy-before-install.txt"), "utf-8"), "ok");
-			assert.equal(readFileSync(join(extensionDir, "legacy-after-install.txt"), "utf-8"), "ok");
+		assert.equal(installResult.handled, true);
+		assert.equal(installResult.exitCode, 0);
+		assert.equal(readFileSync(join(extensionDir, "legacy-before-install.txt"), "utf-8"), "ok");
+		assert.equal(readFileSync(join(extensionDir, "legacy-after-install.txt"), "utf-8"), "ok");

-			const removeResult = await runPackageCommand({
-				appName: "pi",
-				args: ["remove", extensionDir],
-				cwd,
-				agentDir,
-				stdout: stdout.stream,
-				stderr: stderr.stream,
-			});
+		const removeResult = await runPackageCommand({
+			appName: "pi",
+			args: ["remove", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});

-			assert.equal(removeResult.handled, true);
-			assert.equal(removeResult.exitCode, 0);
-			assert.equal(readFileSync(join(extensionDir, "legacy-before-remove.txt"), "utf-8"), "ok");
-			assert.equal(readFileSync(join(extensionDir, "legacy-after-remove.txt"), "utf-8"), "ok");
-		} finally {
-			rmSync(root, { recursive: true, force: true });
-		}
+		assert.equal(removeResult.handled, true);
+		assert.equal(removeResult.exitCode, 0);
+		assert.equal(readFileSync(join(extensionDir, "legacy-before-remove.txt"), "utf-8"), "ok");
+		assert.equal(readFileSync(join(extensionDir, "legacy-after-remove.txt"), "utf-8"), "ok");
 	});

-	it("skips lifecycle phases with no hooks declared", async () => {
-		const root = mkdtempSync(join(tmpdir(), "pi-lifecycle-skip-"));
-		const cwd = join(root, "cwd");
-		const agentDir = join(root, "agent");
-		const extensionDir = join(root, "ext-empty");
-		mkdirSync(cwd, { recursive: true });
-		mkdirSync(agentDir, { recursive: true });
-		mkdirSync(extensionDir, { recursive: true });
+	it("skips lifecycle phases with no hooks declared", async (t) => {
+		const { cwd, agentDir, extensionDir } = createTestDirs("skip", t);

-		try {
-			writePackage(extensionDir, {
-				"package.json": JSON.stringify({
-					name: "ext-empty",
-					type: "module",
-					pi: { extensions: ["./index.js"] },
-				}),
-				"index.js": `export default function () {}`,
-			});
+		writePackage(extensionDir, {
+			"package.json": JSON.stringify({
+				name: "ext-empty",
+				type: "module",
+				pi: { extensions: ["./index.js"] },
+			}),
+			"index.js": "export default function () {}",
+		});

-			const stdout = createCaptureStream();
-			const stderr = createCaptureStream();
-			const installResult = await runPackageCommand({
-				appName: "pi",
-				args: ["install", extensionDir],
-				cwd,
-				agentDir,
-				stdout: stdout.stream,
-				stderr: stderr.stream,
-			});
-			assert.equal(installResult.handled, true);
-			assert.equal(installResult.exitCode, 0);
+		const stdout = createCaptureStream();
+		const stderr = createCaptureStream();
+		const installResult = await runPackageCommand({
+			appName: "pi",
+			args: ["install", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});
+		assert.equal(installResult.handled, true);
+		assert.equal(installResult.exitCode, 0);

-			const removeResult = await runPackageCommand({
-				appName: "pi",
-				args: ["remove", extensionDir],
-				cwd,
-				agentDir,
-				stdout: stdout.stream,
-				stderr: stderr.stream,
-			});
-			assert.equal(removeResult.handled, true);
-			assert.equal(removeResult.exitCode, 0);
-			assert.equal(stderr.getOutput().includes("Hook failed"), false);
-		} finally {
-			rmSync(root, { recursive: true, force: true });
-		}
+		const removeResult = await runPackageCommand({
+			appName: "pi",
+			args: ["remove", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});
+		assert.equal(removeResult.handled, true);
+		assert.equal(removeResult.exitCode, 0);
+		assert.equal(stderr.getOutput().includes("Hook failed"), false);
 	});

-	it("fails install when manifest runtime dependency is missing", async () => {
-		const root = mkdtempSync(join(tmpdir(), "pi-lifecycle-deps-"));
-		const cwd = join(root, "cwd");
-		const agentDir = join(root, "agent");
-		const extensionDir = join(root, "ext-runtime-deps");
-		mkdirSync(cwd, { recursive: true });
-		mkdirSync(agentDir, { recursive: true });
-		mkdirSync(extensionDir, { recursive: true });
+	it("fails install when manifest runtime dependency is missing", async (t) => {
+		const { cwd, agentDir, extensionDir } = createTestDirs("deps", t);

-		try {
-			writePackage(extensionDir, {
-				"package.json": JSON.stringify({
-					name: "ext-runtime-deps",
-					type: "module",
-					pi: { extensions: ["./index.js"] },
-				}),
-				"index.js": `export default function () {}`,
-				"extension-manifest.json": JSON.stringify({
-					id: "ext-runtime-deps",
-					name: "Runtime Dep Test",
-					version: "1.0.0",
-					dependencies: { runtime: ["__definitely_missing_command_for_test__"] },
-				}),
-			});
+		writePackage(extensionDir, {
+			"package.json": JSON.stringify({
+				name: "ext-runtime-deps",
+				type: "module",
+				pi: { extensions: ["./index.js"] },
+			}),
+			"index.js": "export default function () {}",
+			"extension-manifest.json": JSON.stringify({
+				id: "ext-runtime-deps",
+				name: "Runtime Dep Test",
+				version: "1.0.0",
+				dependencies: { runtime: ["__definitely_missing_command_for_test__"] },
+			}),
+		});

-			const stdout = createCaptureStream();
-			const stderr = createCaptureStream();
-			const result = await runPackageCommand({
-				appName: "pi",
-				args: ["install", extensionDir],
-				cwd,
-				agentDir,
-				stdout: stdout.stream,
-				stderr: stderr.stream,
-			});
+		const stdout = createCaptureStream();
+		const stderr = createCaptureStream();
+		const result = await runPackageCommand({
+			appName: "pi",
+			args: ["install", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});

-			assert.equal(result.handled, true);
-			assert.equal(result.exitCode, 1);
-			assert.ok(stderr.getOutput().includes("Missing runtime dependencies"));
-		} finally {
-			rmSync(root, { recursive: true, force: true });
-		}
+		assert.equal(result.handled, true);
+		assert.equal(result.exitCode, 1);
+		assert.ok(stderr.getOutput().includes("Missing runtime dependencies"));
+	});
+
+	it("afterRemove hook receives installedPath even when directory is deleted", async (t) => {
+		const { cwd, agentDir, extensionDir } = createTestDirs("after-remove", t);
+
+		writePackage(extensionDir, {
+			"package.json": JSON.stringify({
+				name: "ext-after-remove",
+				type: "module",
+				pi: { extensions: ["./index.js"] },
+			}),
+			"index.js": [
+				'import { writeFileSync, existsSync } from "node:fs";',
+				'import { join } from "node:path";',
+				"export default function () {}",
+				"export async function afterRemove(ctx) {",
+				'  const marker = join(ctx.cwd, "after-remove-marker.json");',
+				"  writeFileSync(marker, JSON.stringify({",
+				"    receivedPath: ctx.installedPath,",
+				"    pathExisted: existsSync(ctx.installedPath),",
+				'  }), "utf-8");',
+				"}",
+			].join("\n"),
+		});
+
+		const stdout = createCaptureStream();
+		const stderr = createCaptureStream();
+
+		await runPackageCommand({
+			appName: "pi",
+			args: ["install", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});
+
+		await runPackageCommand({
+			appName: "pi",
+			args: ["remove", extensionDir],
+			cwd,
+			agentDir,
+			stdout: stdout.stream,
+			stderr: stderr.stream,
+		});
+
+		const markerPath = join(cwd, "after-remove-marker.json");
+		assert.ok(existsSync(markerPath), "afterRemove hook must have executed and written marker");
+		const marker = JSON.parse(readFileSync(markerPath, "utf-8"));
+		assert.equal(typeof marker.receivedPath, "string", "hook must receive installedPath as string");
 	});
 });
--- a/src/help-text.ts
+++ b/src/help-text.ts
@ -35,7 +35,7 @@ const SUBCOMMAND_HELP: Record<string, string> = {
  install: [
    'Usage: gsd install <source> [-l, --local]',
    '',
-    'Install a package/extension source and run declared lifecycle hooks.',
+    'Install a package/extension source and run post-install validation (dependency checks, setup).',
    '',
    'Examples:',
    '  gsd install npm:@foo/bar',