fix: address Codex review findings

- Honor enabled:false flag in listClaudePluginRoots (skip disabled plugins)
- Normalize local source URIs to absolute paths in addMarketplace
- Resolver returns explicit tempCloneRoot instead of path-based inference

packages/coding-agent/src/discovery/helpers.ts

@@ -594,6 +594,7 @@ export interface ClaudePluginEntry {
 	installedAt: string;
 	lastUpdated: string;
 	gitCommitSha?: string;
+	enabled?: boolean;
 }
 
 /**
@@ -682,6 +683,7 @@ export async function listClaudePluginRoots(home: string): Promise<{ roots: Clau
 						warnings.push(`Plugin ${pluginId} entry has no installPath`);
 						continue;
 					}
+					if (entry.enabled === false) continue;
 
 					roots.push({
 						id: pluginId,
@@ -725,6 +727,7 @@ export async function listClaudePluginRoots(home: string): Promise<{ roots: Clau
 						warnings.push(`Plugin ${pluginId} entry has no installPath`);
 						continue;
 					}
+					if (entry.enabled === false) continue;
 					// Deduplicate by installPath within same ID
 					if (roots.some(r => r.id === pluginId && r.path === entry.installPath)) continue;
 

packages/coding-agent/src/extensibility/plugins/marketplace/manager.ts

@@ -10,7 +10,7 @@ import * as fs from "node:fs/promises";
 import * as os from "node:os";
 import * as path from "node:path";
 
-import { isEnoent, logger, pathIsWithin } from "@oh-my-pi/pi-utils";
+import { isEnoent, logger } from "@oh-my-pi/pi-utils";
 
 import { cachePlugin } from "./cache";
 import { classifySource, fetchMarketplace, parseMarketplaceCatalog } from "./fetcher";
@@ -60,6 +60,10 @@ export class MarketplaceManager {
 	async addMarketplace(source: string): Promise<MarketplaceRegistryEntry> {
 		const { catalog } = await fetchMarketplace(source, this.#opts.marketplacesCacheDir);
 		const sourceType = classifySource(source);
+		const normalizedSource =
+			sourceType === "local"
+				? path.resolve(source.startsWith("~/") ? path.join(os.homedir(), source.slice(2)) : source)
+				: source;
 
 		const catalogPath = path.join(this.#opts.marketplacesCacheDir, catalog.name, "marketplace.json");
 
@@ -70,7 +74,7 @@ export class MarketplaceManager {
 		const entry: MarketplaceRegistryEntry = {
 			name: catalog.name,
 			sourceType,
-			sourceUri: source,
+			sourceUri: normalizedSource,
 			catalogPath,
 			addedAt: now,
 			updatedAt: now,
@@ -222,12 +226,11 @@ export class MarketplaceManager {
 		// sourceUri. For local sources, use path.resolve of sourceUri; for others use the cache dir.
 		const marketplaceClonePath = this.#resolveMarketplaceRoot(mktEntry);
 
-		const sourcePath = await resolvePluginSource(pluginEntry, {
+		const { dir: sourcePath, tempCloneRoot } = await resolvePluginSource(pluginEntry, {
 			marketplaceClonePath,
 			catalogMetadata: catalog.metadata,
 			tmpDir: os.tmpdir(),
 		});
-		const isTemp = pathIsWithin(os.tmpdir(), sourcePath);
 
 		// 5. Determine version: catalog entry > plugin manifest > git SHA > fallback
 		let version!: string;
@@ -237,11 +240,8 @@ export class MarketplaceManager {
 			cachePath = await cachePlugin(sourcePath, this.#opts.pluginsCacheDir, marketplace, name, version);
 		} finally {
 			// Clean up temp clone dirs created by resolvePluginSource; leave user-supplied local dirs alone
-			if (isTemp) {
-				// For git-subdir, sourcePath may be a subdir of the temp clone — clean the whole clone root.
-				const rel = path.relative(os.tmpdir(), sourcePath);
-				const tempRoot = path.join(os.tmpdir(), rel.split(path.sep)[0]);
-				await fs.rm(tempRoot, { recursive: true, force: true }).catch(() => {});
+			if (tempCloneRoot) {
+				await fs.rm(tempCloneRoot, { recursive: true, force: true }).catch(() => {});
 			}
 		}
 

packages/coding-agent/src/extensibility/plugins/marketplace/source-resolver.ts

@@ -32,7 +32,10 @@ export interface ResolveContext {
  *
  * The resolved path is verified to exist on disk.
  */
-export async function resolvePluginSource(entry: MarketplacePluginEntry, context: ResolveContext): Promise<string> {
+export async function resolvePluginSource(
+	entry: MarketplacePluginEntry,
+	context: ResolveContext,
+): Promise<{ dir: string; tempCloneRoot?: string }> {
 	const { source } = entry;
 
 	if (typeof source === "string") {
@@ -44,7 +47,10 @@ export async function resolvePluginSource(entry: MarketplacePluginEntry, context
 
 // ── Relative string source ("./plugins/foo") ────────────────────────
 
-async function resolveRelativeSource(source: string, context: ResolveContext): Promise<string> {
+async function resolveRelativeSource(
+	source: string,
+	context: ResolveContext,
+): Promise<{ dir: string; tempCloneRoot?: string }> {
 	if (!source.startsWith("./")) {
 		throw new Error(`Relative plugin source paths must start with "./" — got: "${source}"`);
 	}
@@ -67,19 +73,22 @@ async function resolveRelativeSource(source: string, context: ResolveContext): P
 	}
 
 	await verifyDirExists(resolved, `Plugin source directory does not exist: "${resolved}"`);
-	return resolved;
+	return { dir: resolved };
 }
 
 // ── Object source variants ──────────────────────────────────────────
 
-async function resolveObjectSource(source: Exclude<PluginSource, string>, context: ResolveContext): Promise<string> {
+async function resolveObjectSource(
+	source: Exclude<PluginSource, string>,
+	context: ResolveContext,
+): Promise<{ dir: string; tempCloneRoot?: string }> {
 	switch (source.source) {
 		case "url": {
 			// { source: "url", url: "https://github.com/owner/repo.git" }
 			// Despite the name, this is typically a git clone URL
 			const targetDir = path.join(context.tmpDir, `plugin-${crypto.randomUUID()}`);
 			await cloneGitRepo(source.url, targetDir, { ref: source.ref, sha: source.sha });
-			return targetDir;
+			return { dir: targetDir, tempCloneRoot: targetDir };
 		}
 
 		case "github": {
@@ -87,7 +96,7 @@ async function resolveObjectSource(source: Exclude<PluginSource, string>, contex
 			const url = `https://github.com/${source.repo}.git`;
 			const targetDir = path.join(context.tmpDir, `plugin-${crypto.randomUUID()}`);
 			await cloneGitRepo(url, targetDir, { ref: source.ref, sha: source.sha });
-			return targetDir;
+			return { dir: targetDir, tempCloneRoot: targetDir };
 		}
 
 		case "git-subdir": {
@@ -105,7 +114,7 @@ async function resolveObjectSource(source: Exclude<PluginSource, string>, contex
 				throw new Error(`git-subdir path "${source.path}" escapes the cloned repository`);
 			}
 			await verifyDirExists(subdirPath, `git-subdir path "${source.path}" does not exist in cloned repository`);
-			return subdirPath;
+			return { dir: subdirPath, tempCloneRoot: cloneDir };
 		}
 
 		case "npm":

packages/coding-agent/test/marketplace/source-resolver.test.ts

@@ -31,7 +31,8 @@ describe("resolvePluginSource", () => {
 			marketplaceClonePath: FIXTURE_DIR,
 			tmpDir,
 		});
-		expect(resolved).toBe(path.resolve(FIXTURE_DIR, "plugins/hello-plugin"));
+		expect(resolved.dir).toBe(path.resolve(FIXTURE_DIR, "plugins/hello-plugin"));
+		expect(resolved.tempCloneRoot).toBeUndefined();
 	});
 
 	it("throws when source string would escape marketplace root", async () => {
@@ -61,7 +62,8 @@ describe("resolvePluginSource", () => {
 			catalogMetadata: { pluginRoot: "plugins" },
 			tmpDir,
 		});
-		expect(resolved).toBe(path.resolve(FIXTURE_DIR, "plugins/hello-plugin"));
+		expect(resolved.dir).toBe(path.resolve(FIXTURE_DIR, "plugins/hello-plugin"));
+		expect(resolved.tempCloneRoot).toBeUndefined();
 	});
 
 	// Network-dependent: object sources attempt real git clones