Merge pull request 'fix(pi-ext): stop importing @earendil-works/pi-ai at runtime' (#105) from fix/pi-ext-module-resolution into main
Some checks failed
Some checks failed
This commit was merged in pull request #105.
This commit is contained in:
@@ -101,9 +101,34 @@ src/pi-ext/
|
||||
mcp-http.ts # vendored Streamable-HTTP JSON-RPC client (no deps)
|
||||
```
|
||||
|
||||
The extension imports only from pi-bundled packages
|
||||
(`@earendil-works/pi-coding-agent`, `@earendil-works/pi-ai`, `typebox`), so it
|
||||
loads standalone.
|
||||
The extension imports only from pi-bundled packages, so it loads standalone.
|
||||
|
||||
### `typebox` is the only bare runtime import
|
||||
|
||||
pi does not let an extension resolve modules the ordinary way: it hands jiti a
|
||||
hard-coded alias table built from its *own* dependencies, and that table is not
|
||||
the same across pi distributions. The newer `@earendil-works/pi-coding-agent`
|
||||
aliases both the `@earendil-works/*` and the legacy `@mariozechner/*` names;
|
||||
older `@mariozechner/pi-coding-agent` installs (0.73.x and earlier) alias only
|
||||
the `@mariozechner/*` ones. Neither resolves the other's namespace.
|
||||
|
||||
So an import of anything outside the intersection kills the *whole* extension on
|
||||
someone else's pi — every tool, the `/mcpctl` command, the status line — with:
|
||||
|
||||
```
|
||||
Failed to load extension ".../mcpctl-pi.ts": Cannot find module '@earendil-works/pi-ai'
|
||||
```
|
||||
|
||||
which is exactly what `import { StringEnum } from "@earendil-works/pi-ai"` did.
|
||||
`typebox` is aliased by every published pi, so it is the only bare specifier
|
||||
allowed at runtime. Everything else must be a `node:` builtin, a relative path,
|
||||
an `import type` (erased before jiti resolves anything), or inlined — pi-ai's
|
||||
`StringEnum` is now a six-line local `stringEnum`. The
|
||||
`tests/config/pi-extension-embed.test.ts` guard fails the build on a reintroduced
|
||||
runtime import.
|
||||
|
||||
If a user does hit this error, check `type -a pi`: two installs on `$PATH` is the
|
||||
usual cause, and the extension has to load under whichever one wins.
|
||||
|
||||
## Typechecking
|
||||
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -29,6 +29,34 @@ describe('embedded pi extension', () => {
|
||||
expect(PI_EXTENSION_FILES['mcpctl-pi.ts']).toContain('./mcp-http.js');
|
||||
});
|
||||
|
||||
/**
|
||||
* pi resolves an extension's bare specifiers through a hard-coded alias table
|
||||
* in its own loader, and that table is not the same across pi distributions:
|
||||
* `@earendil-works/*` exists only in the newer packages, `@mariozechner/*`
|
||||
* installs alias only the old names, and neither resolves the other. An
|
||||
* import of a package outside the intersection makes the whole extension fail
|
||||
* to load with `Cannot find module` — every tool gone, on someone else's pi.
|
||||
*
|
||||
* `typebox` is aliased by every published pi, so it is the only safe bare
|
||||
* runtime import. Type-only imports are erased before jiti resolves anything,
|
||||
* so they may name whatever they like.
|
||||
*/
|
||||
it('imports nothing at runtime that some pi build cannot resolve', () => {
|
||||
// `import x from "s"` / `import {..} from "s"` (but not `import type`),
|
||||
// plus the side-effect form `import "s"`.
|
||||
const runtimeImport =
|
||||
/^\s*import\s+(?!type\s)[^;]*?from\s*["']([^"']+)["']|^\s*import\s*["']([^"']+)["']/gm;
|
||||
const allowed = /^(node:|\.\/|\.\.\/|typebox$|typebox\/)/;
|
||||
|
||||
for (const name of PI_EXTENSION_FILENAMES) {
|
||||
const src = PI_EXTENSION_FILES[name] ?? '';
|
||||
for (const match of src.matchAll(runtimeImport)) {
|
||||
const specifier = match[1] ?? match[2] ?? '';
|
||||
expect(specifier, `${name} runtime-imports ${specifier}`).toMatch(allowed);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('carries the fixes the pi API requires', () => {
|
||||
const main = PI_EXTENSION_FILES['mcpctl-pi.ts'] ?? '';
|
||||
// ctx.ui.select takes string[] and returns the chosen string.
|
||||
|
||||
@@ -20,9 +20,16 @@
|
||||
* or via settings: "extensions": ["/abs/path/to/mcpctl-pi.ts"]
|
||||
*
|
||||
* Only imports pi-bundled packages — no @mcpctl/*, no ~/.claude.
|
||||
*
|
||||
* RUNTIME IMPORTS ARE LOAD-BEARING: pi resolves an extension's bare specifiers
|
||||
* through a fixed alias table in its own loader, and that table differs between
|
||||
* pi distributions — `@earendil-works/*` exists only in the newer packages,
|
||||
* while `@mariozechner/*` installs alias only the old names. `typebox` is the
|
||||
* one specifier every published pi aliases, so it is the ONLY runtime import
|
||||
* allowed here. Anything else must be `import type` (erased before jiti runs)
|
||||
* or inlined — see `stringEnum` below.
|
||||
*/
|
||||
import { Type, type TSchema } from "typebox";
|
||||
import { StringEnum } from "@earendil-works/pi-ai";
|
||||
import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent";
|
||||
import {
|
||||
McpHttpSession,
|
||||
@@ -110,6 +117,23 @@ async function listProjects(mcplocalUrl: string, token?: string): Promise<string
|
||||
}
|
||||
|
||||
// ── JSON Schema → TypeBox ────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* `{ type: "string", enum: [...] }` rather than a union of literals: Google's
|
||||
* API (and other providers that reject anyOf/const) only accept the flat form.
|
||||
*
|
||||
* Inlined from pi-ai's `StringEnum` on purpose — importing it dragged in
|
||||
* `@earendil-works/pi-ai`, which older pi installs cannot resolve, and the
|
||||
* whole extension then failed to load. See the import note at the top.
|
||||
*/
|
||||
function stringEnum(values: string[], description?: string): TSchema {
|
||||
return Type.Unsafe<string>({
|
||||
type: "string",
|
||||
enum: values,
|
||||
...(description ? { description } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
function convertSchema(inputSchema: unknown): TSchema {
|
||||
if (!inputSchema || typeof inputSchema !== "object") {
|
||||
return Type.Object({});
|
||||
@@ -147,7 +171,7 @@ function convertProp(raw: unknown): TSchema {
|
||||
const enumVals = Array.isArray(s.enum) && s.enum.length > 0 ? s.enum : undefined;
|
||||
|
||||
if (enumVals && enumVals.every((v) => typeof v === "string")) {
|
||||
return StringEnum(enumVals as string[]);
|
||||
return stringEnum(enumVals as string[], desc);
|
||||
}
|
||||
if (enumVals && enumVals.every((v) => typeof v === "number")) {
|
||||
const literals = enumVals.map((v) => Type.Literal(v));
|
||||
|
||||
Reference in New Issue
Block a user