diff --git a/docs/plugin-manager-installer-plumbing.md b/docs/plugin-manager-installer-plumbing.md index a9329bf91..a2f69a709 100644 --- a/docs/plugin-manager-installer-plumbing.md +++ b/docs/plugin-manager-installer-plumbing.md @@ -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 diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fa9ab5469..72dd04126 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -111,6 +111,7 @@ - Fixed the DAP `runInTerminal` reverse request leaving the spawned debuggee's stdout undrained: the child was spawned with a piped stdout that was never consumed, so a chatty debuggee's output buffered unboundedly in the omp process (toward OOM) and was lost from the session output. Its stdout is now continuously drained into the session output buffer. ([#8111](https://github.com/can1357/oh-my-pi/issues/8111)) - Fixed the `/ssh add` inline hint omitting the `--scope project|user` option. - Fixed `omp://` throwing `ENOENT` for npm/SDK consumers: `@oh-my-pi/pi-coding-agent`'s `exports` resolve to TypeScript source where the build-time `PI_DOCS_EMBED` is empty, and the dev-tree fallback pointed at an unreachable `node_modules/docs`, so `OmpProtocolHandler.complete()`/`.resolve()` crashed for any consumer importing the package from npm. `gen:bundle` now also ships the docs corpus as `dist/docs-index.generated.txt`, the source path reads it when the env embed is empty and the on-disk `docs/` is absent, and a missing corpus degrades to an empty index (with a warning) instead of propagating `ENOENT` ([#8134](https://github.com/can1357/oh-my-pi/issues/8134)). +- Fixed the legacy TypeBox facade rejecting `Type.Optional(Type.Unsafe(...))`, losing optional object properties when raw schemas were present, dropping JSON-Schema-only keywords (e.g. `patternProperties`) from nested `Type.Unsafe` wire schemas, 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 diff --git a/packages/coding-agent/src/extensibility/legacy-typebox.ts b/packages/coding-agent/src/extensibility/legacy-typebox.ts index 7270aaebd..0c9cb94ac 100644 --- a/packages/coding-agent/src/extensibility/legacy-typebox.ts +++ b/packages/coding-agent/src/extensibility/legacy-typebox.ts @@ -1,4 +1,6 @@ +import { type } from "@oh-my-pi/omptype"; import { + type AnySchema, type ObjectOpts, Type as OmpType, type TypeBuilder as OmpTypeBuilder, @@ -25,11 +27,18 @@ interface SafeParseFailure { error: ValidationFailure; } -type LegacyUnsafeSchema = Record & - TUnsafe & { - __validator(data: unknown): T | ValidationFailure; - safeParse(input: unknown): SafeParseSuccess | SafeParseFailure; - }; +type LegacyUnsafeSchema = TUnsafe & { + __validator(data: unknown): T | ValidationFailure; + safeParse(input: unknown): SafeParseSuccess | SafeParseFailure; +}; + +function isValidationFailure(result: T | ValidationFailure): result is ValidationFailure { + return typeof result === "object" && result !== null && VALIDATION_FAILURE in result; +} + +function isRuntimeSchema(value: unknown): value is AnySchema { + return typeof value === "function"; +} function defineHidden(target: object, key: PropertyKey, value: unknown): void { Object.defineProperty(target, key, { @@ -40,8 +49,13 @@ function defineHidden(target: object, key: PropertyKey, value: unknown): void { } function unsafe(jsonSchema: Record = {}): LegacyUnsafeSchema { - const schema = { ...jsonSchema } as LegacyUnsafeSchema; - const upgradedSchema = upgradeJsonSchemaTo202012(jsonSchema); + // `document` is the verbatim wire schema; keep it isolated from the validator. + // `upgradeJsonSchemaTo202012` returns its input untouched when no upgrade is + // needed, and `validateJsonSchemaValue` then annotates that object with JIT + // epoch metadata and normalized keywords — which would leak into emission if + // the two shared a reference. + const document = structuredClone(jsonSchema); + const upgradedSchema = upgradeJsonSchemaTo202012(structuredClone(jsonSchema)); const validate = (data: unknown): T | ValidationFailure => { const result = validateJsonSchemaValue(upgradedSchema, data); if (result.success) return data as T; @@ -54,47 +68,59 @@ function unsafe(jsonSchema: Record = {}): LegacyUn defineHidden(failure, VALIDATION_FAILURE, true); return failure; }; + // Validate through the authoritative JSON Schema validator, not + // `fromJsonSchema`: lowering `additionalProperties: false` while dropping the + // keywords it cannot model (e.g. `patternProperties`) would reject values the + // raw document accepts. `type.withJsonSchema` then emits the raw document + // verbatim so nested composition (`Type.Object`, `Type.Optional`) keeps every + // keyword in the wire schema, not just at the top level. + const runtime = type.unknown.narrow((data, ctx) => { + const result = validate(data); + return isValidationFailure(result) ? ctx.mustBe(result.message) : true; + }); + const schema = type.withJsonSchema(runtime, document) as unknown as LegacyUnsafeSchema; defineHidden(schema, "__validator", validate); defineHidden(schema, "safeParse", (input: unknown): SafeParseSuccess | 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, opts?: ObjectOpts) => { + let normalizedOpts = opts; + const additionalProperties: unknown = opts?.additionalProperties; + if ( + additionalProperties !== undefined && + typeof additionalProperties !== "boolean" && + !isRuntimeSchema(additionalProperties) + ) { + normalizedOpts = { + ...opts, + additionalProperties: unsafe(additionalProperties as Record), + }; + } + let hasRawProperty = false; for (const key in properties) { - if (typeof properties[key] !== "function") { + if (!isRuntimeSchema(properties[key])) { hasRawProperty = true; break; } } - if (!hasRawProperty) return OmpType.Object(properties as Parameters[0], opts); + if (!hasRawProperty) { + return OmpType.Object(properties as Record, normalizedOpts); + } - const propertySchemas: Record = {}; - const required: string[] = []; + const normalizedProperties: Record = {}; for (const key in properties) { const property = properties[key]; - propertySchemas[key] = - typeof property === "function" && "toJsonSchema" in property - ? (property as { toJsonSchema(): Record }).toJsonSchema() - : property; - required.push(key); + normalizedProperties[key] = isRuntimeSchema(property) ? property : unsafe(property as Record); } - const document: Record = { type: "object", properties: propertySchemas, required }; - if (opts?.additionalProperties !== undefined) { - document.additionalProperties = - typeof opts.additionalProperties === "function" - ? opts.additionalProperties.toJsonSchema() - : opts.additionalProperties; - } - return unsafe(document); + return OmpType.Object(normalizedProperties, normalizedOpts); }) as typeof OmpType.Object; -export const Type: OmpTypeBuilder = { ...OmpType, Object: object, Unsafe: unsafe } as unknown as OmpTypeBuilder; +export const Type = { ...OmpType, Object: object, Unsafe: unsafe } as unknown as OmpTypeBuilder; export type TypeBuilder = OmpTypeBuilder; const legacyTypeBox: { Type: OmpTypeBuilder } = { Type }; diff --git a/packages/coding-agent/src/extensibility/plugins/manager.ts b/packages/coding-agent/src/extensibility/plugins/manager.ts index 564f31a60..e4959e20f 100644 --- a/packages/coding-agent/src/extensibility/plugins/manager.ts +++ b/packages/coding-agent/src/extensibility/plugins/manager.ts @@ -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): 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}`); } } diff --git a/packages/coding-agent/test/extensibility/typebox-remap.test.ts b/packages/coding-agent/test/extensibility/typebox-remap.test.ts index 3963b02ef..d95b6ce8d 100644 --- a/packages/coding-agent/test/extensibility/typebox-remap.test.ts +++ b/packages/coding-agent/test/extensibility/typebox-remap.test.ts @@ -68,10 +68,43 @@ describe("legacy-pi TypeBox remap", () => { }; expect(loaded.probe).toBe(TypeBoxShimType); - expect(toolWireSchema({ name: "fixture", description: "", parameters: loaded.schema })).toEqual({ + // `Type.Unsafe` is now a first-class omptype schema (so `Type.Optional`/ + // `Type.Object` can compose it), so a top-level Unsafe tool param takes the + // omptype wire path and is closed like every other tool param. Compare the + // JSON-serialized wire — internal memoization stamps are non-serialized. + const wire = toolWireSchema({ name: "fixture", description: "", parameters: loaded.schema }); + expect(JSON.parse(JSON.stringify(wire))).toEqual({ type: "object", properties: { path: { type: "string" } }, required: ["path"], + additionalProperties: false, + }); + }); + + it("preserves raw JSON Schema properties passed directly to Type.Object", async () => { + const entry = await writeFixtureExtension( + [ + 'import { Type } from "typebox";', + "export const schema = Type.Object({ cfg: { type: 'string', pattern: '^ok' }, label: Type.Optional(Type.String()) });", + ].join("\n"), + ); + + const loaded = (await loadLegacyPiModule(entry)) as { + schema: Record & { safeParse(input: unknown): { success: boolean } }; + }; + + expect(loaded.schema.safeParse({ cfg: "okay" }).success).toBe(true); + expect(loaded.schema.safeParse({ cfg: "bad" }).success).toBe(false); + expect(loaded.schema.safeParse({ cfg: { type: "string" } }).success).toBe(false); + const wire = toolWireSchema({ name: "fixture", description: "", parameters: loaded.schema }); + expect(JSON.parse(JSON.stringify(wire))).toEqual({ + type: "object", + properties: { + cfg: { type: "string", pattern: "^ok" }, + label: { type: "string" }, + }, + required: ["cfg"], + additionalProperties: false, }); }); diff --git a/packages/coding-agent/test/extensibility/typebox-shim.test.ts b/packages/coding-agent/test/extensibility/typebox-shim.test.ts index ade6fc715..b1952aa77 100644 --- a/packages/coding-agent/test/extensibility/typebox-shim.test.ts +++ b/packages/coding-agent/test/extensibility/typebox-shim.test.ts @@ -59,6 +59,53 @@ 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("preserves nested Type.Unsafe keywords fromJsonSchema cannot lower", () => { + const raw = Type.Unsafe({ + type: "object", + properties: { a: { type: "string" } }, + patternProperties: { "^x-": { type: "number" } }, + additionalProperties: false, + }); + const nested = { + type: "object", + properties: { a: { type: "string" } }, + patternProperties: { "^x-": { type: "number" } }, + additionalProperties: false, + }; + + // The wire schema must keep patternProperties even when the Unsafe schema + // is embedded inside Type.Object / Type.Optional — a `.toJsonSchema` + // method override would vanish at these nested positions. + expect((Type.Object({ cfg: raw }).toJsonSchema().properties as Record).cfg).toEqual(nested); + expect( + (Type.Object({ cfg: Type.Optional(raw) }).toJsonSchema().properties as Record).cfg, + ).toEqual(nested); + + // Runtime validation still enforces the nested keyword. + const object = Type.Object({ cfg: raw }); + expect(object.safeParse({ cfg: { a: "ok", "x-n": 3 } }).success).toBe(true); + expect(object.safeParse({ cfg: { a: "ok", "x-n": "bad" } }).success).toBe(false); + }); + it("validates Type.Unsafe draft-07 documents like the wire path", () => { const schema = Type.Unsafe({ type: "object", diff --git a/packages/coding-agent/test/plugin-install-validation.test.ts b/packages/coding-agent/test/plugin-install-validation.test.ts index 1621954ce..e4efcd125 100644 --- a/packages/coding-agent/test/plugin-install-validation.test.ts +++ b/packages/coding-agent/test/plugin-install-validation.test.ts @@ -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"]); diff --git a/packages/omptype/CHANGELOG.md b/packages/omptype/CHANGELOG.md index 1d0964543..542ae301a 100644 --- a/packages/omptype/CHANGELOG.md +++ b/packages/omptype/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added `type.withJsonSchema(schema, json)`, wrapping a validation-only schema so JSON Schema emission yields `json` verbatim even when embedded in objects, arrays, or unions — a `.toJsonSchema()` method override is dropped at nested positions because parents emit a child's IR directly. Schemas with defaults or output-changing morphs are rejected to prevent their transformed outputs from being discarded. + ## [17.2.10] - 2026-08-06 ### Changed diff --git a/packages/omptype/src/type.ts b/packages/omptype/src/type.ts index 49da769da..eff13bcc2 100644 --- a/packages/omptype/src/type.ts +++ b/packages/omptype/src/type.ts @@ -3675,6 +3675,42 @@ export namespace type { export function raw(def: unknown): BaseType { return makeType(parseDef(def), [], {}) as unknown as BaseType; } + + /** + * Return a validation-only schema that emits `json` verbatim — even when + * embedded in an object, array, or union. + * + * A `.toJsonSchema()` method override cannot survive nesting: a parent schema + * emits each child's IR directly and never calls the child's method, so the + * override silently disappears from the wire schema. This stores the override + * on the IR instead. + * + * # Errors + * + * Throws when `schema` has a default or output-changing morph/pipe. A refine + * can preserve validation and the input value, but silently discarding a + * transformed output would violate the returned {@link Type}. + */ + export function withJsonSchema(schema: Type, json: Record): Type { + const internal = schema as unknown as InternalType; + if (internal.hasDefault || hasMorph(internal.ir) || internal[kSteps].some(step => step.kind === "pipe")) { + throw new OmpTypeError("type.withJsonSchema cannot wrap schemas with defaults or output-changing morphs"); + } + return makeType( + { + k: "refine", + base: { k: "unknown" }, + pred: value => { + const result = schema(value); + return result instanceof OmpErrors ? result : true; + }, + expected: schema.expression, + json: { ...json }, + }, + [], + {}, + ); + } } // Reserved words cannot be declared as namespace bindings, but ArkType exposes diff --git a/packages/omptype/test/type.test.ts b/packages/omptype/test/type.test.ts index 5412d5c5e..a079a66c8 100644 --- a/packages/omptype/test/type.test.ts +++ b/packages/omptype/test/type.test.ts @@ -491,3 +491,38 @@ describe("Standard Schema V1", () => { expect(first).not.toBe(second); }); }); + +describe("type.withJsonSchema", () => { + it("emits the override verbatim even when embedded, and still validates", () => { + const raw = { type: "string", enum: ["a", "b"], "x-vendor": true }; + const inner = type.withJsonSchema( + type.unknown.narrow(v => v === "a" || v === "b"), + raw, + ); + + // Top-level emission is the override. + expect(inner.toJsonSchema()).toEqual(raw); + // Nested inside an object, the override survives (a `.toJsonSchema` + // method override would be dropped by the parent emitter here). + const object = type({ mode: inner }); + expect((object.toJsonSchema().properties as Record).mode).toEqual(raw); + + // Runtime validation is delegated to the wrapped schema. + expect(inner("a")).toBe("a"); + expect(inner("c")).toBeInstanceOf(OmpErrors); + expect(object({ mode: "b" })).toEqual({ mode: "b" }); + expect(object({ mode: "c" })).toBeInstanceOf(OmpErrors); + }); + + it("rejects defaults and output-changing morphs", () => { + expect(() => type.withJsonSchema(type.string.default("fallback"), { type: "string" })).toThrow( + "cannot wrap schemas with defaults or output-changing morphs", + ); + expect(() => + type.withJsonSchema(type("string.integer.parse"), { + type: "string", + pattern: "^[0-9]+$", + }), + ).toThrow("cannot wrap schemas with defaults or output-changing morphs"); + }); +});