fix(coding-agent): prevented puppeteer startup crashes
- Avoided value-imports of `puppeteer-core` in main startup files to prevent eager loading of the browser-data dependency graph. - Used `import type` and literal string checks to defer `puppeteer-core` initialization until browser tools are actually invoked. - Added a test to verify that `puppeteer-core` and `@puppeteer/browsers` remain off the eager startup import graph. - Externalized additional heavy dependencies to reduce bundle size and improved minification settings in the build script.
This commit is contained in:
can1357
@@ -8,6 +8,7 @@
|
||||
- Fixed plan-mode `Refine plan` so the internal approval abort is hidden and the editor is ready for a follow-up prompt instead of showing `Operation aborted` ([#2971](https://github.com/can1357/oh-my-pi/issues/2971)).
|
||||
- Fixed TUI prompts beginning with shell-style variables such as `$HOME` being misrouted to Python eval; Python shortcuts now require `$ <code>` or `$$ <code>`. ([#2944](https://github.com/can1357/oh-my-pi/issues/2944))
|
||||
- Fixed LM Studio runtime discovery to use native `/api/v0/models` metadata so `inspect_image` can select VLM models. ([#2945](https://github.com/can1357/oh-my-pi/issues/2945))
|
||||
- Fixed `omp` crashing at startup with `Cannot find module './browser-data/browser-data.js'` (e.g. on Termux/proot Linux arm64). `tools/browser/attach.ts` value-imported the `TargetType` enum from `puppeteer-core`, which dragged the puppeteer-core barrel (and its `@puppeteer/browsers` Chromium downloader) back onto the eager startup import graph — defeating the lazy-load deferral and turning any bundling quirk in that subtree into a fatal boot crash instead of a recoverable browser-tool error. The import is now `import type` and the target check compares against the `"page"` literal, so puppeteer only loads on first browser use.
|
||||
|
||||
## [16.0.7] - 2026-06-18
|
||||
|
||||
|
||||
@@ -9,6 +9,31 @@ const outDir = path.join(packageDir, "dist");
|
||||
const cliPath = path.join(outDir, "cli.js");
|
||||
const shebang = "#!/usr/bin/env bun\n";
|
||||
|
||||
// Native / optional / platform-specific deps that are never bundled — installed on
|
||||
// demand (transformers/fastembed/onnxruntime) or shipped as their own artifact
|
||||
// (native addon, mupdf).
|
||||
const ALWAYS_EXTERNAL = ["mupdf", "@oh-my-pi/pi-natives", "@huggingface/transformers", "fastembed", "onnxruntime-node"];
|
||||
|
||||
// Heavy, lazily-used third-party leaf deps. Each is a declared `dependency`, so the
|
||||
// published package resolves it from node_modules at runtime; bundling only embeds a
|
||||
// redundant copy that bloats dist/cli.js. NEVER add a patched dependency here — the
|
||||
// bundle is where a root `patchedDependencies` patch is baked in, so an externalized
|
||||
// import would load the unpatched npm package in users' installs (currently
|
||||
// beautiful-mermaid and @ark/schema are patched, so they — and arktype, which pulls
|
||||
// @ark/schema — stay bundled).
|
||||
const RUNTIME_EXTERNAL = [
|
||||
"puppeteer-core",
|
||||
"@puppeteer/browsers",
|
||||
"@babel/parser",
|
||||
"@xterm/headless",
|
||||
"turndown",
|
||||
"turndown-plugin-gfm",
|
||||
"@mozilla/readability",
|
||||
"linkedom",
|
||||
"markit-ai",
|
||||
"@agentclientprotocol/sdk",
|
||||
];
|
||||
|
||||
async function runCommand(command: string[]): Promise<void> {
|
||||
const proc = Bun.spawn(command, {
|
||||
cwd: packageDir,
|
||||
@@ -63,19 +88,11 @@ async function main(): Promise<void> {
|
||||
"--target=bun",
|
||||
"--outdir",
|
||||
"dist",
|
||||
"--minify-whitespace",
|
||||
"--minify-syntax",
|
||||
// Full minify (whitespace + syntax + identifiers); --keep-names retains
|
||||
// fn/class .name where code depends on it.
|
||||
"--minify",
|
||||
"--keep-names",
|
||||
"--external",
|
||||
"mupdf",
|
||||
"--external",
|
||||
"@oh-my-pi/pi-natives",
|
||||
"--external",
|
||||
"@huggingface/transformers",
|
||||
"--external",
|
||||
"fastembed",
|
||||
"--external",
|
||||
"onnxruntime-node",
|
||||
...[...ALWAYS_EXTERNAL, ...RUNTIME_EXTERNAL].flatMap(dep => ["--external", dep]),
|
||||
"--define",
|
||||
'process.env.PI_BUNDLED="true"',
|
||||
"./src/cli.ts",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import * as net from "node:net";
|
||||
import { Process, ProcessStatus } from "@oh-my-pi/pi-natives";
|
||||
import { type Browser, type Page, TargetType } from "puppeteer-core";
|
||||
import type { Browser, Page } from "puppeteer-core";
|
||||
import { ToolError, throwIfAborted } from "../tool-errors";
|
||||
|
||||
const ATTACH_TARGET_SKIP_PATTERN =
|
||||
@@ -126,7 +126,7 @@ export async function findReusableCdp(
|
||||
export async function pickElectronTarget(browser: Browser, matcher?: string): Promise<Page> {
|
||||
const discoveredPages = await Promise.all(
|
||||
browser.targets().map(async target => {
|
||||
if (target.type() !== TargetType.PAGE) return null;
|
||||
if (String(target.type()) !== "page") return null;
|
||||
return await target.page().catch(() => null);
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -31,4 +31,24 @@ describe("startup import graph", () => {
|
||||
expect(mainSource).not.toContain('from "./extensibility/plugins/marketplace"');
|
||||
expect(starterSource).toContain('await import("./marketplace")');
|
||||
});
|
||||
|
||||
it("keeps puppeteer-core/@puppeteer/browsers off the eager startup graph", async () => {
|
||||
// The builtin tool registry (tools/index.ts) statically imports BrowserTool, which
|
||||
// reaches attach.ts -> registry.ts -> launch.ts -> tab-supervisor.ts. A *value*
|
||||
// import from puppeteer-core (e.g. the TargetType enum) executes puppeteer-core's
|
||||
// barrel at boot, which re-exports the node launchers + BrowserConnector and pulls in
|
||||
// @puppeteer/browsers (the Chromium downloader). A packaging quirk in that subtree then
|
||||
// becomes a hard startup crash (e.g. `Cannot find module './browser-data/browser-data.js'`)
|
||||
// instead of a recoverable ToolError on first browser use. These modules must therefore
|
||||
// import puppeteer packages as `import type` only. tab-worker.ts is excluded: it runs
|
||||
// solely inside the spawned browser worker, off the main startup graph.
|
||||
const browserDir = path.join(sourceRoot, "tools", "browser");
|
||||
const eagerFiles = ["attach.ts", "registry.ts", "launch.ts", "tab-supervisor.ts"];
|
||||
const valueImportRe = /^import\s+(?!type\b)[^;]*?from\s+["'](?:puppeteer-core|@puppeteer\/browsers)["']/gm;
|
||||
for (const file of eagerFiles) {
|
||||
const src = await Bun.file(path.join(browserDir, file)).text();
|
||||
const offenders = src.match(valueImportRe) ?? [];
|
||||
expect(offenders, `${file} value-imports puppeteer; use \`import type\``).toEqual([]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -110,7 +110,7 @@ async function buildBinary(target: BinaryTarget): Promise<void> {
|
||||
console.log(`Building ${target.outfile}...`);
|
||||
await embedNative(target);
|
||||
if (isDryRun) {
|
||||
console.log(`DRY RUN bun build --compile --no-compile-autoload-bunfig --no-compile-autoload-dotenv --no-compile-autoload-tsconfig --no-compile-autoload-package-json --keep-names --define process.env.PI_COMPILED="true" --root . --external mupdf --target=${target.target} ${entrypoint} --outfile ${target.outfile}`);
|
||||
console.log(`DRY RUN bun build --compile --no-compile-autoload-bunfig --no-compile-autoload-dotenv --no-compile-autoload-tsconfig --no-compile-autoload-package-json --minify-identifiers --keep-names --define process.env.PI_COMPILED="true" --root . --external mupdf --target=${target.target} ${entrypoint} --outfile ${target.outfile}`);
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -126,6 +126,7 @@ async function buildBinary(target: BinaryTarget): Promise<void> {
|
||||
"--no-compile-autoload-dotenv",
|
||||
"--no-compile-autoload-tsconfig",
|
||||
"--no-compile-autoload-package-json",
|
||||
"--minify-identifiers",
|
||||
"--keep-names",
|
||||
"--define",
|
||||
'process.env.PI_COMPILED="true"',
|
||||
|
||||
Reference in New Issue
Block a user