diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f9b8bb0ef..939322d37 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed configured or `-e` extensions in compiled binaries failing to resolve bundled `@oh-my-pi/*` value imports through the `omp-legacy-pi-bundled:` registry, and surfaced extension load failures during interactive and `-p` session startup. ([#4954](https://github.com/can1357/oh-my-pi/issues/4954)) + ## [16.3.12] - 2026-07-08 ### Added diff --git a/packages/coding-agent/src/extensibility/extensions/load-errors.ts b/packages/coding-agent/src/extensibility/extensions/load-errors.ts new file mode 100644 index 000000000..5ba213d58 --- /dev/null +++ b/packages/coding-agent/src/extensibility/extensions/load-errors.ts @@ -0,0 +1,13 @@ +import { replaceTabs, shortenPath, TRUNCATE_LENGTHS, truncateToWidth } from "../../tools/render-utils"; +import type { LoadExtensionsResult } from "./types"; + +/** Formats extension load failures for user-visible startup diagnostics. */ +export function formatExtensionLoadNotifications(errors: LoadExtensionsResult["errors"]): string[] { + const messages: string[] = []; + for (const { path, error } of errors) { + const displayPath = truncateToWidth(replaceTabs(shortenPath(path)), TRUNCATE_LENGTHS.CONTENT); + const displayError = truncateToWidth(replaceTabs(error.replace(/\s+/g, " ").trim()), TRUNCATE_LENGTHS.LONG); + messages.push(`Failed to load extension ${displayPath}: ${displayError}`); + } + return messages; +} diff --git a/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts b/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts index 20f55ad19..c3b8c83d0 100644 --- a/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts +++ b/packages/coding-agent/src/extensibility/plugins/legacy-pi-compat.ts @@ -39,11 +39,22 @@ const IS_COMPILED_BINARY = isCompiledBinary(); // exception to the static-import rule. const BUNDLED_VIRTUAL_SCHEME = "omp-legacy-pi-bundled:"; const BUNDLED_VIRTUAL_NAMESPACE = "omp-legacy-pi-bundled"; +const BUNDLED_VIRTUAL_SPECIFIER_FILTER = /^omp-legacy-pi-bundled:.+$/; const BUNDLED_REGISTRY_GLOBAL = "__ompLegacyPiBundledRegistry"; const TYPEBOX_BUNDLED_REGISTRY_KEY = "typebox"; type BundledRegistry = Readonly>>>; +interface LegacyPiResolveResult { + path: string; + namespace?: string; +} + +interface BundledVirtualResolveResult { + path: string; + namespace: typeof BUNDLED_VIRTUAL_NAMESPACE; +} + let bundledRegistryPromise: Promise | null = null; /** @@ -83,6 +94,26 @@ function isBundledVirtualSpecifier(value: string): boolean { return value.startsWith(BUNDLED_VIRTUAL_SCHEME); } +function toLegacyPiResolveResult(resolvedPath: string): LegacyPiResolveResult { + if (isBundledVirtualSpecifier(resolvedPath)) { + const registryKey = resolvedPath.slice(BUNDLED_VIRTUAL_SCHEME.length); + return { path: registryKey, namespace: BUNDLED_VIRTUAL_NAMESPACE }; + } + return { path: resolvedPath }; +} + +/** Maps a bundled virtual specifier to Bun's plugin namespace shape. */ +export function resolveBundledVirtualSpecifier(specifier: string): BundledVirtualResolveResult { + if (!isBundledVirtualSpecifier(specifier)) { + throw new Error(`omp:legacy-pi-shim: not a bundled virtual specifier: ${specifier}`); + } + const registryKey = specifier.slice(BUNDLED_VIRTUAL_SCHEME.length); + if (!registryKey) { + throw new Error("omp:legacy-pi-shim: bundled virtual specifier has no registry key"); + } + return { path: registryKey, namespace: BUNDLED_VIRTUAL_NAMESPACE }; +} + /** * Build the synthetic ES module source for a `omp-legacy-pi-bundled:` * import against an explicit registry. Pure: takes the live module namespace @@ -1216,7 +1247,7 @@ function getLoader(path: string): "js" | "jsx" | "ts" | "tsx" { return "js"; } -function resolveLegacyPiSpecifier(args: { path: string; importer: string }): { path: string } | undefined { +function resolveLegacyPiSpecifier(args: { path: string; importer: string }): LegacyPiResolveResult | undefined { const remappedSpecifier = remapLegacyPiSpecifier(args.path); if (!remappedSpecifier) { return undefined; @@ -1225,7 +1256,7 @@ function resolveLegacyPiSpecifier(args: { path: string; importer: string }): { p // Primary: resolve the canonical @oh-my-pi/* specifier from the host binary // location. Works in dev mode and in source-link installs. try { - return { path: resolveCanonicalPiSpecifier(remappedSpecifier) }; + return toLegacyPiResolveResult(resolveCanonicalPiSpecifier(remappedSpecifier)); } catch { // Fallback for compiled binary mode: the bundled packages live inside // /$bunfs/root and aren't reachable by filesystem resolution. Prefer the @@ -1235,10 +1266,10 @@ function resolveLegacyPiSpecifier(args: { path: string; importer: string }): { p // @earendil-works peer deps. const importerDir = path.dirname(args.importer); try { - return { path: Bun.resolveSync(remappedSpecifier, importerDir) }; + return toLegacyPiResolveResult(Bun.resolveSync(remappedSpecifier, importerDir)); } catch { try { - return { path: Bun.resolveSync(args.path, importerDir) }; + return toLegacyPiResolveResult(Bun.resolveSync(args.path, importerDir)); } catch { return undefined; } @@ -1246,8 +1277,8 @@ function resolveLegacyPiSpecifier(args: { path: string; importer: string }): { p } } -function resolveTypeBoxSpecifier(): { path: string } | undefined { - return TYPEBOX_SHIM_PATH ? { path: TYPEBOX_SHIM_PATH } : undefined; +function resolveTypeBoxSpecifier(): LegacyPiResolveResult | undefined { + return TYPEBOX_SHIM_PATH ? toLegacyPiResolveResult(TYPEBOX_SHIM_PATH) : undefined; } export function installLegacyPiSpecifierShim(): void { @@ -1261,6 +1292,9 @@ export function installLegacyPiSpecifierShim(): void { setup(build) { build.onResolve({ filter: LEGACY_PI_SPECIFIER_FILTER, namespace: "file" }, resolveLegacyPiSpecifier); build.onResolve({ filter: TYPEBOX_SPECIFIER_FILTER, namespace: "file" }, resolveTypeBoxSpecifier); + build.onResolve({ filter: BUNDLED_VIRTUAL_SPECIFIER_FILTER, namespace: "file" }, args => + resolveBundledVirtualSpecifier(args.path), + ); // Compiled-binary mode: serve `omp-legacy-pi-bundled:` imports // from the JS-heap registry. The rewrite path emits these specifiers // in place of unreachable `file:///$bunfs/...` URLs (issue #3423). diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index e832c23f2..e8ca84fef 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -47,6 +47,7 @@ import { resolveActiveProjectRegistryPath, } from "./discovery/helpers"; import { injectOmpExtensionCliRoots } from "./discovery/omp-extension-roots"; +import { formatExtensionLoadNotifications } from "./extensibility/extensions/load-errors"; import { ExtensionRunner } from "./extensibility/extensions/runner"; import type { ExtensionUIContext } from "./extensibility/extensions/types"; import { scheduleMarketplaceAutoUpdate } from "./extensibility/plugins/marketplace-auto-update"; @@ -1317,6 +1318,13 @@ export async function runRootCommand( }, }; const initialArgs = applyExtensionFlags(extensionFlagSink, rawArgs) ?? parsedArgs; + for (const message of formatExtensionLoadNotifications(extensionsResult.errors)) { + if (isInteractive) { + notifs.push({ kind: "warn", message }); + } else { + process.stderr.write(`${chalk.yellow(`${message}\n`)}`); + } + } // Fail fast on stale/typo flags (e.g. `omp --list-models`) now that we // know the real extension flag set. Without this check the unrecognized // token gets silently consumed and any following positional leaks as the diff --git a/packages/coding-agent/src/prompts/system/workflow-notice.md b/packages/coding-agent/src/prompts/system/workflow-notice.md index 74eddee21..3e62ca5b0 100644 --- a/packages/coding-agent/src/prompts/system/workflow-notice.md +++ b/packages/coding-agent/src/prompts/system/workflow-notice.md @@ -51,20 +51,20 @@ Decompose first, then {{#if taskBatch}}batch the independent leaves{{else}}issue {{#if taskBatch}} task( - context: "# Goal\nReview the auth diff...\n# Constraints\nRead-only...\n# Contract\nReturn findings as severity/file/line/fix...", + context: "# Goal\nReview the auth diff…\n# Constraints\nRead-only…\n# Contract\nReturn findings as severity/file/line/fix…", tasks: [ - { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection...\n# Acceptance\nReturn confirmed findings only..." }, - { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance...\n# Acceptance\nReturn mismatches and exact prompt lines..." }, + { id: "AuthOwner", role: "Auth Storage Reviewer", assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nTrace credential selection…\n# Acceptance\nReturn confirmed findings only…" }, + { id: "PromptOwner", role: "Prompt Contract Reviewer", assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance…\n# Acceptance\nReturn mismatches and exact prompt lines…" }, ] ) {{else}} task( role: "Auth Storage Reviewer", - assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/ai/src/auth-storage.ts\n# Change\nReview the auth diff. Shared contract: read-only; return findings as severity/file/line/fix.\n# Acceptance\nReturn confirmed findings only…" ) task( role: "Prompt Contract Reviewer", - assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only..." + assignment: "# Target\npackages/coding-agent/src/prompts/**\n# Change\nCheck active-tool guidance. Shared contract: read-only; return mismatches and exact prompt lines.\n# Acceptance\nReturn confirmed findings only…" ) {{/if}} diff --git a/packages/coding-agent/test/extensibility/extension-load-notifications.test.ts b/packages/coding-agent/test/extensibility/extension-load-notifications.test.ts new file mode 100644 index 000000000..7bbf1b377 --- /dev/null +++ b/packages/coding-agent/test/extensibility/extension-load-notifications.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "bun:test"; +import * as os from "node:os"; +import * as path from "node:path"; +import { formatExtensionLoadNotifications } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/load-errors"; + +describe("extension load startup notifications", () => { + it("formats load failures as sanitized single-line warnings for TUI and print startup paths", () => { + const homeDir = os.homedir(); + const extensionPath = path.join(homeDir, "omp-notification-fixture", "plugin\tname", "extension.ts"); + const tailMarker = "TAIL_MARKER_AFTER_TRUNCATION"; + const [message] = formatExtensionLoadNotifications([ + { + path: extensionPath, + error: `SyntaxError: Missing named export\n\tat extension loader\n${"x".repeat(200)}${tailMarker}`, + }, + ]); + + expect(message).toBeDefined(); + expect(message?.startsWith("Failed to load extension ~/omp-notification-fixture/plugin")).toBe(true); + expect(message).toContain("name/extension.ts: SyntaxError: Missing named export at extension loader"); + expect(message).not.toContain(homeDir); + expect(message).not.toContain("\n"); + expect(message).not.toContain("\t"); + expect(message).not.toContain(tailMarker); + }); +}); diff --git a/packages/coding-agent/test/extensibility/legacy-pi-bundled-virtual.test.ts b/packages/coding-agent/test/extensibility/legacy-pi-bundled-virtual.test.ts index 0fece9ee8..f122bae71 100644 --- a/packages/coding-agent/test/extensibility/legacy-pi-bundled-virtual.test.ts +++ b/packages/coding-agent/test/extensibility/legacy-pi-bundled-virtual.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; import { __getLegacyPiBundledRegistryGlobal, __synthesizeLegacyPiBundledSourceWithRegistry, + resolveBundledVirtualSpecifier, } from "@oh-my-pi/pi-coding-agent/extensibility/plugins/legacy-pi-compat"; +import { TempDir } from "@oh-my-pi/pi-utils"; +import type { BunPlugin } from "bun"; // Regression for issue #3423: Bun 1.3.14 made `--compile` extras unreachable // via every filesystem-style API, so `legacy-pi-compat.ts` now routes @@ -93,4 +97,69 @@ describe("legacy-pi bundled virtual module synthesizer (issue #3423)", () => { delete (globalThis as Record)[globalKey]; } }); + + it("routes Bun plugin resolution through the bundled namespace so onLoad can serve extension imports", async () => { + using tempDir = TempDir.createSync("@omp-legacy-pi-bundled-virtual-"); + const entryPath = tempDir.join("extension-entry.ts"); + const bundlePath = tempDir.join("extension-entry.bundle.mjs"); + + await Bun.write( + entryPath, + [ + 'import { legacyAnswer } from "omp-legacy-pi-bundled:@oh-my-pi/pi-utils";', + "process.stdout.write(legacyAnswer);", + "", + ].join("\n"), + ); + + const resolveResult = resolveBundledVirtualSpecifier("omp-legacy-pi-bundled:@oh-my-pi/pi-utils"); + expect(resolveResult).toEqual({ + namespace: "omp-legacy-pi-bundled", + path: "@oh-my-pi/pi-utils", + }); + + const onLoadPaths: string[] = []; + const plugin: BunPlugin = { + name: "omp-legacy-pi-bundled-virtual-regression", + setup(build) { + build.onResolve({ filter: /^omp-legacy-pi-bundled:.+$/, namespace: "file" }, args => + resolveBundledVirtualSpecifier(args.path), + ); + build.onLoad({ filter: /.*/, namespace: "omp-legacy-pi-bundled" }, args => { + onLoadPaths.push(args.path); + return { + contents: `export const legacyAnswer = ${JSON.stringify(`served:${args.path}`)};`, + loader: "js", + }; + }); + }, + }; + + const buildResult = await Bun.build({ + entrypoints: [entryPath], + external: ["bun"], + format: "esm", + plugins: [plugin], + target: "bun", + }); + const buildLogs = buildResult.logs.map(log => log.message).join("\n"); + expect(buildResult.success, buildLogs).toBe(true); + await Bun.write(bundlePath, await buildResult.outputs[0]!.text()); + expect(onLoadPaths).toEqual(["@oh-my-pi/pi-utils"]); + + const proc = Bun.spawn([process.execPath, `./${path.basename(bundlePath)}`], { + cwd: path.dirname(bundlePath), + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + + expect(exitCode, stderr).toBe(0); + expect(stderr).toBe(""); + expect(stdout).toBe("served:@oh-my-pi/pi-utils"); + }); });