mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): follow local schema refs in tool search and codemode summaries (#11850)
Fixes #11803. **Problem** `tool_search` builds a tool's search text from its parameter schema but never followed `$ref`. MCP servers (Pydantic, FastMCP) often put nested models in `$defs`: ```json { "properties": { "options": { "$ref": "#/$defs/options" } }, "$defs": { "options": { "properties": { "mode": { "description": "reconciliation" } } } } } ``` Search text was `Inspect records options`, so `tool_search("reconciliation")` found nothing. Durable's copy of `tool_search` had the same gap. The codemode `resolves to` summary had the same blind spot for output schemas: a field behind a `$ref` showed as `structuredContent?` with no field names. **Fix** - pi-codemode exports its existing local JSON Pointer resolver as `resolveSchemaRef(ref, root)`. A malformed percent escape now resolves to nothing instead of throwing `URIError` (this also affected `schemaToType()`). - coding-agent and Durable `schemaText()` follow local `$ref`s. Each referenced subschema is read once, so recursive and shared definitions neither loop nor inflate BM25 term counts. Unreferenced `$defs` and remote refs stay out. - coding-agent `describeFields()`/`describeUnionFields()` resolve `$ref` chains against the output schema root. **Tests** - `packages/codemode/test/declarations.test.ts`: escaped, remote, missing, malformed refs. - `packages/coding-agent/test/tool-search.test.ts`: refs, escaped segments, recursion and shared definitions, unused definitions. - `packages/coding-agent/test/suite/agent-session-codemode.test.ts`: summary through root, field, const, and union refs. - `packages/durable/test/tool-search.test.ts`: end-to-end load via a ref'd description; unused definition does not match. `npm run check` passes.
This commit is contained in:
Petr Houška
GitHub
parent
850ea2eae2
commit
6d3e342b25
12 changed files with 262 additions and 29 deletions
|
|
@ -38,7 +38,9 @@
|
|||
- Fixed Ctrl+P not switching to the only model in scope (or the only available model) when the active model is a different one; it showed "Only one model in scope" instead. Cycling forward from a model outside the scope now also selects the first scoped model instead of skipping it ([#11806](https://github.com/earendil-works/pi/issues/11806))
|
||||
- Fixed `/reload` and session switches leaving extension overlays on screen when an extension had more than one `ctx.ui.custom({ overlay: true })` open: only the topmost overlay was removed, and the others kept focus and input although the extension that opened them was gone. All extension overlays, including hidden ones, are now removed and disposed; pi's own overlays stay open
|
||||
- Fixed `/mcp` sign-in staying on "Connecting…" for minutes after a successful browser sign-in, with Ctrl+C doing nothing: pi waited for the browser to drop an unused connection to the OAuth callback port. The callback server now closes remaining connections when it shuts down ([#11311](https://github.com/earendil-works/pi/issues/11311))
|
||||
|
||||
- Fixed `tool_search` and `searchTools()` not finding a tool by property names and descriptions behind a local `$ref`, as in MCP input schemas that put their models in `$defs`. Referenced definitions are read once, so recursive schemas do not loop, and definitions nothing references stay out of the search text ([#11803](https://github.com/earendil-works/pi/issues/11803))
|
||||
- Fixed the codemode `resolves to` summary of a tool leaving out field names behind a local `$ref`, for example `structuredContent?` instead of `structuredContent?: { results }` for MCP output schemas that put their models in `$defs` ([#11803](https://github.com/earendil-works/pi/issues/11803))
|
||||
- Fixed every codemode script failing with `URI malformed` when any callable tool, such as an MCP tool, had a `$ref` with a malformed percent escape (`#/$defs/%E0`) in its schema, even scripts that do not call that tool. The parameter now renders as `unknown` in the tool's declaration ([#11803](https://github.com/earendil-works/pi/issues/11803))
|
||||
|
||||
## [1.5.0] - 2026-11-02
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,12 @@
|
|||
import { join } from "node:path";
|
||||
import type { AgentTool } from "@earendil-works/pi-agent-core";
|
||||
import type { CodemodeJsonSchema, CodemodeTool } from "@earendil-works/pi-codemode";
|
||||
import { renderToolOutputType, renderToolSample, toCodemodeIdentifier } from "@earendil-works/pi-codemode/declarations";
|
||||
import {
|
||||
renderToolOutputType,
|
||||
renderToolSample,
|
||||
resolveSchemaRef,
|
||||
toCodemodeIdentifier,
|
||||
} from "@earendil-works/pi-codemode/declarations";
|
||||
import { CODEMODE_SOURCE_GRAMMAR } from "@earendil-works/pi-codemode/source";
|
||||
import { type Static, Type } from "typebox";
|
||||
import { getDocsPath } from "../../config.ts";
|
||||
|
|
@ -272,20 +277,22 @@ export function createCodemodeDescription(
|
|||
/**
|
||||
* The field names of an object schema, `{ output, exit_code, full_output_path? }`, with the fields of
|
||||
* object-typed fields `depth` levels deep: `{ content, structuredContent?: { results } }`, and the
|
||||
* value of `const` fields: `{ action: "start", id }`. Undefined for other schemas.
|
||||
* value of `const` fields: `{ action: "start", id }`. Undefined for other schemas. Local `$ref`s resolve
|
||||
* against `root`.
|
||||
*/
|
||||
function describeFields(schema: unknown, depth: number): string | undefined {
|
||||
if (typeof schema !== "object" || schema === null) return undefined;
|
||||
const { type, properties, required } = schema as Record<string, unknown>;
|
||||
function describeFields(schema: unknown, root: CodemodeJsonSchema, depth: number): string | undefined {
|
||||
const resolved = followRef(schema, root);
|
||||
if (typeof resolved !== "object" || resolved === null) return undefined;
|
||||
const { type, properties, required } = resolved as Record<string, unknown>;
|
||||
if (type !== "object" || typeof properties !== "object" || properties === null) return undefined;
|
||||
const requiredNames = new Set(Array.isArray(required) ? required : []);
|
||||
const fields = Object.entries(properties).map(([name, field]) => {
|
||||
const constValue = (field as Record<string, unknown> | null)?.const;
|
||||
const constValue = (followRef(field, root) as Record<string, unknown> | null)?.const;
|
||||
const nested =
|
||||
constValue !== undefined
|
||||
? JSON.stringify(constValue)
|
||||
: depth > 0
|
||||
? describeFields(field, depth - 1)
|
||||
? describeFields(field, root, depth - 1)
|
||||
: undefined;
|
||||
return `${name}${requiredNames.has(name) ? "" : "?"}${nested ? `: ${nested}` : ""}`;
|
||||
});
|
||||
|
|
@ -301,7 +308,8 @@ function describeFields(schema: unknown, depth: number): string | undefined {
|
|||
function describeOutput(name: string, schema: CodemodeJsonSchema | undefined): string {
|
||||
const type = renderToolOutputType(schema);
|
||||
if (type === "string") return "a string";
|
||||
const fields = describeFields(schema, 1) ?? describeUnionFields(schema);
|
||||
const fields =
|
||||
schema === undefined ? undefined : (describeFields(schema, schema, 1) ?? describeUnionFields(schema, schema));
|
||||
if (fields) return `\`${fields}\``;
|
||||
if (!type.includes("\n")) return `\`${type}\``;
|
||||
return `a value; \`await describeTool(${JSON.stringify(name)})\` shows its type`;
|
||||
|
|
@ -311,15 +319,34 @@ function describeOutput(name: string, schema: CodemodeJsonSchema | undefined): s
|
|||
* The field names of each variant of an `anyOf`/`oneOf` whose variants are all objects, joined by
|
||||
* `|`.
|
||||
*/
|
||||
function describeUnionFields(schema: unknown): string | undefined {
|
||||
if (typeof schema !== "object" || schema === null) return undefined;
|
||||
const { anyOf, oneOf } = schema as Record<string, unknown>;
|
||||
function describeUnionFields(schema: unknown, root: CodemodeJsonSchema): string | undefined {
|
||||
const resolved = followRef(schema, root);
|
||||
if (typeof resolved !== "object" || resolved === null) return undefined;
|
||||
const { anyOf, oneOf } = resolved as Record<string, unknown>;
|
||||
const variants = anyOf ?? oneOf;
|
||||
if (!Array.isArray(variants) || variants.length === 0) return undefined;
|
||||
const described = variants.map((variant) => describeFields(variant, 0));
|
||||
const described = variants.map((variant) => describeFields(variant, root, 0));
|
||||
return described.every((fields) => fields !== undefined) ? described.join(" | ") : undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* The subschema the local `$ref` chain of `schema` names in `root`, so MCP output schemas that put their models in
|
||||
* `$defs` still show field names. Remote, missing, and cyclic references stop at the schema holding them.
|
||||
*/
|
||||
function followRef(schema: unknown, root: CodemodeJsonSchema): unknown {
|
||||
const seen = new Set<unknown>();
|
||||
let current = schema;
|
||||
while (typeof current === "object" && current !== null && !seen.has(current)) {
|
||||
const ref = (current as Record<string, unknown>).$ref;
|
||||
if (typeof ref !== "string") break;
|
||||
const target = resolveSchemaRef(ref, root);
|
||||
if (target === undefined) break;
|
||||
seen.add(current);
|
||||
current = target;
|
||||
}
|
||||
return current;
|
||||
}
|
||||
|
||||
/**
|
||||
* A declared tool's description followed by how scripts call it and what the call resolves to. The
|
||||
* arguments are the tool's declared parameters, so they are not repeated.
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@
|
|||
* like any other tool change and survives `/tree`, resume, and fork on that branch.
|
||||
*/
|
||||
|
||||
import { resolveSchemaRef } from "@earendil-works/pi-codemode/declarations";
|
||||
import { type Static, Type } from "typebox";
|
||||
import type {
|
||||
ExtensionAPI,
|
||||
|
|
@ -95,20 +96,31 @@ function isObject(value: unknown): value is Record<string, unknown> {
|
|||
return typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
}
|
||||
|
||||
/** Schema descriptions and property names, recursively. */
|
||||
function schemaText(schema: unknown, parts: string[]): void {
|
||||
/**
|
||||
* Schema descriptions and property names, recursively, following local `$ref`s into `root`. `seen` holds the
|
||||
* subschemas already reached through a reference, so recursive and shared definitions are read once, and
|
||||
* definitions no property references are not read.
|
||||
*/
|
||||
function schemaText(schema: unknown, root: unknown, parts: string[], seen: Set<unknown>): void {
|
||||
if (!isObject(schema)) return;
|
||||
if (typeof schema.$ref === "string") {
|
||||
const target = resolveSchemaRef(schema.$ref, root);
|
||||
if (isObject(target) && !seen.has(target)) {
|
||||
seen.add(target);
|
||||
schemaText(target, root, parts, seen);
|
||||
}
|
||||
}
|
||||
if (typeof schema.description === "string") parts.push(schema.description);
|
||||
if (isObject(schema.properties)) {
|
||||
for (const [name, property] of Object.entries(schema.properties)) {
|
||||
parts.push(name);
|
||||
schemaText(property, parts);
|
||||
schemaText(property, root, parts, seen);
|
||||
}
|
||||
}
|
||||
schemaText(schema.items, parts);
|
||||
schemaText(schema.items, root, parts, seen);
|
||||
for (const key of ["anyOf", "oneOf", "allOf"]) {
|
||||
const variants = schema[key];
|
||||
if (Array.isArray(variants)) for (const variant of variants) schemaText(variant, parts);
|
||||
if (Array.isArray(variants)) for (const variant of variants) schemaText(variant, root, parts, seen);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -121,7 +133,7 @@ export function createToolSearchDocument(
|
|||
namespace?: ToolNamespace,
|
||||
): ToolSearchDocument {
|
||||
const parts = [tool.description];
|
||||
schemaText(tool.parameters, parts);
|
||||
schemaText(tool.parameters, tool.parameters, parts, new Set([tool.parameters]));
|
||||
const namespaceText = namespace
|
||||
? [namespace.name, namespace.description ?? "", namespace.instructions ?? ""].join(" ")
|
||||
: "";
|
||||
|
|
|
|||
|
|
@ -230,6 +230,50 @@ describe("AgentSession codemode tool", () => {
|
|||
expect(description("count")).toBe("count tool\n\nCodemode: `tools.count(args)` resolves to `number`.");
|
||||
});
|
||||
|
||||
it("summarizes output fields behind local schema references", async () => {
|
||||
const outputTool = (name: string, outputSchema: Record<string, unknown>): AgentTool => ({
|
||||
name,
|
||||
label: name,
|
||||
description: `${name} tool`,
|
||||
parameters: Type.Object({}),
|
||||
outputSchema: Type.Unsafe(outputSchema),
|
||||
execute: async () => ({ content: [], details: {} }),
|
||||
});
|
||||
const report = outputTool("report", {
|
||||
$ref: "#/$defs/Report",
|
||||
$defs: {
|
||||
Report: {
|
||||
type: "object",
|
||||
properties: { kind: { $ref: "#/$defs/Kind" }, summary: { $ref: "#/$defs/Summary" } },
|
||||
required: ["kind", "summary"],
|
||||
},
|
||||
Kind: { const: "ledger" },
|
||||
Summary: { type: "object", properties: { total: { type: "number" } }, required: ["total"] },
|
||||
},
|
||||
});
|
||||
const result = outputTool("result", {
|
||||
anyOf: [{ $ref: "#/$defs/Ok" }, { $ref: "#/$defs/Failed" }],
|
||||
$defs: {
|
||||
Ok: { type: "object", properties: { value: { type: "string" } }, required: ["value"] },
|
||||
Failed: { type: "object", properties: { error: { type: "string" } }, required: ["error"] },
|
||||
},
|
||||
});
|
||||
const harness = await setup([
|
||||
(pi: ExtensionAPI) => {
|
||||
for (const tool of [report, result]) pi.registerTool(createToolDefinitionFromAgentTool(tool));
|
||||
},
|
||||
]);
|
||||
harness.session.setActiveToolsByName(["report", "result", "codemode"]);
|
||||
const description = (name: string) =>
|
||||
harness.session.agent.state.tools.find((tool) => tool.name === name)?.description ?? "";
|
||||
expect(description("report")).toBe(
|
||||
'report tool\n\nCodemode: `tools.report(args)` resolves to `{ kind: "ledger", summary: { total } }`.',
|
||||
);
|
||||
expect(description("result")).toBe(
|
||||
"result tool\n\nCodemode: `tools.result(args)` resolves to `{ value } | { error }`.",
|
||||
);
|
||||
});
|
||||
|
||||
// #10343
|
||||
it("shows the guidelines of tools that do not fit the inline budget through describeTool()", async () => {
|
||||
const harness = await setup();
|
||||
|
|
|
|||
|
|
@ -195,6 +195,61 @@ describe("Bm25Ranker", () => {
|
|||
});
|
||||
});
|
||||
|
||||
// #11803
|
||||
describe("schema references in the search text", () => {
|
||||
function searchText(parameters: Record<string, unknown>): string {
|
||||
const entry = tool("mcp__ledger__inspect", "Inspect records");
|
||||
return createToolSearchDocument({ ...entry, parameters: Type.Unsafe(parameters) }).text;
|
||||
}
|
||||
|
||||
it("reads names and descriptions behind local references", () => {
|
||||
const text = searchText({
|
||||
type: "object",
|
||||
properties: { options: { $ref: "#/$defs/options" } },
|
||||
$defs: {
|
||||
options: { type: "object", properties: { mode: { type: "string", description: "reconciliation" } } },
|
||||
},
|
||||
});
|
||||
expect(text).toBe("Inspect records options mode reconciliation");
|
||||
});
|
||||
|
||||
it("follows escaped pointer segments", () => {
|
||||
const text = searchText({
|
||||
type: "object",
|
||||
properties: { range: { $ref: "#/$defs/date~1range" } },
|
||||
$defs: { "date/range": { description: "fiscal period" } },
|
||||
});
|
||||
expect(text).toBe("Inspect records range fiscal period");
|
||||
});
|
||||
|
||||
it("reads recursive and shared definitions once", () => {
|
||||
const text = searchText({
|
||||
type: "object",
|
||||
properties: { root: { $ref: "#/$defs/node" }, other: { $ref: "#/$defs/node" }, self: { $ref: "#" } },
|
||||
$defs: {
|
||||
node: {
|
||||
description: "tree node",
|
||||
properties: { children: { type: "array", items: { $ref: "#/$defs/node" } } },
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(text).toBe("Inspect records root tree node children other self");
|
||||
});
|
||||
|
||||
it("leaves out unreferenced definitions and remote or missing references", () => {
|
||||
const text = searchText({
|
||||
type: "object",
|
||||
properties: {
|
||||
remote: { $ref: "https://example.com/schema.json#/$defs/x" },
|
||||
missing: { $ref: "#/$defs/missing" },
|
||||
malformed: { $ref: "#/$defs/%E0%A4%A" },
|
||||
},
|
||||
$defs: { unused: { description: "reconciliation" } },
|
||||
});
|
||||
expect(text).toBe("Inspect records remote missing malformed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("codemode description catalog", () => {
|
||||
const plain = tool("read_notes", "Read notes.");
|
||||
const github = ["a", "b", "c"].map((suffix) => tool(`mcp__github__${suffix}`, `GitHub ${suffix}.`));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue