smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(extensions): preserved unsafe schemas during install

Browse Source 
- Lifted legacy Type.Unsafe documents into callable omptype schemas so Optional and Object composition preserve validation and required fields.
- Ran installed extension factories against the normal throwaway loader surface before accepting a plugin install.
- Added regression coverage and documented the stronger rollback gate.

Fixes #8143
This commit is contained in:
roboomp
2026-08-10 07:37:28 +00:00
parent 45e12e5bb7
commit e414585155
6 changed files with 89 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plugin-manager-installer-plumbing.md
+1 -1
View File
@@ -109,7 +109,7 @@ Malformed `package.json` JSON is a hard failure at read time; malformed manifest
- `[a,b]`: validates each feature exists in manifest features map
- `[]`: empty feature list
- bare spec: `null` (use defaults policy later in loader)
7. Validate declared extension entries (`#validateInstalledExtensions`): each manifest `extensions` entry must resolve on disk and import to a factory function. On failure, roll back the install — restore the previous `plugins/package.json`, remove the freshly installed package, and restore any prior version from a backup taken before `bun install` — then abort.
7. Validate declared extension entries (`#validateInstalledExtensions`): each manifest `extensions` entry must resolve on disk, import to a factory function, and initialize successfully against a throwaway registration surface. On failure, roll back the install — restore the previous `plugins/package.json`, remove the freshly installed package, and restore any prior version from a backup taken before `bun install` — then abort.
8. Upsert lockfile runtime state: `{ version, enabledFeatures, enabled: true }`.
### Update semantics
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed the legacy TypeBox facade rejecting `Type.Optional(Type.Unsafe(...))`, losing optional object properties when raw schemas were present, and plugin installs accepting extension factories that fail during initialization ([#8143](https://github.com/can1357/oh-my-pi/issues/8143)).
## [17.2.12] - 2026-08-08
### Fixed
packages/coding-agent/src/extensibility/legacy-typebox.ts
+19 -47
View File
@@ -1,9 +1,5 @@
import {
type ObjectOpts,
Type as OmpType,
type TypeBuilder as OmpTypeBuilder,
type TUnsafe,
} from "@oh-my-pi/omptype/typebox";
import { fromJsonSchema } from "@oh-my-pi/omptype";
import { Type as OmpType, type TypeBuilder as OmpTypeBuilder, type TUnsafe } from "@oh-my-pi/omptype/typebox";
import { upgradeJsonSchemaTo202012, validateJsonSchemaValue } from "@oh-my-pi/pi-ai/utils/schema";
export * from "@oh-my-pi/omptype/typebox";
@@ -25,11 +21,14 @@ interface SafeParseFailure {
error: ValidationFailure;
}
type LegacyUnsafeSchema<T> = Record<string, unknown> &
TUnsafe<T> & {
__validator(data: unknown): T | ValidationFailure;
safeParse(input: unknown): SafeParseSuccess<T> | SafeParseFailure;
};
type LegacyUnsafeSchema<T> = TUnsafe<T> & {
__validator(data: unknown): T | ValidationFailure;
safeParse(input: unknown): SafeParseSuccess<T> | SafeParseFailure;
};
function isValidationFailure<T>(result: T | ValidationFailure): result is ValidationFailure {
return typeof result === "object" && result !== null && VALIDATION_FAILURE in result;
}
function defineHidden(target: object, key: PropertyKey, value: unknown): void {
Object.defineProperty(target, key, {
@@ -40,8 +39,8 @@ function defineHidden(target: object, key: PropertyKey, value: unknown): void {
}
function unsafe<T = unknown>(jsonSchema: Record<string, unknown> = {}): LegacyUnsafeSchema<T> {
const schema = { ...jsonSchema } as LegacyUnsafeSchema<T>;
const upgradedSchema = upgradeJsonSchemaTo202012(jsonSchema);
const document = { ...jsonSchema };
const upgradedSchema = upgradeJsonSchemaTo202012(document);
const validate = (data: unknown): T | ValidationFailure => {
const result = validateJsonSchemaValue(upgradedSchema, data);
if (result.success) return data as T;
@@ -54,47 +53,20 @@ function unsafe<T = unknown>(jsonSchema: Record<string, unknown> = {}): LegacyUn
defineHidden(failure, VALIDATION_FAILURE, true);
return failure;
};
const schema = fromJsonSchema(upgradedSchema).narrow((data, ctx) => {
const result = validate(data);
return isValidationFailure(result) ? ctx.mustBe(result.message) : true;
}) as unknown as LegacyUnsafeSchema<T>;
defineHidden(schema, "toJsonSchema", () => document);
defineHidden(schema, "__validator", validate);
defineHidden(schema, "safeParse", (input: unknown): SafeParseSuccess<T> | SafeParseFailure => {
const result = validate(input);
return typeof result === "object" && result !== null && VALIDATION_FAILURE in result
? { success: false, error: result }
: { success: true, data: result as T };
return isValidationFailure(result) ? { success: false, error: result } : { success: true, data: result };
});
return schema;
}
const object = ((properties: Record<string, unknown>, opts?: ObjectOpts) => {
let hasRawProperty = false;
for (const key in properties) {
if (typeof properties[key] !== "function") {
hasRawProperty = true;
break;
}
}
if (!hasRawProperty) return OmpType.Object(properties as Parameters<typeof OmpType.Object>[0], opts);
const propertySchemas: Record<string, unknown> = {};
const required: string[] = [];
for (const key in properties) {
const property = properties[key];
propertySchemas[key] =
typeof property === "function" && "toJsonSchema" in property
? (property as { toJsonSchema(): Record<string, unknown> }).toJsonSchema()
: property;
required.push(key);
}
const document: Record<string, unknown> = { type: "object", properties: propertySchemas, required };
if (opts?.additionalProperties !== undefined) {
document.additionalProperties =
typeof opts.additionalProperties === "function"
? opts.additionalProperties.toJsonSchema()
: opts.additionalProperties;
}
return unsafe(document);
}) as typeof OmpType.Object;
export const Type: OmpTypeBuilder = { ...OmpType, Object: object, Unsafe: unsafe } as unknown as OmpTypeBuilder;
export const Type = { ...OmpType, Unsafe: unsafe } as unknown as OmpTypeBuilder;
export type TypeBuilder = OmpTypeBuilder;
const legacyTypeBox: { Type: OmpTypeBuilder } = { Type };
packages/coding-agent/src/extensibility/plugins/manager.ts
+4 -21
View File
@@ -11,10 +11,9 @@ import {
isEnoent,
logger,
} from "@oh-my-pi/pi-utils";
import { withHostGuard } from "../utils";
import { loadExtensions } from "../extensions/loader";
import { refreshBunGitCache } from "./bun-git-cache";
import { type GitSource, parseGitUrl } from "./git-url";
import { installLegacyPiSpecifierShim, loadLegacyPiModule } from "./legacy-pi-compat";
import { resolvePluginManifestEntries } from "./loader";
import { getInstalledPluginsRegistryPath, readInstalledPluginsRegistry } from "./marketplace/registry";
import { parsePluginId } from "./marketplace/types";
@@ -95,14 +94,6 @@ function findGitPackageName(source: GitSource, deps: Record<string, string>): st
return undefined;
}
function hasDefaultExport(value: unknown): value is { default?: unknown } {
return typeof value === "object" && value !== null && "default" in value;
}
function hasExtensionFactoryExport(module: unknown): boolean {
return typeof module === "function" || (hasDefaultExport(module) && typeof module.default === "function");
}
interface PluginPackageSnapshot {
readonly actualName: string;
readonly packagePath: string;
@@ -372,17 +363,9 @@ export class PluginManager {
}
if (loadable.length > 0) {
installLegacyPiSpecifierShim();
for (const extensionPath of loadable) {
try {
const module = await withHostGuard(() => loadLegacyPiModule(extensionPath));
if (!hasExtensionFactoryExport(module)) {
errors.push(`${extensionPath}: extension does not export a valid factory function`);
}
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
errors.push(`${extensionPath}: ${message}`);
}
const result = await loadExtensions(loadable, this.#cwd);
for (const failure of result.errors) {
errors.push(`${failure.path}: ${failure.error}`);
}
}
packages/coding-agent/test/extensibility/typebox-shim.test.ts
+19
View File
@@ -59,6 +59,25 @@ describe("pi.typebox compatibility shim", () => {
).toThrow('Validation failed for tool "unsafe-schema"');
});
it("composes optional Type.Unsafe schemas without making sibling properties required", () => {
const schema = Type.Object({
kind: Type.Unsafe({ type: "string", enum: ["question"] }),
mode: Type.Optional(Type.Unsafe({ type: "string", enum: ["overlay", "inline"] })),
label: Type.Optional(Type.String()),
});
const document = schema.toJsonSchema();
expect(document.required).toEqual(["kind"]);
expect(document).toMatchObject({
properties: {
mode: { type: "string", enum: ["overlay", "inline"] },
label: { type: "string" },
},
});
expect(schema.safeParse({ kind: "question", mode: "overlay" }).success).toBe(true);
expect(schema.safeParse({ kind: "question", mode: "other" }).success).toBe(false);
});
it("validates Type.Unsafe draft-07 documents like the wire path", () => {
const schema = Type.Unsafe({
type: "object",
packages/coding-agent/test/plugin-install-validation.test.ts
+42
View File
@@ -126,6 +126,48 @@ describe("PluginManager.install load validation", () => {
expect(result.path).toBe(path.join(pluginsNodeModules, "pi-figma-remote-auth"));
});
test("rejects and rolls back an install when the extension factory throws", async () => {
vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
expect(cmd).toEqual(["bun", "install", "factory-failure-plugin"]);
const prepare = (async () => {
await Bun.write(
pluginsPkgJson,
JSON.stringify(
{
name: "omp-plugins",
private: true,
dependencies: { "factory-failure-plugin": "1.0.0" },
},
null,
2,
),
);
await writePluginPackage(pluginsNodeModules, "factory-failure-plugin", {
version: "1.0.0",
source: 'export default function() { throw new Error("factory-time failure"); }\n',
});
})();
return {
pid: 1,
stdout: emptyStream(),
stderr: emptyStream(),
exited: prepare.then(() => 0),
} as Subprocess;
}) as typeof Bun.spawn);
await expect(new PluginManager(tmpRoot).install("factory-failure-plugin")).rejects.toThrow(
/factory-time failure/,
);
const pluginsPackage = await Bun.file(pluginsPkgJson).json();
expect(pluginsPackage.dependencies ?? {}).toEqual({});
expect(await Bun.file(path.join(pluginsNodeModules, "factory-failure-plugin", "package.json")).exists()).toBe(
false,
);
});
test("rejects an install whose extension entry cannot resolve its dependencies", async () => {
vi.spyOn(Bun, "spawn").mockImplementation(((cmd: string[]) => {
expect(cmd).toEqual(["bun", "install", "broken-plugin"]);