Merge PR #8826: fix: resolve workspace-member imports in installed git-dep monorepo plugins (@sjawhar)

2026-08-19 01:36:59 +02:00
parent 83e2ab8b2a b8db96e9fe
commit 9cf0ab1e8a
3 changed files with 148 additions and 0 deletions
@@ -17,6 +17,9 @@
 ### Added

 - Added `ExtensionAPI.registerFileWriteFallback(handler)` and `ExtensionAPI.registerFileDeleteFallback(handler)`, letting an extension supply a fallback writer or deleter that is consulted when a native `write`, `edit`, or `apply_patch` byte-write or unlink is denied with a permission error (`EPERM`/`EACCES`/`EROFS`) — for hosts that embed the agent inside a sandbox that denies direct filesystem access but exposes a privileged channel. The brokered path is symlink-resolved so a handler's allowlist sees the real destination, a destination that cannot be resolved is not brokered at all, and `req.sessionId` names the session that issued the mutation so a handler sharing the process-wide registry can enforce policy per session. See [`docs/extensions.md`](../../docs/extensions.md).
+### Fixed
+
+- Extension bare imports of workspace members now resolve inside installed git-dependency monorepo plugins (the walk recognizes `workspaces` roots; installed node_modules copies still shadow members)

 ### Changed

@@ -1351,6 +1351,10 @@ async function findNodePackageRootUncached(packageName: string, importerPath: st
 		if (await pathExists(path.join(candidate, "package.json"))) {
 			return candidate;
 		}
+		const workspaceMember = await findWorkspaceMemberPackageRoot(dir, packageName);
+		if (workspaceMember) {
+			return workspaceMember;
+		}
 		const parent = path.dirname(dir);
 		if (parent === dir) {
 			return null;
@@ -1359,6 +1363,49 @@ async function findNodePackageRootUncached(packageName: string, importerPath: st
 	}
 }

+/**
+ * Resolve `packageName` as a workspace member when `dir` is a workspace root.
+ *
+ * An installed git dependency of a monorepo plugin contains the full
+ * workspace tree but no node_modules links: `bun install` materializes a git
+ * dependency's regular npm dependencies into the host tree and skips its
+ * `workspace:*` / `file:` edges. Bare imports between workspace siblings
+ * therefore never resolve through the node_modules walk above. When a
+ * directory on that walk declares `workspaces` (array form or the yarn-style
+ * `{ packages: [...] }` object), scan the member manifests for the requested
+ * package name. node_modules candidates at the same level win, so an
+ * explicitly installed copy still shadows the workspace member.
+ */
+async function findWorkspaceMemberPackageRoot(dir: string, packageName: string): Promise<string | null> {
+	if (!(await pathExists(path.join(dir, "package.json")))) {
+		return null;
+	}
+	const manifest = await readPackageManifest(dir);
+	const rawWorkspaces = manifest?.workspaces;
+	const patterns = Array.isArray(rawWorkspaces)
+		? rawWorkspaces
+		: isRecord(rawWorkspaces) && Array.isArray(rawWorkspaces.packages)
+			? rawWorkspaces.packages
+			: null;
+	if (!patterns) {
+		return null;
+	}
+	for (const pattern of patterns) {
+		if (typeof pattern !== "string" || pattern.startsWith("!")) {
+			continue;
+		}
+		const glob = new Bun.Glob(path.join(pattern, "package.json"));
+		for await (const match of glob.scan({ cwd: dir, onlyFiles: true })) {
+			const memberRoot = path.dirname(path.join(dir, match));
+			const memberManifest = await readPackageManifest(memberRoot);
+			if (memberManifest?.name === packageName) {
+				return memberRoot;
+			}
+		}
+	}
+	return null;
+}
+
 async function readPackageManifest(packageRoot: string): Promise<Record<string, unknown> | null> {
 	const cached = packageManifestCache.get(packageRoot);
 	if (cached) return cached;
@@ -0,0 +1,98 @@
+import { afterEach, expect, mock, spyOn, test } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { __rewriteLegacyExtensionSourceForTests } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/legacy-pi-compat";
+import { removeWithRetries } from "@oh-my-pi/pi-utils";
+
+const tempRoots: string[] = [];
+
+afterEach(async () => {
+	mock.restore();
+	for (const root of tempRoots.splice(0)) {
+		await removeWithRetries(root);
+	}
+});
+
+async function writeJson(filePath: string, value: unknown): Promise<void> {
+	await Bun.write(filePath, `${JSON.stringify(value)}\n`);
+}
+
+/**
+ * Regression: an extension inside an installed git-dep monorepo (full
+ * workspace tree, no node_modules links for `workspace:*` siblings) must
+ * resolve bare imports of workspace members through the workspace root's
+ * `workspaces` globs, honoring the member's exports conditions.
+ */
+test("bare workspace-member imports resolve through the workspace root manifest", async () => {
+	const root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-workspace-resolve-"));
+	tempRoots.push(root);
+	const repoRoot = path.join(root, "plugins", "node_modules", "monorepo-plugin");
+	const importer = path.join(repoRoot, "packages", "extension", "extensions", "entry.ts");
+	const memberRoot = path.join(repoRoot, "packages", "contracts");
+	await fs.mkdir(path.dirname(importer), { recursive: true });
+	await fs.mkdir(path.join(memberRoot, "src"), { recursive: true });
+	await Bun.write(importer, "export {};\n");
+	await writeJson(path.join(repoRoot, "package.json"), {
+		name: "monorepo-plugin",
+		version: "1.0.0",
+		workspaces: ["packages/*"],
+	});
+	await writeJson(path.join(memberRoot, "package.json"), {
+		name: "@monorepo/contracts",
+		version: "1.0.0",
+		main: "dist/index.js",
+		exports: { ".": { bun: "./src/index.ts", default: "./dist/index.js" } },
+	});
+	await Bun.write(path.join(memberRoot, "src", "index.ts"), "export const marker = 1;\n");
+
+	spyOn(Bun, "resolveSync").mockImplementation(() => {
+		throw new Error("compiled fallback");
+	});
+
+	const rewritten = await __rewriteLegacyExtensionSourceForTests(
+		'import { marker } from "@monorepo/contracts";',
+		importer,
+	);
+
+	expect(rewritten).toContain(path.join("packages", "contracts", "src", "index.ts"));
+});
+
+test("installed node_modules copies shadow workspace members at the same level", async () => {
+	const root = await fs.mkdtemp(path.join(os.tmpdir(), "omp-workspace-shadow-"));
+	tempRoots.push(root);
+	const repoRoot = path.join(root, "monorepo-plugin");
+	const importer = path.join(repoRoot, "packages", "extension", "entry.ts");
+	const memberRoot = path.join(repoRoot, "packages", "dep");
+	const installedRoot = path.join(repoRoot, "node_modules", "@monorepo", "dep");
+	await fs.mkdir(path.dirname(importer), { recursive: true });
+	await fs.mkdir(memberRoot, { recursive: true });
+	await fs.mkdir(installedRoot, { recursive: true });
+	await Bun.write(importer, "export {};\n");
+	await writeJson(path.join(repoRoot, "package.json"), {
+		name: "monorepo-plugin",
+		version: "1.0.0",
+		workspaces: ["packages/*"],
+	});
+	await writeJson(path.join(memberRoot, "package.json"), {
+		name: "@monorepo/dep",
+		version: "1.0.0",
+		main: "member.js",
+	});
+	await Bun.write(path.join(memberRoot, "member.js"), "export default 1;\n");
+	await writeJson(path.join(installedRoot, "package.json"), {
+		name: "@monorepo/dep",
+		version: "2.0.0",
+		main: "installed.js",
+	});
+	await Bun.write(path.join(installedRoot, "installed.js"), "export default 2;\n");
+
+	spyOn(Bun, "resolveSync").mockImplementation(() => {
+		throw new Error("compiled fallback");
+	});
+
+	const rewritten = await __rewriteLegacyExtensionSourceForTests('import dep from "@monorepo/dep";', importer);
+
+	expect(rewritten).toContain("installed.js");
+	expect(rewritten).not.toContain("member.js");
+});