fix(extensibility): read lazy graph modules from disk at import time
The consume-once source map kept entries for graph modules the initial import never loaded (modules only reached via lazy dynamic imports). Their first import - possibly long after load, and after an on-disk edit - was served the boot-time snapshot instead of current file content, and the unconsumed sources stayed in the plugin closure for the process lifetime. Clear the map once the entry import settles: everything Bun loaded at startup was already consumed (keeping the read-once win), and anything left must be read at its actual import time, matching pre-dedup behavior for lazy modules. The new regression test passes on the pre-dedup baseline and fails on the unfixed dedup.
This commit is contained in:
@@ -1005,10 +1005,14 @@ async function collectExtensionModules(entryRealPath: string): Promise<Map<strin
|
||||
* so the filter is an exact-path alternation of the graph's realpaths — it
|
||||
* never matches the host, other extensions, `node_modules` deps, or unrelated
|
||||
* project source.
|
||||
*
|
||||
* Returns the collected path→source map on first install so the caller can
|
||||
* drop entries the initial import never consumed; `undefined` when the hook
|
||||
* was already installed.
|
||||
*/
|
||||
async function ensureExtensionGraphHook(entryRealPath: string): Promise<void> {
|
||||
async function ensureExtensionGraphHook(entryRealPath: string): Promise<Map<string, string> | undefined> {
|
||||
if (hookedExtensionEntries.has(entryRealPath)) {
|
||||
return;
|
||||
return undefined;
|
||||
}
|
||||
hookedExtensionEntries.add(entryRealPath);
|
||||
|
||||
@@ -1032,6 +1036,7 @@ async function ensureExtensionGraphHook(entryRealPath: string): Promise<void> {
|
||||
});
|
||||
},
|
||||
});
|
||||
return modules;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1050,9 +1055,16 @@ export async function loadLegacyPiModule(resolvedPath: string): Promise<unknown>
|
||||
// `bun link`/pnpm installs) so the rewrite filter matches the path Bun
|
||||
// actually hands the hook.
|
||||
const entryRealPath = await realpathOrSelf(path.resolve(resolvedPath));
|
||||
await ensureExtensionGraphHook(entryRealPath);
|
||||
// `?mtime` busts Bun's module cache so repeat loads pick up edited source.
|
||||
return import(`${toImportSpecifier(entryRealPath)}?mtime=${Date.now()}`);
|
||||
const pendingSources = await ensureExtensionGraphHook(entryRealPath);
|
||||
try {
|
||||
// `?mtime` busts Bun's module cache so repeat loads pick up edited source.
|
||||
return await import(`${toImportSpecifier(entryRealPath)}?mtime=${Date.now()}`);
|
||||
} finally {
|
||||
// Drop whatever the initial import didn't consume: graph modules only
|
||||
// reached by lazy dynamic imports must be read from disk at their actual
|
||||
// import time, not served from this load-time snapshot.
|
||||
pendingSources?.clear();
|
||||
}
|
||||
}
|
||||
|
||||
function getLoader(path: string): "js" | "jsx" | "ts" | "tsx" {
|
||||
|
||||
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, type Mock, spyOn } from "b
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
import { loadExtensions } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/loader";
|
||||
import { loadLegacyPiModule } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/legacy-pi-compat";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
import type { BunFile } from "bun";
|
||||
|
||||
@@ -98,4 +99,32 @@ export default function(pi) {
|
||||
checkReadCount(path.join(extDir, `mod-${i}.ts`));
|
||||
}
|
||||
});
|
||||
|
||||
it("should read graph modules skipped by the initial import from disk at import time", async () => {
|
||||
const cwd = tempDir.absolute();
|
||||
const extDir = path.join(cwd, "ext");
|
||||
fs.mkdirSync(extDir, { recursive: true });
|
||||
|
||||
const lazyPath = path.join(extDir, "lazy.ts");
|
||||
fs.writeFileSync(lazyPath, `export const value = "before";\n`, "utf-8");
|
||||
|
||||
// The fixture's dynamic import is the loading boundary under test: the
|
||||
// graph scan collects `./lazy.ts` at load time, but nothing imports it
|
||||
// until `readLazy()` runs.
|
||||
const entryPath = path.join(extDir, "index.ts");
|
||||
const entryContent = `export async function readLazy(): Promise<string> {
|
||||
const mod = await import("./lazy.ts");
|
||||
return mod.value;
|
||||
}
|
||||
`;
|
||||
fs.writeFileSync(entryPath, entryContent, "utf-8");
|
||||
|
||||
const ns = (await loadLegacyPiModule(entryPath)) as { readLazy(): Promise<string> };
|
||||
|
||||
// Edit the module after load but before its first import: the loader
|
||||
// must serve the on-disk content, not a stale load-time snapshot.
|
||||
fs.writeFileSync(lazyPath, `export const value = "after";\n`, "utf-8");
|
||||
|
||||
expect(await ns.readLazy()).toBe("after");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user