mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(schema): apply a relative root $id only once when resolving references (#11726)
A schema whose root `$id` was a relative path with a directory part, such as `schemas/tool.json`, could not resolve its local `$ref`s. The `Resolver` constructor applied the root `$id` to `DEFAULT_BASE` once to compute `rootBase`. The compiler, the error explainer and the coercer start from `rootBase` and call `baseFor(root, rootBase)`, which applied the `$id` a second time. Local references therefore resolved against `https://json-schema.invalid/schemas/schemas/tool.json`. The effect was that `compile(schema)` threw `Unresolved $ref #/$defs/count`. With `unresolvedRefs: "reject"`, which is how tool arguments are validated (pi-ai's `validateToolArguments`, used by the agent loop and durable, and durable's `structuredOutput` check), valid arguments were rejected and the tool never ran. Absolute `$id`s and bare names like `tool.json` were not affected, because resolving them twice gives the same URI. The fix: - `rootBase` is now `DEFAULT_BASE`; walkers apply the root's own `$id` through `baseFor`. - `baseFor` indexes the schema before applying an `$id` and then returns the indexed base, so a caller whose base already includes the `$id` cannot apply it again. Without this, `unevaluatedProperties`/`unevaluatedItems` still failed: `build()` passes the already-based node to `annotating()`, which applied the `$id` once more. - The coercer resolves bases through `baseFor` instead of reading `baseOf` directly. Otherwise the `rootBase` change broke the first `parse`/`coerce` of a schema with `$dynamicRef`: the restarted dynamic compiler is not indexed yet, so a relative `$ref` resolved against `DEFAULT_BASE` and conversion was silently skipped. - The explainer drops `$id` from the copy it makes for unevaluated keywords, and resolves `anyOf`/`oneOf` branch `$ref`s against the branch's own base (both threw in `errors()` on main for nested resources with a relative `$id`). Testing: - Regression tests in `validator.test.ts` (relative root `$id` with local `$ref`, with `unevaluatedProperties`/`unevaluatedItems`, error reporting under nested relative resources) and `coerce.test.ts` (first coercion of a `$dynamicRef` schema with a relative external `$ref`). They fail without the change and pass with it. - Full pi-schema suite: 8712 passed. - Independently reviewed against edge cases: draft 7 (inert `$id` beside `$ref`, `#anchor` ids), fragment ids, nested relative chains, anchors, registered schemas referring back to the root, fresh `parse`/`coerce`/`errors` after a dynamic restart. Fixes #11697 Requested by @badlogic --------- Co-authored-by: triagent <triagent@localhost> Co-authored-by: Petr Houška <houskape@gmail.com>
This commit is contained in:
parent
eebd5d90ca
commit
2bd2c2d0a6
6 changed files with 101 additions and 8 deletions
|
|
@ -2,6 +2,10 @@
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed local `$ref`s in schemas whose root `$id` is a relative path with a directory, such as `schemas/tool.json`: the `$id` was applied twice, so `compile` threw and `unresolvedRefs: "reject"` rejected valid tool arguments. Relative `$id`s are now also applied once with `unevaluatedProperties`/`unevaluatedItems`, when explaining errors under nested resources, and on the first coercion of a schema with `$dynamicRef` ([#11697](https://github.com/earendil-works/pi/issues/11697))
|
||||
|
||||
## [1.5.0] - 2026-11-02
|
||||
|
||||
### Added
|
||||
|
|
|
|||
|
|
@ -105,7 +105,7 @@ export class Coercer {
|
|||
}
|
||||
|
||||
private baseOf(node: unknown, base: string): string {
|
||||
return isSchemaObject(node) ? (this.c.r.baseOf.get(node) ?? base) : base;
|
||||
return isSchemaObject(node) ? this.c.r.baseFor(node, base) : base;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -314,14 +314,15 @@ export class Explainer {
|
|||
|
||||
/** How many of the keys of `v` the branch `s` (through a `$ref`) declares in `properties`. */
|
||||
private declaredKeys(s: unknown, b: string, v: Record<string, unknown>): number {
|
||||
const node = isSchemaObject(s) && typeof s.$ref === "string" ? this.c.r.resolve(s.$ref, b).node : s;
|
||||
const node =
|
||||
isSchemaObject(s) && typeof s.$ref === "string" ? this.c.r.resolve(s.$ref, this.c.r.baseFor(s, b)).node : s;
|
||||
const properties = isSchemaObject(node) && isSchemaObject(node.properties) ? node.properties : {};
|
||||
return Object.keys(v).filter((k) => Object.hasOwn(properties, k)).length;
|
||||
}
|
||||
|
||||
private tagOf(branches: readonly unknown[], b: string): { key: string; values: unknown[] } | undefined {
|
||||
const resolved = branches.map((s) =>
|
||||
isSchemaObject(s) && typeof s.$ref === "string" ? this.c.r.resolve(s.$ref, b).node : s,
|
||||
isSchemaObject(s) && typeof s.$ref === "string" ? this.c.r.resolve(s.$ref, this.c.r.baseFor(s, b)).node : s,
|
||||
);
|
||||
const first = resolved[0];
|
||||
if (!isSchemaObject(first) || !isSchemaObject(first.properties)) return undefined;
|
||||
|
|
@ -337,7 +338,8 @@ export class Explainer {
|
|||
}
|
||||
|
||||
private walkUnevaluated(node: SchemaObject, b: string, v: unknown, path: string, out: ValidationError[]): void {
|
||||
const { unevaluatedProperties, unevaluatedItems, ...rest } = node;
|
||||
// `b` already applies the node's `$id`; the copy must not apply it again.
|
||||
const { unevaluatedProperties, unevaluatedItems, $id: _id, ...rest } = node;
|
||||
const a = newAnnotations();
|
||||
this.c.annotating(rest, b)(v, a);
|
||||
if (isObject(v) && unevaluatedProperties !== undefined && !a.allProps) {
|
||||
|
|
|
|||
|
|
@ -14,11 +14,12 @@ export const DEFAULT_BASE = "https://json-schema.invalid/root.json";
|
|||
|
||||
/**
|
||||
* Resolves references in the root schema and the registered documents. The schema is indexed on the first
|
||||
* reference, so a schema without references is never walked for identifiers.
|
||||
* reference or the first `$id` a walker meets, so a schema with neither is never walked for identifiers.
|
||||
*/
|
||||
export class Resolver {
|
||||
readonly draft: "2020-12" | "7";
|
||||
readonly rootBase: string;
|
||||
/** The base URI the root schema is walked from; walkers apply the root's own `$id` through `baseFor`. */
|
||||
readonly rootBase = DEFAULT_BASE;
|
||||
/** Schema object to the base URI it was indexed under. */
|
||||
readonly baseOf = new Map<object, string>();
|
||||
/** URI (with a `#name` fragment) of every `$dynamicAnchor`, to its schema. */
|
||||
|
|
@ -38,7 +39,6 @@ export class Resolver {
|
|||
this.root = root;
|
||||
this.options = options;
|
||||
this.rejectUnresolved = options.unresolvedRefs === "reject";
|
||||
this.rootBase = isSchemaObject(root) ? this.baseFor(root, DEFAULT_BASE) : DEFAULT_BASE;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -46,10 +46,16 @@ export class Resolver {
|
|||
* the schema is indexed, the indexed base (a resolved reference target already has its own).
|
||||
*/
|
||||
baseFor(node: SchemaObject, base: string): string {
|
||||
const indexed = this.baseOf.get(node);
|
||||
let indexed = this.baseOf.get(node);
|
||||
if (indexed !== undefined) return indexed;
|
||||
const id = node.$id;
|
||||
if (typeof id !== "string") return base;
|
||||
// Index before applying an `$id`: a caller's `base` may already include it, and the index applies it once.
|
||||
if (!this.indexed) {
|
||||
this.ensureIndexed();
|
||||
indexed = this.baseOf.get(node);
|
||||
if (indexed !== undefined) return indexed;
|
||||
}
|
||||
if (this.draft === "7" && (typeof node.$ref === "string" || id.startsWith("#"))) return base;
|
||||
return documentOf(resolveUri(id, base));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -201,6 +201,20 @@ describe("pass 2: TypeBox's conversion rules on JSON Schema", () => {
|
|||
expect(coerce(schema, { item: { id: "7" } })).toEqual({ item: { id: 7 } });
|
||||
});
|
||||
|
||||
it("follows a relative $ref from the root $id on the first call of a schema with $dynamicRef", () => {
|
||||
const schema = {
|
||||
$id: "https://example.com/root.json",
|
||||
$dynamicAnchor: "node",
|
||||
type: "object",
|
||||
properties: { count: { $ref: "defs/n.json" }, children: { type: "array", items: { $dynamicRef: "#node" } } },
|
||||
};
|
||||
const options = { coerce: true, schemas: { "https://example.com/defs/n.json": { type: "integer" } } } as const;
|
||||
expect(compile(schema, options).coerce({ count: "2" })).toEqual({ count: 2 });
|
||||
const validator = compile(schema, options);
|
||||
expect(validator.parse({ count: "2" })).toEqual({ ok: true, value: { count: 2 } });
|
||||
expect(validator.parse({ count: "2" })).toEqual({ ok: true, value: { count: 2 } });
|
||||
});
|
||||
|
||||
it("converts patternProperties and additionalProperties", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
|
|
|
|||
|
|
@ -176,6 +176,73 @@ describe("schemas", () => {
|
|||
]);
|
||||
});
|
||||
|
||||
// #11697
|
||||
it("applies a relative root $id once when resolving local references", () => {
|
||||
const schema = {
|
||||
$id: "schemas/tool.json",
|
||||
type: "object",
|
||||
properties: { count: { $ref: "#/$defs/count" } },
|
||||
required: ["count"],
|
||||
$defs: { count: { type: "integer" } },
|
||||
};
|
||||
expect(compile(schema).check({ count: 2 })).toBe(true);
|
||||
const validator = compile(schema, { unresolvedRefs: "reject" });
|
||||
expect(validator.check({ count: 2 })).toBe(true);
|
||||
expect(validator.errors({ count: "a" })).toEqual([
|
||||
{ path: "/count", keyword: "type", message: "expected integer, got string" },
|
||||
]);
|
||||
expect(compile(schema, { coerce: true }).parse({ count: "2" })).toEqual({ ok: true, value: { count: 2 } });
|
||||
});
|
||||
|
||||
it("applies a relative root $id once in schemas with unevaluated keywords", () => {
|
||||
const props = {
|
||||
$id: "schemas/tool.json",
|
||||
type: "object",
|
||||
properties: { count: { $ref: "#/$defs/count" } },
|
||||
$defs: { count: { type: "integer" } },
|
||||
unevaluatedProperties: false,
|
||||
};
|
||||
const validator = compile(props, { unresolvedRefs: "reject" });
|
||||
expect(validator.check({ count: 2 })).toBe(true);
|
||||
expect(validator.errors({ count: 2, extra: 1 })).toEqual([
|
||||
{ path: "/extra", keyword: "unevaluatedProperties", message: "unknown field" },
|
||||
]);
|
||||
const items = {
|
||||
$id: "schemas/list.json",
|
||||
type: "array",
|
||||
prefixItems: [{ $ref: "#/$defs/count" }],
|
||||
$defs: { count: { type: "integer" } },
|
||||
unevaluatedItems: false,
|
||||
};
|
||||
expect(compile(items, { unresolvedRefs: "reject" }).check([2])).toBe(true);
|
||||
expect(compile(items).check([2, 3])).toBe(false);
|
||||
});
|
||||
|
||||
it("explains errors under nested resources with a relative $id", () => {
|
||||
const cloned = {
|
||||
$id: "https://example.com/root.json",
|
||||
allOf: [{ $ref: "#/$defs/child" }],
|
||||
$defs: {
|
||||
child: {
|
||||
$id: "schemas/tool.json",
|
||||
$ref: "#/$defs/props",
|
||||
unevaluatedProperties: false,
|
||||
$defs: { props: { properties: { count: { type: "integer" } } } },
|
||||
},
|
||||
},
|
||||
};
|
||||
expect(compile(cloned).errors({ count: 2, extra: 1 })).toEqual([
|
||||
{ path: "/extra", keyword: "unevaluatedProperties", message: "unknown field" },
|
||||
]);
|
||||
const branch = {
|
||||
$id: "https://example.com/root.json",
|
||||
anyOf: [{ $id: "a/b.json", $ref: "#/$defs/n", $defs: { n: { type: "integer" } } }, { type: "boolean" }],
|
||||
};
|
||||
expect(compile(branch).errors({})).toEqual([
|
||||
{ path: "", keyword: "type", message: "expected integer, got object" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("throws for a malformed schema", () => {
|
||||
expect(() => compile({ properties: { a: 1 } })).toThrow(/must be an object or a boolean/);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue