From 53056478e97e2ec30d372a6955957b79575e5226 Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Fri, 25 Sep 2026 19:35:49 -0700 Subject: [PATCH 1/3] Add schema-aware fast actions for MCP tools Expand MCP JSON Schema conversion to support common nullable, composed, open-object, array, and local-reference forms while preserving existing catalog safety limits and runtime validation. Enable learned grammar generation for inline dynamic schemas and persist schema hashes plus stable provider/action fingerprints with generated rules. Reconcile these identities during startup and catalog refresh, suspending stale rules and falling back to normal translation when a tool changes or disappears. Rules are automatically reactivated when the original identity returns. Also expose suspended-rule status in grammar management, migrate existing grammar stores compatibly, and roll back rules that fail in-memory activation. Add focused coverage for schema conversion, tool fingerprints, manifest bindings, grammar-store migration, and stale-rule reconciliation. --- ts/packages/actionGrammar/src/grammarStore.ts | 131 +++++- ts/packages/actionGrammar/src/index.ts | 4 + .../actionGrammar/test/grammarStore.spec.ts | 86 ++++ .../actionSchema/src/jsonSchemaParser.ts | 383 +++++++++++------- .../actionSchema/src/jsonSchemaTypes.ts | 35 +- .../test/jsonSchemaParser.spec.ts | 179 ++++++++ ts/packages/agentSdk/src/agentInterface.ts | 10 + ts/packages/agentSdk/src/index.ts | 1 + ts/packages/cache/src/cache/cache.ts | 35 +- .../src/explanation/schemaInfoProvider.ts | 10 + .../src/mcp/mcpServerProvider.ts | 11 + .../src/mcp/mcpToolCatalog.ts | 40 +- .../test/mcpServerProvider.spec.ts | 6 + .../test/mcpToolCatalog.spec.ts | 52 +++ .../dispatcher/src/context/appAgentManager.ts | 40 ++ .../src/context/commandHandlerContext.ts | 73 ++-- .../handlers/requestCommandHandler.ts | 2 +- .../system/action/grammarActionHandler.ts | 13 +- .../src/translation/actionSchemaFileCache.ts | 10 + 19 files changed, 885 insertions(+), 236 deletions(-) create mode 100644 ts/packages/actionSchema/test/jsonSchemaParser.spec.ts diff --git a/ts/packages/actionGrammar/src/grammarStore.ts b/ts/packages/actionGrammar/src/grammarStore.ts index d786eb06f9..084843dc11 100644 --- a/ts/packages/actionGrammar/src/grammarStore.ts +++ b/ts/packages/actionGrammar/src/grammarStore.ts @@ -19,6 +19,13 @@ import { Grammar, GrammarJson } from "./grammarTypes.js"; import { grammarFromJson } from "./grammarDeserializer.js"; import { grammarToJson } from "./grammarSerializer.js"; +export type GrammarRuleStatus = "active" | "suspended"; + +export type GrammarRuleActionBinding = { + sourceId: string; + actionFingerprint: string; +}; + /** * Stored grammar rule with metadata */ @@ -35,8 +42,26 @@ export interface StoredGrammarRule { actionName?: string; // The schema/agent this belongs to schemaName: string; + // Hash of the action schema used to generate this rule. + schemaHash?: string; + // Optional identity supplied by the dynamic action provider. + actionBinding?: GrammarRuleActionBinding; + status?: GrammarRuleStatus; + invalidationReason?: string; } +export type GrammarSchemaIdentity = { + schemaHash: string; + sourceId?: string; + actionFingerprints?: Readonly>; +}; + +export type GrammarRuleReconciliation = { + activated: number; + suspended: number; + unchanged: number; +}; + /** * Grammar store data structure (JSON serializable) */ @@ -72,7 +97,7 @@ export class GrammarStore { constructor() { this.data = { - version: "1.0", + version: "1.1", nextId: 1, schemas: {}, }; @@ -131,7 +156,7 @@ export class GrammarStore { */ public async addRule( rule: Omit, - ): Promise { + ): Promise { if (this.data.nextId === undefined) { this.data.nextId = 1; } @@ -139,6 +164,7 @@ export class GrammarStore { ...rule, id: this.data.nextId++, timestamp: Date.now(), + status: rule.status ?? "active", }; if (!this.data.schemas[rule.schemaName]) { @@ -149,6 +175,7 @@ export class GrammarStore { this._compiledCache = undefined; this.modified = true; await this.doAutoSave(); + return storedRule; } /** @@ -158,6 +185,12 @@ export class GrammarStore { return this.data.schemas[schemaName] || []; } + public getActiveRulesForSchema(schemaName: string): StoredGrammarRule[] { + return this.getRulesForSchema(schemaName).filter( + (rule) => rule.status !== "suspended", + ); + } + /** * Get all rules across all schemas */ @@ -169,6 +202,49 @@ export class GrammarStore { return rules; } + public getAllActiveRules(): StoredGrammarRule[] { + return this.getAllRules().filter((rule) => rule.status !== "suspended"); + } + + public async reconcileSchema( + schemaName: string, + identity: GrammarSchemaIdentity, + ): Promise { + const result: GrammarRuleReconciliation = { + activated: 0, + suspended: 0, + unchanged: 0, + }; + let changed = false; + for (const rule of this.getRulesForSchema(schemaName)) { + const reason = getInvalidationReason(rule, identity); + const nextStatus: GrammarRuleStatus = + reason === undefined ? "active" : "suspended"; + if ( + rule.status === nextStatus && + rule.invalidationReason === reason + ) { + result.unchanged++; + continue; + } + rule.status = nextStatus; + if (reason === undefined) { + delete rule.invalidationReason; + result.activated++; + } else { + rule.invalidationReason = reason; + result.suspended++; + } + changed = true; + } + if (changed) { + this._compiledCache = undefined; + this.modified = true; + await this.doAutoSave(); + } + return result; + } + /** * Delete a rule by index within a schema */ @@ -245,7 +321,7 @@ export class GrammarStore { */ public clear(): void { this.data = { - version: "1.0", + version: "1.1", nextId: this.data.nextId ?? 1, schemas: {}, }; @@ -258,6 +334,7 @@ export class GrammarStore { */ public async load(filePath: string): Promise { const resolvedPath = path.resolve(filePath); + let migrated = false; if (!fs.existsSync(resolvedPath)) { throw new Error(`Grammar store file not found: ${resolvedPath}`); @@ -268,7 +345,7 @@ export class GrammarStore { if (fileContent === "") { // Empty file indicates new/empty store this.data = { - version: "1.0", + version: "1.1", nextId: 1, schemas: {}, }; @@ -285,6 +362,7 @@ export class GrammarStore { // Migration: assign stable IDs to rules from older files that lack them if (!this.data.nextId) { + migrated = true; let maxId = 0; for (const rules of Object.values(this.data.schemas)) { for (const rule of rules) { @@ -298,13 +376,26 @@ export class GrammarStore { for (const rule of rules) { if (!rule.id) { rule.id = this.data.nextId++; + migrated = true; } } } } + if (this.data.version !== "1.1") { + this.data.version = "1.1"; + migrated = true; + } + for (const rules of Object.values(this.data.schemas)) { + for (const rule of rules) { + if (rule.status === undefined) { + rule.status = "active"; + migrated = true; + } + } + } this.filePath = resolvedPath; - this.modified = false; + this.modified = migrated; } /** @@ -382,7 +473,7 @@ export class GrammarStore { return this._compiledCache; } - const allRules = this.getAllRules(); + const allRules = this.getAllActiveRules(); if (allRules.length === 0) { return undefined; @@ -413,7 +504,7 @@ export class GrammarStore { * Export grammars for a specific schema as a single .agr file */ public exportSchemaGrammar(schemaName: string): string { - const rules = this.getRulesForSchema(schemaName); + const rules = this.getActiveRulesForSchema(schemaName); if (rules.length === 0) { return ""; @@ -445,6 +536,32 @@ export class GrammarStore { } } +function getInvalidationReason( + rule: StoredGrammarRule, + identity: GrammarSchemaIdentity, +): string | undefined { + if (rule.actionBinding !== undefined) { + if (rule.actionBinding.sourceId !== identity.sourceId) { + return "action source changed"; + } + const fingerprint = + rule.actionName === undefined + ? undefined + : identity.actionFingerprints?.[rule.actionName]; + if (fingerprint !== rule.actionBinding.actionFingerprint) { + return "action definition changed"; + } + return undefined; + } + if ( + rule.schemaHash !== undefined && + rule.schemaHash !== identity.schemaHash + ) { + return "action schema changed"; + } + return undefined; +} + /** * Get the path to the grammar store directory for a session */ diff --git a/ts/packages/actionGrammar/src/index.ts b/ts/packages/actionGrammar/src/index.ts index df6180cfd1..b9cc3919d1 100644 --- a/ts/packages/actionGrammar/src/index.ts +++ b/ts/packages/actionGrammar/src/index.ts @@ -232,6 +232,10 @@ export type { StoredGrammarRule, GrammarStoreData, GrammarStoreInfo, + GrammarRuleStatus, + GrammarRuleActionBinding, + GrammarSchemaIdentity, + GrammarRuleReconciliation, } from "./grammarStore.js"; export { GrammarStore, diff --git a/ts/packages/actionGrammar/test/grammarStore.spec.ts b/ts/packages/actionGrammar/test/grammarStore.spec.ts index c34156a35a..28344c18cf 100644 --- a/ts/packages/actionGrammar/test/grammarStore.spec.ts +++ b/ts/packages/actionGrammar/test/grammarStore.spec.ts @@ -283,11 +283,14 @@ describe("GrammarStore", () => { version: "1.0", schemas: store["data"].schemas, }; + delete oldFormat.schemas.player[0].status; fs.mkdirSync(path.dirname(testFile), { recursive: true }); fs.writeFileSync(testFile, JSON.stringify(oldFormat, null, 2)); const store2 = new GrammarStore(); await store2.load(testFile); + expect(store2.isModified()).toBe(true); + expect(store2.getRulesForSchema("player")[0].status).toBe("active"); const grammar = store2.compileToGrammar(); expect(grammar).toBeDefined(); @@ -298,6 +301,89 @@ describe("GrammarStore", () => { }); }); + describe("Rule identity", () => { + it("suspends stale rules and reactivates them when identity matches", async () => { + const store = new GrammarStore(); + await store.addRule({ + grammarText: ' = search -> { actionName: "search" };', + schemaName: "tools", + actionName: "search", + schemaHash: "schema-v1", + actionBinding: { + sourceId: "provider-1", + actionFingerprint: "tool-v1", + }, + }); + + const suspended = await store.reconcileSchema("tools", { + schemaHash: "schema-v2", + sourceId: "provider-1", + actionFingerprints: { search: "tool-v2" }, + }); + + expect(suspended.suspended).toBe(1); + expect(store.getActiveRulesForSchema("tools")).toHaveLength(0); + expect(store.getRulesForSchema("tools")[0]).toMatchObject({ + status: "suspended", + invalidationReason: "action definition changed", + }); + expect(store.compileToGrammar()).toBeUndefined(); + + const activated = await store.reconcileSchema("tools", { + schemaHash: "schema-v1", + sourceId: "provider-1", + actionFingerprints: { search: "tool-v1" }, + }); + + expect(activated.activated).toBe(1); + expect(store.getActiveRulesForSchema("tools")).toHaveLength(1); + expect( + store.getRulesForSchema("tools")[0].invalidationReason, + ).toBeUndefined(); + }); + + it("keeps a bound action active when only another action changes", async () => { + const store = new GrammarStore(); + await store.addRule({ + grammarText: ' = search -> { actionName: "search" };', + schemaName: "tools", + actionName: "search", + schemaHash: "catalog-v1", + actionBinding: { + sourceId: "provider-1", + actionFingerprint: "search-v1", + }, + }); + + const result = await store.reconcileSchema("tools", { + schemaHash: "catalog-v2", + sourceId: "provider-1", + actionFingerprints: { + search: "search-v1", + changedAction: "changed-v2", + }, + }); + + expect(result.unchanged).toBe(1); + expect(store.getActiveRulesForSchema("tools")).toHaveLength(1); + }); + + it("keeps legacy rules active when identity metadata is absent", async () => { + const store = new GrammarStore(); + await store.addRule({ + grammarText: ' = pause -> { actionName: "pause" };', + schemaName: "player", + }); + + const result = await store.reconcileSchema("player", { + schemaHash: "current", + }); + + expect(result.unchanged).toBe(1); + expect(store.getActiveRulesForSchema("player")).toHaveLength(1); + }); + }); + describe("Session Helpers", () => { it("should provide session directory paths", () => { const sessionDir = "/home/user/.typeagent/sessions/2026-01-25-001"; diff --git a/ts/packages/actionSchema/src/jsonSchemaParser.ts b/ts/packages/actionSchema/src/jsonSchemaParser.ts index 396979829b..9870268964 100644 --- a/ts/packages/actionSchema/src/jsonSchemaParser.ts +++ b/ts/packages/actionSchema/src/jsonSchemaParser.ts @@ -1,93 +1,236 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { - JsonSchema, - JsonSchemaArray, - JsonSchemaBoolean, - JsonSchemaNumber, - JsonSchemaObject, - JsonSchemaReference, - JsonSchemaString, - JsonSchemaUnion, -} from "./jsonSchemaTypes.js"; +import { JsonSchemaObject } from "./jsonSchemaTypes.js"; import * as sc from "./creator.js"; -import { SchemaObjectField, SchemaType, SchemaTypeReference } from "./type.js"; +import { + SchemaObjectField, + SchemaType, + SchemaTypeObject, + SchemaTypeReference, +} from "./type.js"; import { createParsedActionSchema } from "./parser.js"; -function parseJsonSchemaObject(schema: JsonSchemaObject) { +type JsonSchemaRecord = Record; + +type ParseContext = { + root: JsonSchemaRecord; + references: Set; +}; + +function schemaError(path: string, message: string): never { + throw new Error(`${path}: ${message}`); +} + +function parseJsonSchemaObject( + schema: JsonSchemaRecord, + context: ParseContext, + path: string, +) { + const properties = schema.properties; + if ( + schema.additionalProperties !== undefined && + schema.additionalProperties !== false && + (!isObject(properties) || Object.keys(properties).length === 0) + ) { + return sc.any(); + } const fields: Record = {}; - for (const [key, value] of Object.entries(schema.properties)) { - const type = parseJsonSchema(value); - fields[key] = schema.required?.includes(key) - ? sc.field(type, value.description) - : sc.optional(type, value.description); + if (properties !== undefined && !isObject(properties)) { + schemaError(`${path}.properties`, "must be an object"); + } + const required = readRequired(schema.required, properties ?? {}, path); + for (const [key, value] of Object.entries(properties ?? {})) { + const type = parseJsonSchema(value, context, `${path}.${key}`); + const description = readDescription(value, `${path}.${key}`); + fields[key] = required.has(key) + ? sc.field(type, description) + : sc.optional(type, description); } return sc.obj(fields); } -function parseJsonSchemaArray(schema: JsonSchemaArray) { - const type = parseJsonSchema(schema.items); - return sc.array(type); -} - -function parseJsonSchemaString(schema: JsonSchemaString) { - return sc.string(...(schema.enum ?? [])); -} -function parseJsonSchemaNumber(schema: JsonSchemaNumber) { - return sc.number(); -} -function parseJsonSchemaBoolean(schema: JsonSchemaBoolean) { - return sc.boolean(); +function readRequired( + value: unknown, + properties: JsonSchemaRecord, + path: string, +): Set { + if (value === undefined) return new Set(); + if (!Array.isArray(value) || !value.every(isString)) { + schemaError(`${path}.required`, "must be an array of strings"); + } + const keys = new Set(Object.keys(properties)); + for (const required of value) { + if (!keys.has(required)) { + schemaError( + `${path}.required`, + `references missing property '${required}'`, + ); + } + } + return new Set(value); } -function parseJsonSchemaUnion(schema: JsonSchemaUnion) { - const types = schema.anyOf.map(parseJsonSchema); - return sc.union(types); -} -function parseJsonSchemaReference( - schema: JsonSchemaReference, -): SchemaTypeReference { - // TODO: resolve? - // return sc.ref(schema.$ref); - throw new Error("Not implemented"); +function readDescription(schema: unknown, path: string): string | undefined { + if (!isObject(schema) || schema.description === undefined) return undefined; + if (!isString(schema.description)) { + schemaError(`${path}.description`, "must be a string"); + } + return schema.description; } -function isJsonSchemaUnion(schema: JsonSchema): schema is JsonSchemaUnion { - return (schema as JsonSchemaUnion).anyOf !== undefined; +function resolveReference( + reference: string, + context: ParseContext, + path: string, +): SchemaType { + if (!reference.startsWith("#/")) { + schemaError(path, `external reference '${reference}' is not supported`); + } + if (context.references.has(reference)) { + schemaError(path, `cyclic reference '${reference}' is not supported`); + } + let target: unknown = context.root; + for (const encodedSegment of reference.slice(2).split("/")) { + const segment = encodedSegment + .replaceAll("~1", "/") + .replaceAll("~0", "~"); + if ( + !isObject(target) || + !Object.prototype.hasOwnProperty.call(target, segment) + ) { + schemaError(path, `reference '${reference}' cannot be resolved`); + } + target = target[segment]; + } + context.references.add(reference); + try { + return parseJsonSchema(target, context, reference); + } finally { + context.references.delete(reference); + } } -function isJsonSchemaReference( - schema: JsonSchema, -): schema is JsonSchemaReference { - return (schema as JsonSchemaReference).$ref !== undefined; +function parseComposition( + schemas: unknown, + context: ParseContext, + path: string, +): SchemaType[] { + if (!Array.isArray(schemas) || schemas.length === 0) { + schemaError(path, "must be a non-empty array"); + } + return schemas.map((schema, index) => + parseJsonSchema(schema, context, `${path}[${index}]`), + ); } -function parseJsonSchema(schema: JsonSchema): SchemaType { - if (isJsonSchemaUnion(schema)) { - return parseJsonSchemaUnion(schema); +function mergeObjectComposition(parts: SchemaType[]): SchemaType { + if ( + !parts.every((part): part is SchemaTypeObject => part.type === "object") + ) { + return sc.any(); } - if (isJsonSchemaReference(schema)) { - return parseJsonSchemaReference(schema); + const fields: Record = {}; + for (const part of parts) { + for (const [name, field] of Object.entries(part.fields)) { + if (fields[name] !== undefined) { + return sc.any(); + } + fields[name] = field; + } } + return sc.obj(fields); +} - switch (schema.type) { +function parseTypeName( + type: string, + schema: JsonSchemaRecord, + context: ParseContext, + path: string, +): SchemaType { + switch (type) { case "object": - return parseJsonSchemaObject(schema); + return parseJsonSchemaObject(schema, context, path); case "array": - return parseJsonSchemaArray(schema); - case "string": - return parseJsonSchemaString(schema); + return sc.array( + schema.items === undefined + ? sc.any() + : parseJsonSchema(schema.items, context, `${path}.items`), + ); + case "string": { + if (schema.enum === undefined) return sc.string(); + if (!Array.isArray(schema.enum) || !schema.enum.every(isString)) { + schemaError( + `${path}.enum`, + "string enum values must be strings", + ); + } + return sc.string(...schema.enum); + } case "number": case "integer": - return parseJsonSchemaNumber(schema); + return sc.number(); case "boolean": - return parseJsonSchemaBoolean(schema); + return sc.boolean(); case "null": - throw new Error("Null type is not supported"); + return sc.undefined_(); + default: + schemaError(`${path}.type`, `unsupported type '${type}'`); } +} - throw new Error(`Invalid schema type: ${JSON.stringify(schema)}`); +function parseJsonSchema( + schema: unknown, + context: ParseContext, + path: string, +): SchemaType { + if (schema === true) return sc.any(); + if (schema === false) { + schemaError(path, "the false schema cannot be translated"); + } + if (!isObject(schema)) { + schemaError(path, "must be an object or boolean schema"); + } + readDescription(schema, path); + if (isString(schema.$ref)) { + return resolveReference(schema.$ref, context, path); + } + if (schema.anyOf !== undefined || schema.oneOf !== undefined) { + return sc.union( + parseComposition( + schema.anyOf ?? schema.oneOf, + context, + `${path}.${schema.anyOf !== undefined ? "anyOf" : "oneOf"}`, + ), + ); + } + if (schema.allOf !== undefined) { + const parts = parseComposition(schema.allOf, context, `${path}.allOf`); + return parts.length === 1 ? parts[0] : mergeObjectComposition(parts); + } + if (Array.isArray(schema.type)) { + if (schema.type.length === 0 || !schema.type.every(isString)) { + schemaError(`${path}.type`, "must contain schema type names"); + } + return sc.union( + schema.type.map((type) => + parseTypeName(type, schema, context, path), + ), + ); + } + if (isString(schema.type)) { + return parseTypeName(schema.type, schema, context, path); + } + if (schema.properties !== undefined) { + return parseJsonSchemaObject(schema, context, path); + } + if (isString(schema.const)) { + return sc.string(schema.const); + } + if (Array.isArray(schema.enum) && schema.enum.every(isString)) { + return sc.string(...schema.enum); + } + return sc.any(); } type ToolsJsonSchema = { @@ -129,10 +272,22 @@ export function parseToolsJsonSchema( } const actionName = tool.name; - const inputSchema = tool.inputSchema; + const inputSchema = tool.inputSchema as JsonSchemaRecord; + const context: ParseContext = { + root: inputSchema, + references: new Set(), + }; + const parsedParameters = parseJsonSchemaObject( + inputSchema, + context, + `${actionName}.inputSchema`, + ); const type = sc.obj({ actionName: sc.string(actionName), - parameters: parseJsonSchemaObject(inputSchema), + parameters: + parsedParameters.type === "object" + ? parsedParameters + : sc.obj({}), }); const def = sc.type(nameTransform(tool.name), type, tool.description); refs.push(sc.ref(def)); @@ -157,96 +312,26 @@ function validateToolsJsonSchema(schema: unknown): schema is ToolsJsonSchema { ); } const inputSchema = tool.inputSchema; - if (!validateJsonSchemaObject(inputSchema)) { - throw new Error(`Invalid tool input schema ${actionName}`); - } - - // REVIEW: extra properties are ignored? - return true; -} -function validateJsonSchemaObject(schema: unknown): schema is JsonSchema { - if (!isObject(schema) || schema.type !== "object") { - return false; - } - return validateJsonSchemaObjectFields(schema); -} - -function validateJsonSchemaObjectFields( - schema: Record, -): schema is JsonSchemaObject { - if (schema.properties === undefined) { - return schema.required === undefined; - } - if (!isObject(schema.properties)) { - return false; - } - if (schema.required !== undefined) { - if (!Array.isArray(schema.required)) { - return false; - } - const keys = Object.keys(schema.properties); - for (const required of schema.required) { - if (!isString(required)) { - return false; - } - if (!keys.includes(required)) { - return false; - } - } + if (!isObject(inputSchema)) { + throw new Error( + `Invalid tool input schema ${actionName}: root must be an object`, + ); } - - for (const value of Object.values(schema.properties)) { - if (!validateJsonSchema(value)) { - return false; - } + if ( + inputSchema.type !== undefined && + inputSchema.type !== "object" && + !( + Array.isArray(inputSchema.type) && + inputSchema.type.includes("object") + ) + ) { + throw new Error( + `Invalid tool input schema ${actionName}: root type must be object`, + ); } return true; } -function validateJsonSchema(schema: unknown): schema is JsonSchema { - if (!isObject(schema)) { - return false; - } - if (schema.description !== undefined && !isString(schema.description)) { - return false; - } - - switch (schema.type) { - case "object": - return validateJsonSchemaObjectFields(schema); - case "array": - return ( - (schema.description === undefined || - isString(schema.description)) && - (schema.items === undefined || validateJsonSchema(schema.items)) - ); - case "string": - return schema.enum === undefined || isStringArray(schema.enum); - case "number": - case "integer": - case "boolean": - case "null": - return true; - case undefined: - if (schema.anyOf !== undefined) { - // JsonSchemaUnion - if (!Array.isArray(schema.anyOf)) { - return false; - } - for (const value of schema.anyOf) { - if (!validateJsonSchema(value)) { - return false; - } - } - return true; - } - // JsonSchemaReference - return isString(schema.$ref); - default: - return false; - } -} - function isObject(value: unknown): value is Record { return typeof value === "object" && value !== null; } @@ -254,7 +339,3 @@ function isObject(value: unknown): value is Record { function isString(value: unknown): value is string { return typeof value === "string"; } - -function isStringArray(value: unknown): value is string[] { - return Array.isArray(value) && value.every(isString); -} diff --git a/ts/packages/actionSchema/src/jsonSchemaTypes.ts b/ts/packages/actionSchema/src/jsonSchemaTypes.ts index eeb0362741..4a0113b6b0 100644 --- a/ts/packages/actionSchema/src/jsonSchemaTypes.ts +++ b/ts/packages/actionSchema/src/jsonSchemaTypes.ts @@ -8,6 +8,7 @@ export type JsonSchema = | JsonSchemaString | JsonSchemaNumber | JsonSchemaBoolean + | JsonSchemaMultiType | JsonSchemaTrue | JsonSchemaFalse | JsonSchemaNull @@ -17,19 +18,43 @@ export type JsonSchema = export type JsonSchemaAny = { type?: undefined; description?: string; + anyOf?: JsonSchema[]; + oneOf?: JsonSchema[]; + allOf?: JsonSchema[]; + $ref?: string; + $defs?: Record; +}; + +export type JsonSchemaTypeName = + | "object" + | "array" + | "string" + | "number" + | "integer" + | "boolean" + | "null"; + +export type JsonSchemaMultiType = { + type: JsonSchemaTypeName[]; + description?: string; + properties?: Record; + required?: string[]; + additionalProperties?: boolean | JsonSchema; + items?: JsonSchema; + enum?: string[]; }; export type JsonSchemaObject = { type: "object"; description?: string; - properties: Record; + properties?: Record; required?: string[]; - additionalProperties: false; + additionalProperties?: boolean | JsonSchema; }; export type JsonSchemaArray = { type: "array"; description?: string; - items: JsonSchema; + items?: JsonSchema; }; export type JsonSchemaString = { @@ -54,7 +79,9 @@ export type JsonSchemaNull = { }; export type JsonSchemaUnion = { - anyOf: JsonSchema[]; + anyOf?: JsonSchema[]; + oneOf?: JsonSchema[]; + allOf?: JsonSchema[]; description?: string; }; diff --git a/ts/packages/actionSchema/test/jsonSchemaParser.spec.ts b/ts/packages/actionSchema/test/jsonSchemaParser.spec.ts new file mode 100644 index 0000000000..b434a3286a --- /dev/null +++ b/ts/packages/actionSchema/test/jsonSchemaParser.spec.ts @@ -0,0 +1,179 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { parseToolsJsonSchema } from "../src/jsonSchemaParser.js"; + +function getParameters(tool: { + name: string; + inputSchema: Record; +}) { + const parsed = parseToolsJsonSchema([tool]); + const action = parsed.actionSchemas.get(tool.name); + if (action === undefined || action.type.type !== "object") { + throw new Error("Expected parsed action object"); + } + const parameters = action.type.fields.parameters?.type; + if (parameters?.type !== "object") { + throw new Error("Expected parsed parameters object"); + } + return parameters; +} + +describe("tool JSON Schema parsing", () => { + it("supports nullable type arrays and oneOf", () => { + const parameters = getParameters({ + name: "search", + inputSchema: { + type: "object", + properties: { + query: { type: ["string", "null"] }, + limit: { + oneOf: [{ type: "integer" }, { type: "null" }], + }, + }, + }, + }); + + expect(parameters.fields.query.type).toMatchObject({ + type: "type-union", + types: [{ type: "string" }, { type: "undefined" }], + }); + expect(parameters.fields.limit.type).toMatchObject({ + type: "type-union", + types: [{ type: "number" }, { type: "undefined" }], + }); + }); + + it("supports unconstrained values, maps, and arrays", () => { + const parameters = getParameters({ + name: "invoke", + inputSchema: { + type: "object", + required: ["arguments"], + properties: { + arguments: { + type: "object", + additionalProperties: true, + }, + metadata: {}, + values: { type: "array" }, + }, + }, + }); + + expect(parameters.fields.arguments.type).toEqual({ type: "any" }); + expect(parameters.fields.metadata.type).toEqual({ type: "any" }); + expect(parameters.fields.values.type).toEqual({ + type: "array", + elementType: { type: "any" }, + }); + }); + + it("supports a fully open parameters object", () => { + const parsed = parseToolsJsonSchema([ + { + name: "invoke", + inputSchema: { + type: "object", + additionalProperties: true, + }, + }, + ]); + + const action = parsed.actionSchemas.get("invoke"); + expect(action?.type.type).toBe("object"); + expect( + action?.type.type === "object" + ? action.type.fields.parameters?.type + : undefined, + ).toEqual({ type: "object", fields: {} }); + }); + + it("resolves local references and rejects unresolved references precisely", () => { + const parameters = getParameters({ + name: "lookup", + inputSchema: { + type: "object", + properties: { + filter: { $ref: "#/$defs/filter" }, + }, + $defs: { + filter: { + type: "object", + required: ["name"], + properties: { name: { type: "string" } }, + }, + }, + }, + }); + + expect(parameters.fields.filter.type).toMatchObject({ + type: "object", + fields: { + name: { type: { type: "string" } }, + }, + }); + expect(() => + getParameters({ + name: "broken", + inputSchema: { + type: "object", + properties: { + value: { $ref: "#/$defs/missing" }, + }, + }, + }), + ).toThrow("reference '#/$defs/missing' cannot be resolved"); + }); + + it("merges non-overlapping object allOf branches", () => { + const parameters = getParameters({ + name: "create", + inputSchema: { + type: "object", + properties: { + item: { + allOf: [ + { + type: "object", + required: ["name"], + properties: { name: { type: "string" } }, + }, + { + type: "object", + properties: { count: { type: "integer" } }, + }, + ], + }, + }, + }, + }); + + expect(parameters.fields.item.type).toMatchObject({ + type: "object", + fields: { + name: { type: { type: "string" } }, + count: { type: { type: "number" }, optional: true }, + }, + }); + }); + + it("reports the unsupported schema path", () => { + expect(() => + getParameters({ + name: "broken", + inputSchema: { + type: "object", + properties: { + filters: { + type: "array", + items: { type: "unsupported" }, + }, + }, + }, + }), + ).toThrow( + "broken.inputSchema.filters.items.type: unsupported type 'unsupported'", + ); + }); +}); diff --git a/ts/packages/agentSdk/src/agentInterface.ts b/ts/packages/agentSdk/src/agentInterface.ts index 2d66d00c78..c7921ce8a6 100644 --- a/ts/packages/agentSdk/src/agentInterface.ts +++ b/ts/packages/agentSdk/src/agentInterface.ts @@ -83,6 +83,13 @@ export type ActionPolicy = { confirmation?: "required"; }; +export type ActionCacheBinding = { + // Stable identity of the system that supplied this dynamic schema. + sourceId: string; + // Fingerprints of the source definitions keyed by action name. + actionFingerprints: Record; +}; + export type SchemaManifest = { description: string; schemaType: string | SchemaTypeNames; // string if there are only action schemas @@ -94,6 +101,9 @@ export type SchemaManifest = { streamingActions?: string[]; // Exact action names. Applies to structured invocation, not NL routing. actionPolicies?: Record; + // Optional provenance used to invalidate learned routes when a dynamic + // action's source definition changes. + cacheBinding?: ActionCacheBinding; }; export type ActionManifest = { diff --git a/ts/packages/agentSdk/src/index.ts b/ts/packages/agentSdk/src/index.ts index 76f637f991..9078078b7a 100644 --- a/ts/packages/agentSdk/src/index.ts +++ b/ts/packages/agentSdk/src/index.ts @@ -9,6 +9,7 @@ export { SchemaContent, SchemaFormat, SchemaManifest, + ActionCacheBinding, ActionEffect, ActionPolicy, AppAgent, diff --git a/ts/packages/cache/src/cache/cache.ts b/ts/packages/cache/src/cache/cache.ts index fdaf46d33b..960c19446f 100644 --- a/ts/packages/cache/src/cache/cache.ts +++ b/ts/packages/cache/src/cache/cache.ts @@ -401,13 +401,28 @@ export class AgentCache { `Grammar rule generated for ${schemaName}.${actionName}: ${genResult.generatedRule}`, ); - // Add rule to persisted grammar store - await this._persistedGrammarStore.addRule({ - schemaName, - grammarText: genResult.generatedRule, - actionName, - sourceRequest: requestAction.request, - }); + const schemaHash = + this.schemaInfoProvider?.getActionSchemaFileHash( + schemaName, + ); + const actionBinding = + this.schemaInfoProvider?.getActionCacheBinding?.( + schemaName, + actionName, + ); + const storedRule = + await this._persistedGrammarStore.addRule({ + schemaName, + grammarText: genResult.generatedRule, + actionName, + sourceRequest: requestAction.request, + ...(schemaHash === undefined + ? {} + : { schemaHash }), + ...(actionBinding === undefined + ? {} + : { actionBinding }), + }); // Add rule to agent grammar registry (in-memory) const agentGrammar = @@ -436,6 +451,9 @@ export class AgentCache { genResult.generatedRule, }; } else { + await this._persistedGrammarStore.deleteRuleById( + storedRule.id, + ); debug( `Failed to add rule to registry: ${addResult.errors.join(", ")}`, ); @@ -449,6 +467,9 @@ export class AgentCache { }; } } else { + await this._persistedGrammarStore.deleteRuleById( + storedRule.id, + ); debug( `Agent grammar not found for ${schemaName}`, ); diff --git a/ts/packages/cache/src/explanation/schemaInfoProvider.ts b/ts/packages/cache/src/explanation/schemaInfoProvider.ts index df65ba7207..d5e20004f8 100644 --- a/ts/packages/cache/src/explanation/schemaInfoProvider.ts +++ b/ts/packages/cache/src/explanation/schemaInfoProvider.ts @@ -119,4 +119,14 @@ export type SchemaInfoProvider = { // Throws if schemaName not found getActionSchemaFileHash: (schemaName: string) => string; + + getActionCacheBinding?: ( + schemaName: string, + actionName: string, + ) => + | { + sourceId: string; + actionFingerprint: string; + } + | undefined; }; diff --git a/ts/packages/defaultAgentProvider/src/mcp/mcpServerProvider.ts b/ts/packages/defaultAgentProvider/src/mcp/mcpServerProvider.ts index 42fd1b33af..bf0b5b8450 100644 --- a/ts/packages/defaultAgentProvider/src/mcp/mcpServerProvider.ts +++ b/ts/packages/defaultAgentProvider/src/mcp/mcpServerProvider.ts @@ -138,6 +138,17 @@ function createSnapshot( description: config.description ?? name, schemaType: entryTypeName, schemaFile, + cacheBinding: Object.freeze({ + sourceId: config.id, + actionFingerprints: Object.freeze( + Object.fromEntries( + [...catalog.entries.values()].map((entry) => [ + entry.name, + entry.fingerprint, + ]), + ), + ), + }), }), }); return Object.freeze({ catalog, manifest }); diff --git a/ts/packages/defaultAgentProvider/src/mcp/mcpToolCatalog.ts b/ts/packages/defaultAgentProvider/src/mcp/mcpToolCatalog.ts index 9c017b7578..0e0d247e53 100644 --- a/ts/packages/defaultAgentProvider/src/mcp/mcpToolCatalog.ts +++ b/ts/packages/defaultAgentProvider/src/mcp/mcpToolCatalog.ts @@ -4,6 +4,7 @@ import type { Tool } from "@modelcontextprotocol/client"; import type { JsonSchemaValidator } from "@modelcontextprotocol/client"; import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv"; +import { createHash } from "node:crypto"; import { convertToolsSchema, type SkippedTool } from "./mcpSchema.js"; const maxSchemaBytes = 256 * 1024; @@ -30,6 +31,7 @@ export interface McpToolCatalogEntry { readonly annotations?: Tool["annotations"]; readonly inputSchema: Tool["inputSchema"]; readonly outputSchema?: Tool["outputSchema"]; + readonly fingerprint: string; readonly validateArguments: JsonSchemaValidator>; readonly validateOutput?: JsonSchemaValidator; } @@ -45,6 +47,7 @@ function canonicalize(value: unknown): unknown { if (Array.isArray(value)) { return value.map(canonicalize); } + if (value !== null && typeof value === "object") { return Object.fromEntries( Object.entries(value as Record) @@ -55,6 +58,12 @@ function canonicalize(value: unknown): unknown { return value; } +function fingerprint(value: unknown): string { + return createHash("sha256") + .update(JSON.stringify(canonicalize(value))) + .digest("hex"); +} + function inspectSchema(schema: unknown, label: string): void { let serialized: string; try { @@ -218,6 +227,11 @@ export function buildMcpToolCatalog( ...(tool.outputSchema === undefined ? {} : { outputSchema: tool.outputSchema }), + fingerprint: fingerprint({ + inputSchema: tool.inputSchema, + outputSchema: tool.outputSchema, + annotations: tool.annotations, + }), validateArguments: compiled.input, ...(compiled.output === undefined ? {} @@ -225,24 +239,22 @@ export function buildMcpToolCatalog( }), ); } - const fingerprint = JSON.stringify( - canonicalize( - [...entries.values()].map((entry) => ({ - id: entry.id, - name: entry.name, - description: entry.description, - title: entry.title, - icons: entry.icons, - annotations: entry.annotations, - inputSchema: entry.inputSchema, - outputSchema: entry.outputSchema, - })), - ), + const catalogFingerprint = fingerprint( + [...entries.values()].map((entry) => ({ + id: entry.id, + name: entry.name, + description: entry.description, + title: entry.title, + icons: entry.icons, + annotations: entry.annotations, + inputSchema: entry.inputSchema, + outputSchema: entry.outputSchema, + })), ); return Object.freeze({ entries, schemaContent: converted.content, skipped: Object.freeze(skipped), - fingerprint, + fingerprint: catalogFingerprint, }); } diff --git a/ts/packages/defaultAgentProvider/test/mcpServerProvider.spec.ts b/ts/packages/defaultAgentProvider/test/mcpServerProvider.spec.ts index fda37b9818..f37617b7a3 100644 --- a/ts/packages/defaultAgentProvider/test/mcpServerProvider.spec.ts +++ b/ts/packages/defaultAgentProvider/test/mcpServerProvider.spec.ts @@ -287,6 +287,12 @@ describe("MCP server provider milestone 5", () => { const initialManifest = await provider.getAppAgentManifest("server"); expect(Object.isFrozen(initialManifest)).toBe(true); expect(schemaContent(initialManifest)).toContain("first"); + expect(initialManifest.schema?.cacheBinding).toMatchObject({ + sourceId: "server-id", + actionFingerprints: { + first: expect.any(String), + }, + }); notify!(null, [tool("second")]); notify!(null, [tool("third")]); diff --git a/ts/packages/defaultAgentProvider/test/mcpToolCatalog.spec.ts b/ts/packages/defaultAgentProvider/test/mcpToolCatalog.spec.ts index d66a79d1b6..779847b775 100644 --- a/ts/packages/defaultAgentProvider/test/mcpToolCatalog.spec.ts +++ b/ts/packages/defaultAgentProvider/test/mcpToolCatalog.spec.ts @@ -52,6 +52,58 @@ describe("MCP tool catalog safety", () => { expect(reverse.schemaContent).toBe(forward.schemaContent); }); + it("fingerprints each accepted tool definition", () => { + const first = buildMcpToolCatalog( + "server", + [tool("search", { type: "object", properties: {} })], + "Actions", + ); + const second = buildMcpToolCatalog( + "server", + [ + tool("search", { + type: "object", + properties: { query: { type: "string" } }, + }), + ], + "Actions", + ); + + expect( + first.entries.get(getMcpToolIdentity("server", "search")) + ?.fingerprint, + ).not.toBe( + second.entries.get(getMcpToolIdentity("server", "search")) + ?.fingerprint, + ); + }); + + it("accepts common open and nullable input schema forms", () => { + const catalog = buildMcpToolCatalog( + "server", + [ + tool("invoke", { + type: "object", + required: ["arguments"], + properties: { + arguments: { + type: "object", + additionalProperties: true, + }, + query: { type: ["string", "null"] }, + values: { type: "array" }, + }, + } as Tool["inputSchema"]), + ], + "Actions", + ); + + expect( + catalog.entries.has(getMcpToolIdentity("server", "invoke")), + ).toBe(true); + expect(catalog.skipped).toHaveLength(0); + }); + it("skips external refs and excessive complexity per tool", () => { const deep: Record = { type: "object" }; let cursor = deep; diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts index f473941ce5..0d46600c5f 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts @@ -2035,6 +2035,46 @@ export class AppAgentManager } } + const persistedGrammarStore = context.persistedGrammarStore; + if (persistedGrammarStore !== undefined) { + for (const schemaName of record.schemas) { + const config = this.actionConfigs.get(schemaName); + if (config === undefined) continue; + const schemaHash = + this.actionSchemaFileCache.getActionSchemaFile( + config, + ).sourceHash; + await persistedGrammarStore.reconcileSchema(schemaName, { + schemaHash, + ...(config.cacheBinding === undefined + ? {} + : { + sourceId: config.cacheBinding.sourceId, + actionFingerprints: + config.cacheBinding.actionFingerprints, + }), + }); + + const agentGrammar = + context.agentGrammarRegistry.getAgent(schemaName); + if (agentGrammar === undefined) continue; + agentGrammar.resetToBase(); + for (const rule of persistedGrammarStore.getActiveRulesForSchema( + schemaName, + )) { + const result = agentGrammar.addGeneratedRules( + rule.grammarText, + ); + if (!result.success) { + debugError( + `Failed to reload learned rule ${rule.id} for ${schemaName}: ${result.errors.join("; ")}`, + ); + } + } + context.agentCache.syncAgentGrammar(schemaName); + } + } + // Clear translator cache to force re-translation with new schema context.translatorCache.clear(); // Drop cached derived keyword vectors for this agent's schemas so the diff --git a/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts b/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts index d7a95abea7..8f2d42ce6b 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts @@ -137,10 +137,6 @@ import { CosmosClient, PartitionKeyBuilder } from "@azure/cosmos"; import { CosmosPartitionKeyBuilder } from "@typeagent/telemetry"; import { DefaultAzureCredential } from "@azure/identity"; import { DisplayLog } from "../displayLog.js"; -import { - fromJSONParsedActionSchema, - ParsedActionSchemaJSON, -} from "@typeagent/action-schema"; import { RequestQueue } from "../queue/requestQueue.js"; import type { QueueExecutionContext } from "../queue/requestQueue.js"; import { createSnapshotCoalescer } from "../queue/snapshotCoalescer.js"; @@ -1776,8 +1772,28 @@ async function setupGrammarGeneration(context: CommandHandlerContext) { await grammarStore.load(grammarStorePath); debug(`Loaded grammar store from ${grammarStorePath}`); + for (const schemaName of grammarStore.getSchemaNames()) { + const actionConfig = + context.agents.tryGetActionConfig(schemaName); + if (actionConfig === undefined) continue; + const schemaHash = + context.agents.getActionSchemaFileForConfig( + actionConfig, + ).sourceHash; + await grammarStore.reconcileSchema(schemaName, { + schemaHash, + ...(actionConfig.cacheBinding === undefined + ? {} + : { + sourceId: actionConfig.cacheBinding.sourceId, + actionFingerprints: + actionConfig.cacheBinding.actionFingerprints, + }), + }); + } + // Merge persisted dynamic rules into agent grammars - const allRules = grammarStore.getAllRules(); + const allRules = grammarStore.getAllActiveRules(); const schemaRules = new Map(); // Group rules by schema @@ -1836,63 +1852,20 @@ async function setupGrammarGeneration(context: CommandHandlerContext) { // Enable auto-save await grammarStore.setAutoSave(config.cache.autoSave); - // Import getPackageFilePath for resolving schema paths - const { getPackageFilePath } = await import( - "../utils/getPackageFilePath.js" - ); - // Configure agent cache with grammar generation support context.agentCache.configureGrammarGeneration( context.agentGrammarRegistry, grammarStore, true, (schemaName: string) => { - // Get compiled schema file path (.pas.json) from action config for grammar generation const actionConfig = context.agents.tryGetActionConfig(schemaName); if (!actionConfig) { throw new Error( `Action config not found for schema: ${schemaName}`, ); } - - let schemaPath: string | undefined; - - // Use schemaFilePath directly if it's already a .pas.json file - if ( - actionConfig.schemaFilePath && - actionConfig.schemaFilePath.endsWith(".pas.json") - ) { - schemaPath = getPackageFilePath(actionConfig.schemaFilePath); - } else if ( - actionConfig.schemaFilePath && - actionConfig.schemaFilePath.endsWith(".ts") - ) { - // Fallback: try to derive .pas.json path from .ts schemaFilePath - // Try common pattern: ./src/schema.ts -> ../dist/schema.pas.json - const derivedPath = actionConfig.schemaFilePath - .replace(/^\.\/src\//, "../dist/") - .replace(/\.ts$/, ".pas.json"); - debug( - `Attempting fallback .pas.json path for ${schemaName}: ${derivedPath}`, - ); - try { - schemaPath = getPackageFilePath(derivedPath); - } catch { - // Fallback path doesn't exist, continue to error - } - } - - if (!schemaPath) { - throw new Error( - `Compiled schema file path (.pas.json) not found for schema: ${schemaName}. ` + - `Please ensure the schema is compiled to a .pas.json file.`, - ); - } - - const content = fs.readFileSync(schemaPath, "utf-8"); - return fromJSONParsedActionSchema( - JSON.parse(content) as ParsedActionSchemaJSON, - ); + return context.agents.getActionSchemaFileForConfig(actionConfig) + .parsedActionSchema; }, ); diff --git a/ts/packages/dispatcher/dispatcher/src/context/dispatcher/handlers/requestCommandHandler.ts b/ts/packages/dispatcher/dispatcher/src/context/dispatcher/handlers/requestCommandHandler.ts index 413a62f5f3..f5e7aebe9f 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/dispatcher/handlers/requestCommandHandler.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/dispatcher/handlers/requestCommandHandler.ts @@ -473,7 +473,7 @@ function findMatchedGrammarRule( const store = context.persistedGrammarStore; const candidates = store - ?.getRulesForSchema(primary.schemaName) + ?.getActiveRulesForSchema(primary.schemaName) .filter((rule) => rule.actionName === primary.actionName) ?? []; if (candidates.length === 1) { return { rule: candidates[0].grammarText }; diff --git a/ts/packages/dispatcher/dispatcher/src/context/system/action/grammarActionHandler.ts b/ts/packages/dispatcher/dispatcher/src/context/system/action/grammarActionHandler.ts index c048354e51..3c7678b2fb 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/system/action/grammarActionHandler.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/system/action/grammarActionHandler.ts @@ -483,11 +483,16 @@ export function renderRulesTable( `#${rule.id}` + `
${highlightGrammarText(rule.grammarText)}
` + ``; + const status = + rule.status === "suspended" + ? `Suspended` + : "Active"; rows += ` ${risks} ${idCell} ${badge} + ${status} ${anchor} ${timeAgo(rule.timestamp)} `; @@ -499,7 +504,7 @@ export function renderRulesTable(
- ${agentCol} + ${agentCol}${rows}
Risks#AnchorDateRisks#StatusAnchorDate
@@ -538,6 +543,10 @@ export function renderRuleDetail(rule: StoredGrammarRule): string { const sourceRow = rule.sourceRequest ? `
Learned from: "${escapeHtml(rule.sourceRequest)}"
` : ""; + const statusRow = + rule.status === "suspended" + ? `
Status: Suspended — ${escapeHtml(rule.invalidationReason ?? "stale rule")}
` + : `
Status: Active
`; const ts = new Date(rule.timestamp).toLocaleString(); @@ -550,7 +559,7 @@ export function renderRuleDetail(rule: StoredGrammarRule): string { ${escapeHtml(rule.actionName ?? "")} ${ts}
- ${munchRow}${compRow}${anchorInfo} + ${statusRow}${munchRow}${compRow}${anchorInfo}
${highlighted}
${sourceRow} diff --git a/ts/packages/dispatcher/dispatcher/src/translation/actionSchemaFileCache.ts b/ts/packages/dispatcher/dispatcher/src/translation/actionSchemaFileCache.ts index be8d19bff4..f8fbc21dd7 100644 --- a/ts/packages/dispatcher/dispatcher/src/translation/actionSchemaFileCache.ts +++ b/ts/packages/dispatcher/dispatcher/src/translation/actionSchemaFileCache.ts @@ -341,6 +341,16 @@ export function createSchemaInfoProvider( const result: SchemaInfoProvider = { getActionSchemaFileHash: (schemaName) => getActionSchemaFile(schemaName).sourceHash, + getActionCacheBinding: (schemaName, actionName) => { + const binding = provider.getActionConfig(schemaName).cacheBinding; + const actionFingerprint = binding?.actionFingerprints[actionName]; + return binding === undefined || actionFingerprint === undefined + ? undefined + : { + sourceId: binding.sourceId, + actionFingerprint, + }; + }, getActionNamespace: (schemaName) => getActionSchemaFile(schemaName).parsedActionSchema.actionNamespace, getActionCacheEnabled: (schemaName, actionName) => From 4e8cf22b967c247f8482a5187428d66c0020fc35 Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:14:32 -0700 Subject: [PATCH 2/3] =?UTF-8?q?Add=20a=20Copilot=20SDK=20grammar=20generat?= =?UTF-8?q?or=20for=20TypeAgent=E2=80=99s=20learned=20action=20routes.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use GPT-5.6 Sol to generate grammar from confirmed request/action pairs, removing the runtime dependency on a separate Claude CLI login. Extract the shared grammar generation and parsing behavior into a provider-independent base class while retaining the Claude generator for existing CLI and API callers. Make Copilot the default generator for runtime cache population, add reliable session/client cleanup, and support injected clients for isolated tests. Document the new runtime generation strategy and add focused coverage for model selection, response handling, and resource cleanup. Validate the implementation end to end with a read-only MCP tool: the first request generates and persists a schema-aware, fingerprinted grammar rule, while repeated requests—including after a server restart—route through the NFA grammar cache with zero translation-model tokens and continue through the normal MCP validation, policy, audit, and execution path. --- ts/docs/architecture/core/actionGrammar.md | 13 +- ts/packages/actionGrammar/package.json | 1 + .../src/generation/copilotGrammarGenerator.ts | 73 +++++++++ .../src/generation/grammarGenerator.ts | 96 ++++++------ .../actionGrammar/src/generation/index.ts | 30 +++- .../test/copilotGrammarGenerator.spec.ts | 146 ++++++++++++++++++ ts/pnpm-lock.yaml | 3 + 7 files changed, 305 insertions(+), 57 deletions(-) create mode 100644 ts/packages/actionGrammar/src/generation/copilotGrammarGenerator.ts create mode 100644 ts/packages/actionGrammar/test/copilotGrammarGenerator.spec.ts diff --git a/ts/docs/architecture/core/actionGrammar.md b/ts/docs/architecture/core/actionGrammar.md index 84cb783ab3..b4047398b9 100644 --- a/ts/docs/architecture/core/actionGrammar.md +++ b/ts/docs/architecture/core/actionGrammar.md @@ -234,14 +234,19 @@ agents at once. The `generation/` subsystem uses LLMs to automatically create grammar rules from action schemas or confirmed user interactions. -**Three generation strategies:** +**Four generation strategies:** -1. **`ClaudeGrammarGenerator`** — Analyzes individual request/action pairs. +1. **`CopilotGrammarGenerator`** — The runtime default for confirmed + request/action pairs. It uses the Copilot SDK with `gpt-5.6-sol` to + extract linguistic patterns, parameter mappings, and alternative + phrasings. Produces `GrammarAnalysis` with rule patterns. + +2. **`ClaudeGrammarGenerator`** — Retained for existing CLI and API callers. Given a natural language request and its confirmed action, Claude extracts linguistic patterns, parameter mappings, and alternative phrasings. Produces `GrammarAnalysis` with rule patterns. -2. **`SchemaToGrammarGenerator`** — Batch generation from action schemas. +3. **`SchemaToGrammarGenerator`** — Batch generation from action schemas. Reads `.pas.json` (Parameter Action Schema) files — JSON representations of an agent's TypeScript action types, containing action names, parameter types, and descriptions extracted from the @@ -249,7 +254,7 @@ rules from action schemas or confirmed user interactions. produces example natural language requests for each action and synthesizes complete `.agr` grammar text with test cases. -3. **`ScenarioBasedGrammarGenerator`** — Uses pre-defined scenario templates +4. **`ScenarioBasedGrammarGenerator`** — Uses pre-defined scenario templates (music player, calendar, lists) to generate grammar rules for common action patterns without LLM calls. diff --git a/ts/packages/actionGrammar/package.json b/ts/packages/actionGrammar/package.json index 525380e85c..9f8cabc623 100644 --- a/ts/packages/actionGrammar/package.json +++ b/ts/packages/actionGrammar/package.json @@ -46,6 +46,7 @@ }, "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.3.150", + "@github/copilot-sdk": "1.0.13", "@typeagent/action-schema": "workspace:*", "@typeagent/common-utils": "workspace:*", "@typeagent/config": "workspace:*", diff --git a/ts/packages/actionGrammar/src/generation/copilotGrammarGenerator.ts b/ts/packages/actionGrammar/src/generation/copilotGrammarGenerator.ts new file mode 100644 index 0000000000..be1f1a5402 --- /dev/null +++ b/ts/packages/actionGrammar/src/generation/copilotGrammarGenerator.ts @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { + AssistantMessageEvent, + MessageOptions, + SessionConfig, +} from "@github/copilot-sdk"; +import registerDebug from "debug"; +import { GrammarGenerator } from "./grammarGenerator.js"; + +const debug = registerDebug("typeagent:actionGrammar:copilotGrammarGenerator"); + +export const defaultCopilotGrammarModel = "gpt-5.6-sol"; + +export interface CopilotGrammarSession { + sendAndWait( + promptOrOptions: string | MessageOptions, + timeout?: number, + ): Promise; + disconnect(): Promise; +} + +export interface CopilotGrammarClient { + createSession(config: SessionConfig): Promise; + stop(): Promise; +} + +export type CopilotGrammarClientFactory = () => Promise; + +async function createCopilotGrammarClient(): Promise { + const { CopilotClient } = await import("@github/copilot-sdk"); + const client = new CopilotClient(); + await client.start(); + return client; +} + +export class CopilotGrammarGenerator extends GrammarGenerator { + constructor( + private readonly model: string = defaultCopilotGrammarModel, + private readonly clientFactory: CopilotGrammarClientFactory = createCopilotGrammarClient, + ) { + super("Copilot"); + } + + protected async queryModel(fullPrompt: string): Promise { + const client = await this.clientFactory(); + let session: CopilotGrammarSession | undefined; + try { + session = await client.createSession({ + model: this.model, + streaming: false, + }); + const response = await session.sendAndWait({ + prompt: fullPrompt, + }); + return response?.data?.content ?? ""; + } finally { + if (session !== undefined) { + try { + await session.disconnect(); + } catch (error) { + debug("Failed to disconnect Copilot session: %O", error); + } + } + try { + await client.stop(); + } catch (error) { + debug("Failed to stop Copilot client: %O", error); + } + } + } +} diff --git a/ts/packages/actionGrammar/src/generation/grammarGenerator.ts b/ts/packages/actionGrammar/src/generation/grammarGenerator.ts index ebda160ce3..ec76a7f0d5 100644 --- a/ts/packages/actionGrammar/src/generation/grammarGenerator.ts +++ b/ts/packages/actionGrammar/src/generation/grammarGenerator.ts @@ -73,7 +73,7 @@ export interface Conversion { } /** - * A helper rule defined by Claude for this specific grammar (e.g., rule-specific filler) + * A helper rule defined by the model for this specific grammar (e.g., rule-specific filler) */ export interface AdditionalRule { /** Rule name without angle brackets, e.g. "ExtraneousPhrase" */ @@ -100,7 +100,7 @@ export interface GrammarAnalysis { grammarPattern: RuleRHS; // Reasoning about the choices made reasoning: string; - // Optional: extra rules Claude defined for this grammar (e.g., rule-specific filler) + // Optional: extra rules the model defined for this grammar (e.g., rule-specific filler) additionalRules?: AdditionalRule[]; // Optional: new phrases to add to global phrase-set matchers (idempotent) phrasesToAdd?: Array<{ matcherName: string; phrase: string }>; @@ -285,12 +285,10 @@ Output: }`; } -export class ClaudeGrammarGenerator { - private model: string; +export abstract class GrammarGenerator { + protected constructor(private readonly providerName: string) {} - constructor(model: string = "claude-sonnet-4-20250514") { - this.model = model; - } + protected abstract queryModel(fullPrompt: string): Promise; async generateGrammar( testCase: GrammarTestCase, @@ -306,7 +304,7 @@ export class ClaudeGrammarGenerator { /** * Refine a previously generated grammar rule that failed to match the original request. - * Gives Claude specific feedback: the failed rule, the tokenized request, and hints. + * Gives the model specific feedback: the failed rule, the tokenized request, and hints. */ async refineGrammar( testCase: GrammarTestCase, @@ -350,36 +348,9 @@ Generate a corrected rule now.`; } private async queryAndParse(fullPrompt: string): Promise { - // Use the Agent SDK query function - const queryInstance = query({ - prompt: fullPrompt, - options: { - model: this.model, - ...claudeExecutableOption(), - }, - }); - - // Collect the result from the SDK - let responseText = ""; - for await (const message of queryInstance) { - if (message.type === "result") { - if (message.subtype === "success") { - responseText = message.result || ""; - break; - } else { - const errors = - "errors" in message - ? (message as any).errors - : undefined; - throw new Error( - `Claude query failed: ${errors?.join(", ") || "Unknown error"}`, - ); - } - } - } - + const responseText = await this.queryModel(fullPrompt); if (!responseText) { - throw new Error("No response from Claude"); + throw new Error(`No response from ${this.providerName}`); } return this.parseAnalysis(responseText); @@ -467,7 +438,7 @@ Generate a corrected rule now.`; const jsonStart = text.indexOf("{"); if (jsonStart === -1) { throw new Error( - `No JSON object found in Claude response. Response starts with: "${text.substring(0, 100)}..."`, + `No JSON object found in ${this.providerName} response. Response starts with: "${text.substring(0, 100)}..."`, ); } @@ -476,7 +447,7 @@ Generate a corrected rule now.`; const preamble = text.substring(0, jsonStart).trim(); if (preamble.length > 0) { debug( - `Claude included text before JSON: "${preamble.substring(0, 100)}..."`, + `${this.providerName} included text before JSON: "${preamble.substring(0, 100)}..."`, ); } } @@ -500,7 +471,7 @@ Generate a corrected rule now.`; if (!jsonText) { throw new Error( - `Found opening brace but no matching closing brace in Claude response. Text from brace: "${text.substring(jsonStart, jsonStart + 100)}..."`, + `Found opening brace but no matching closing brace in ${this.providerName} response. Text from brace: "${text.substring(jsonStart, jsonStart + 100)}..."`, ); } @@ -509,7 +480,7 @@ Generate a corrected rule now.`; analysis = JSON.parse(jsonText); } catch (error) { throw new Error( - `Failed to parse JSON from Claude response: ${error instanceof Error ? error.message : String(error)}\nJSON text preview: ${jsonText.substring(0, 300)}...\nFull response preview: ${text.substring(0, 300)}...`, + `Failed to parse JSON from ${this.providerName} response: ${error instanceof Error ? error.message : String(error)}\nJSON text preview: ${jsonText.substring(0, 300)}...\nFull response preview: ${text.substring(0, 300)}...`, ); } @@ -531,7 +502,7 @@ Generate a corrected rule now.`; throw new Error("Rejected cases must include rejectionReason"); } - // Clean up any unwanted text that Claude might have inserted into string fields + // Clean up any unwanted text that the model inserted into string fields this.sanitizeAnalysisStrings(analysis); return analysis; @@ -539,7 +510,7 @@ Generate a corrected rule now.`; /** * Remove copyright notices, comments, and other unwanted text from analysis string fields - * Claude sometimes inserts these into the JSON, making the grammar patterns invalid + * Models sometimes insert these into the JSON, making the grammar patterns invalid */ private sanitizeAnalysisStrings(analysis: GrammarAnalysis): void { const commentPatterns = [ @@ -564,7 +535,7 @@ Generate a corrected rule now.`; if (hadComments) { debug( - `Removed comment/copyright text from Claude response. Original: "${str.substring(0, 100)}..."`, + `Removed comment/copyright text from model response. Original: "${str.substring(0, 100)}..."`, ); } @@ -678,7 +649,7 @@ Generate a corrected rule now.`; } } - // Replace types in the matchPattern — normalize any type Claude used to the correct one + // Replace types in the matchPattern with the schema's required type let matchPattern = analysis.grammarPattern.matchPattern; for (const [varName, wildcardType] of wildcardTypes) { // Match $(varName:AnyType) and replace with $(varName:CorrectType) @@ -695,7 +666,7 @@ Generate a corrected rule now.`; // Only rule-specific helper rules (additionalRules) need to be prepended. const preambleRules: string[] = []; - // Inject any rule-specific helper rules Claude defined + // Inject any rule-specific helper rules the model defined if (analysis.additionalRules) { for (const rule of analysis.additionalRules) { preambleRules.push(rule.ruleText); @@ -743,3 +714,36 @@ Generate a corrected rule now.`; return paramName; } } + +export class ClaudeGrammarGenerator extends GrammarGenerator { + constructor(private readonly model: string = "claude-sonnet-4-20250514") { + super("Claude"); + } + + protected async queryModel(fullPrompt: string): Promise { + const queryInstance = query({ + prompt: fullPrompt, + options: { + model: this.model, + ...claudeExecutableOption(), + }, + }); + + for await (const message of queryInstance) { + if (message.type !== "result") { + continue; + } + if (message.subtype === "success") { + return message.result || ""; + } + const errors = + "errors" in message && Array.isArray(message.errors) + ? message.errors + : undefined; + throw new Error( + `Claude query failed: ${errors?.join(", ") || "Unknown error"}`, + ); + } + return ""; + } +} diff --git a/ts/packages/actionGrammar/src/generation/index.ts b/ts/packages/actionGrammar/src/generation/index.ts index 6c5c7bab86..7e2eaa38de 100644 --- a/ts/packages/actionGrammar/src/generation/index.ts +++ b/ts/packages/actionGrammar/src/generation/index.ts @@ -11,6 +11,7 @@ import { Cardinal } from "../builtInEntities.js"; export { resolveCliOnPath, claudeExecutableOption } from "./cliPath.js"; export { + GrammarGenerator, ClaudeGrammarGenerator, GrammarAnalysis, ParameterMapping, @@ -20,6 +21,14 @@ export { Token, } from "./grammarGenerator.js"; +export { + CopilotGrammarGenerator, + CopilotGrammarClient, + CopilotGrammarClientFactory, + CopilotGrammarSession, + defaultCopilotGrammarModel, +} from "./copilotGrammarGenerator.js"; + export { SchemaToGrammarGenerator, SchemaGrammarConfig, @@ -68,7 +77,12 @@ export { IterationMetrics, } from "./grammarWarmer.js"; -import { ClaudeGrammarGenerator, GrammarAnalysis } from "./grammarGenerator.js"; +import { + ClaudeGrammarGenerator, + GrammarAnalysis, + GrammarGenerator, +} from "./grammarGenerator.js"; +import { CopilotGrammarGenerator } from "./copilotGrammarGenerator.js"; import { getSchemaInfoFromParsedSchema, loadSchemaInfo, @@ -273,17 +287,21 @@ async function retryOnConnectionError( /** * Generate and add a grammar rule to the cache from a request/action pair - * This is called by agentServer when Claude confirms a user action should be cached + * This is called by agentServer when a user action should be cached. * * @param request The cache population request - * @param model The Claude model to use for analysis (default: claude-sonnet-4-20250514) + * @param generatorOrModel A generator, or a Claude model name for compatibility * @returns Result indicating success or failure with details */ export async function populateCache( request: CachePopulationRequest, - model: string = "claude-sonnet-4-20250514", + generatorOrModel: GrammarGenerator | string = new CopilotGrammarGenerator(), ): Promise { try { + const generator = + typeof generatorOrModel === "string" + ? new ClaudeGrammarGenerator(generatorOrModel) + : generatorOrModel; // Load schema information if (!request.parsedSchema && !request.schemaPath) { throw new Error( @@ -335,8 +353,6 @@ export async function populateCache( action: request.action, }; - // Generate grammar rule using Claude - const generator = new ClaudeGrammarGenerator(model); const analysis = await retryOnConnectionError(() => generator.generateGrammar(testCase, schemaInfo), ); @@ -364,7 +380,7 @@ export async function populateCache( ); // Round-trip verification: compile and test the rule against the original request. - // If it fails, give Claude feedback and retry up to MAX_REFINEMENT_ATTEMPTS times. + // If it fails, give the model feedback and retry up to MAX_REFINEMENT_ATTEMPTS times. const MAX_REFINEMENT_ATTEMPTS = 2; const requestTokens = tokenizeRequest(request.request); let refinedAnalysis = analysis; diff --git a/ts/packages/actionGrammar/test/copilotGrammarGenerator.spec.ts b/ts/packages/actionGrammar/test/copilotGrammarGenerator.spec.ts new file mode 100644 index 0000000000..a3c6064067 --- /dev/null +++ b/ts/packages/actionGrammar/test/copilotGrammarGenerator.spec.ts @@ -0,0 +1,146 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { + AssistantMessageEvent, + MessageOptions, + SessionConfig, +} from "@github/copilot-sdk"; +import { + CopilotGrammarClient, + CopilotGrammarGenerator, + CopilotGrammarSession, + defaultCopilotGrammarModel, +} from "../src/generation/copilotGrammarGenerator.js"; +import { SchemaInfo } from "../src/generation/schemaReader.js"; + +const schemaInfo: SchemaInfo = { + schemaName: "test", + actions: new Map([ + [ + "listCategories", + { + actionName: "listCategories", + parameters: new Map(), + }, + ], + ]), + entityTypes: new Set(), + converters: new Map(), +}; + +const analysis = { + shouldGenerateGrammar: true, + requestAnalysis: { + sentences: [ + { + text: "list categories", + parse: "(verb list) (noun categories)", + tokens: [], + }, + ], + }, + parameterMappings: [], + fixedPhrases: ["list", "categories"], + grammarPattern: { + matchPattern: "list categories", + actionParameters: [], + }, + reasoning: "The request contains only fixed action words.", +}; + +class TestSession implements CopilotGrammarSession { + public disconnected = false; + public prompt: string | undefined; + + constructor(private readonly content: string | undefined) {} + + async sendAndWait( + promptOrOptions: string | MessageOptions, + ): Promise { + this.prompt = + typeof promptOrOptions === "string" + ? promptOrOptions + : promptOrOptions.prompt; + return this.content === undefined + ? undefined + : ({ + data: { content: this.content }, + } as AssistantMessageEvent); + } + + async disconnect(): Promise { + this.disconnected = true; + } +} + +class TestClient implements CopilotGrammarClient { + public config: SessionConfig | undefined; + public stopped = false; + + constructor(public readonly session: TestSession) {} + + async createSession(config: SessionConfig): Promise { + this.config = config; + return this.session; + } + + async stop(): Promise { + this.stopped = true; + } +} + +describe("CopilotGrammarGenerator", () => { + it("uses GPT-5.6 Sol and cleans up the Copilot session", async () => { + const session = new TestSession(JSON.stringify(analysis)); + const client = new TestClient(session); + const generator = new CopilotGrammarGenerator( + undefined, + async () => client, + ); + + const result = await generator.generateGrammar( + { + request: "list categories", + schemaName: "test", + action: { + actionName: "listCategories", + parameters: {}, + }, + }, + schemaInfo, + ); + + expect(result).toEqual(analysis); + expect(client.config?.model).toBe(defaultCopilotGrammarModel); + expect(client.config?.streaming).toBe(false); + expect(session.prompt).toContain("grammar pattern generator"); + expect(session.disconnected).toBe(true); + expect(client.stopped).toBe(true); + }); + + it("reports an empty Copilot response and still cleans up", async () => { + const session = new TestSession(undefined); + const client = new TestClient(session); + const generator = new CopilotGrammarGenerator( + undefined, + async () => client, + ); + + await expect( + generator.generateGrammar( + { + request: "list categories", + schemaName: "test", + action: { + actionName: "listCategories", + parameters: {}, + }, + }, + schemaInfo, + ), + ).rejects.toThrow("No response from Copilot"); + expect(session.disconnected).toBe(true); + expect(client.stopped).toBe(true); + }); +}); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 698b79e60e..740e24bf70 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -1299,6 +1299,9 @@ importers: '@anthropic-ai/claude-agent-sdk': specifier: ^0.3.150 version: 0.3.162(@anthropic-ai/sdk@0.93.0(zod@4.4.3))(@modelcontextprotocol/sdk@1.26.0(zod@4.4.3))(zod@4.4.3) + '@github/copilot-sdk': + specifier: 1.0.13 + version: 1.0.13 '@typeagent/action-schema': specifier: workspace:* version: link:../actionSchema From f28ba6d50376e91b51c4706aaaf1906e9ffabb08 Mon Sep 17 00:00:00 2001 From: Hillary Mutisya <150286414+hillary-mutisya@users.noreply.github.com> Date: Sat, 26 Sep 2026 14:12:24 -0700 Subject: [PATCH 3/3] Fix complexity ratchet --- .../actionGrammar/src/generation/index.ts | 103 +++-- ts/packages/cache/src/cache/cache.ts | 397 +++++++++--------- 2 files changed, 255 insertions(+), 245 deletions(-) diff --git a/ts/packages/actionGrammar/src/generation/index.ts b/ts/packages/actionGrammar/src/generation/index.ts index 7e2eaa38de..9d6530b010 100644 --- a/ts/packages/actionGrammar/src/generation/index.ts +++ b/ts/packages/actionGrammar/src/generation/index.ts @@ -86,6 +86,7 @@ import { CopilotGrammarGenerator } from "./copilotGrammarGenerator.js"; import { getSchemaInfoFromParsedSchema, loadSchemaInfo, + SchemaInfo, } from "./schemaReader.js"; import { GrammarTestCase } from "./testTypes.js"; import { loadGrammarRulesNoThrow } from "../grammarLoader.js"; @@ -247,6 +248,54 @@ export interface CachePopulationResult { appliedPhrasesToAdd?: Array<{ matcherName: string; phrase: string }>; } +function validateAndStripInferredParameters( + request: CachePopulationRequest, + schemaInfo: SchemaInfo, +): string | undefined { + const normalizedRequest = request.request + .toLowerCase() + .replace(/[^\w\s]/g, " "); + const actionInfo = schemaInfo.actions.get(request.action.actionName); + const strippedParams: string[] = []; + for (const [paramName, paramValue] of Object.entries( + request.action.parameters, + )) { + if (isValueInRequest(paramValue, normalizedRequest)) { + continue; + } + if (actionInfo?.parameters.get(paramName)?.optional) { + strippedParams.push(paramName); + continue; + } + return `Required parameter '${paramName}' value "${paramValue}" not found in request (possible LLM correction - don't cache)`; + } + for (const paramName of strippedParams) { + delete request.action.parameters[paramName]; + } + return undefined; +} + +function getCheckedVariables( + testCase: GrammarTestCase, + schemaInfo: SchemaInfo, +): Set { + const checkedVariables = new Set(); + const actionInfo = schemaInfo.actions.get(testCase.action.actionName); + if (!actionInfo) { + return checkedVariables; + } + for (const [paramName, paramInfo] of actionInfo.parameters) { + if (paramInfo.paramSpec !== "checked_wildcard") { + continue; + } + const varName = Array.isArray(testCase.action.parameters[paramName]) + ? getSingularVariableName(paramName) + : paramName; + checkedVariables.add(varName); + } + return checkedVariables; +} + /** * Retry an async operation on transient connection errors with exponential backoff. * Only retries on errors whose message contains "Connection error" or "ECONNRESET" @@ -315,35 +364,15 @@ export async function populateCache( ) : loadSchemaInfo(request.schemaPath!); - // Validate that parameter values appear in the request. - // If a value was inferred by the LLM (not in the request), strip it - // from the action if it's optional in the schema; reject if required. - const normalizedRequest = request.request - .toLowerCase() - .replace(/[^\w\s]/g, " "); - const actionInfo = schemaInfo.actions.get(request.action.actionName); - const strippedParams: string[] = []; - for (const [paramName, paramValue] of Object.entries( - request.action.parameters, - )) { - const isInRequest = isValueInRequest(paramValue, normalizedRequest); - if (!isInRequest) { - const paramInfo = actionInfo?.parameters.get(paramName); - if (paramInfo?.optional) { - // Optional parameter inferred by LLM — strip it - strippedParams.push(paramName); - } else { - // Required parameter not in request — reject - return { - success: false, - rejectionReason: `Required parameter '${paramName}' value "${paramValue}" not found in request (possible LLM correction - don't cache)`, - }; - } - } - } - // Remove inferred optional parameters from the action - for (const paramName of strippedParams) { - delete request.action.parameters[paramName]; + const parameterRejection = validateAndStripInferredParameters( + request, + schemaInfo, + ); + if (parameterRejection) { + return { + success: false, + rejectionReason: parameterRejection, + }; } // Create test case from request @@ -432,21 +461,7 @@ export async function populateCache( ); } - // Extract checked variables from the action parameters - const checkedVariables = new Set(); - if (actionInfo) { - for (const [paramName, paramInfo] of actionInfo.parameters) { - if (paramInfo.paramSpec === "checked_wildcard") { - // Handle array parameters (convert plural to singular) - const varName = Array.isArray( - testCase.action.parameters[paramName], - ) - ? getSingularVariableName(paramName) - : paramName; - checkedVariables.add(varName); - } - } - } + const checkedVariables = getCheckedVariables(testCase, schemaInfo); const result: CachePopulationResult = { success: true, diff --git a/ts/packages/cache/src/cache/cache.ts b/ts/packages/cache/src/cache/cache.ts index 960c19446f..13eb04316d 100644 --- a/ts/packages/cache/src/cache/cache.ts +++ b/ts/packages/cache/src/cache/cache.ts @@ -62,6 +62,10 @@ export type ProcessRequestActionResult = { }; }; +type GrammarGenerationResult = NonNullable< + ProcessRequestActionResult["grammarResult"] +>; + export type CacheConfig = { mergeMatchSets: boolean; cacheConflicts: boolean; @@ -81,6 +85,18 @@ function getFailedResult(message: string): ProcessRequestActionResult { }; } +function getCachingDisabledMessage( + actions: RequestAction["actions"], + schemaInfoProvider?: SchemaInfoProvider, +): string | undefined { + for (const action of actions) { + if (!doCacheAction(action, schemaInfoProvider)) { + return `Caching disabled in schema config for action '${getFullActionName(action)}'`; + } + } + return undefined; +} + export class AgentCache { private _constructionStore: ConstructionStoreImpl; private _grammarStore: GrammarStoreImpl; @@ -205,6 +221,176 @@ export class AgentCache { return this._constructionStore.prune(this.namespaceKeyFilter); } + private async addGeneratedGrammarRule( + requestAction: RequestAction, + schemaName: string, + actionName: string, + generatedRule: string, + checkedVariables?: Set, + ): Promise { + const schemaHash = + this.schemaInfoProvider?.getActionSchemaFileHash(schemaName); + const actionBinding = this.schemaInfoProvider?.getActionCacheBinding?.( + schemaName, + actionName, + ); + const storedRule = await this._persistedGrammarStore.addRule({ + schemaName, + grammarText: generatedRule, + actionName, + sourceRequest: requestAction.request, + ...(schemaHash === undefined ? {} : { schemaHash }), + ...(actionBinding === undefined ? {} : { actionBinding }), + }); + + const agentGrammar = this._agentGrammarRegistry.getAgent(schemaName); + if (!agentGrammar) { + await this._persistedGrammarStore.deleteRuleById(storedRule.id); + debug(`Agent grammar not found for ${schemaName}`); + return { + success: false, + message: `Agent grammar not found for ${schemaName}`, + generatedRule, + }; + } + + debug(`Adding rule to agent grammar registry...`); + const addResult = agentGrammar.addGeneratedRules( + generatedRule, + checkedVariables, + ); + if (!addResult.success) { + await this._persistedGrammarStore.deleteRuleById(storedRule.id); + debug( + `Failed to add rule to registry: ${addResult.errors.join(", ")}`, + ); + return { + success: false, + message: `Failed to add rule to agent registry: ${addResult.errors.join(", ")}`, + generatedRule, + }; + } + + this.syncAgentGrammar(schemaName); + debug(`Grammar rule added for ${schemaName}.${actionName}`); + return { + success: true, + message: `Grammar rule added for ${schemaName}.${actionName}`, + generatedRule, + }; + } + + private async generateGrammarForAction( + requestAction: RequestAction, + schemaName: string, + actionName: string, + parameters: object, + ): Promise { + try { + debug(`Calling getParsedActionSchema("${schemaName}")...`); + const parsedSchema = this._getParsedActionSchema!(schemaName); + debug(`Parsed schema loaded for ${schemaName}`); + + debug(`Importing populateCache...`); + const { populateCache } = await import( + "@typeagent/action-grammar/generation" + ); + debug(`populateCache imported successfully`); + + debug( + `Calling populateCache for request: "${requestAction.request}"`, + ); + const genResult = await populateCache({ + request: requestAction.request, + schemaName, + action: { actionName, parameters }, + parsedSchema, + }); + const grammarResult = + genResult.success && genResult.generatedRule + ? await this.addGeneratedGrammarRule( + requestAction, + schemaName, + actionName, + genResult.generatedRule, + genResult.checkedVariables, + ) + : { + success: false, + message: + genResult.rejectionReason || + "Grammar generation failed", + ...(genResult.generatedRule && { + generatedRule: genResult.generatedRule, + }), + }; + + if (!grammarResult.success) { + debug(`Grammar generation rejected: ${grammarResult.message}`); + } + this.logger?.logEvent("grammarGeneration", { + request: requestAction.request, + schemaName, + actionName, + success: grammarResult.success, + message: grammarResult.message, + }); + return grammarResult; + } catch (genError) { + debug(`Error during generation: %O`, genError); + return { + success: false, + message: `Generation error: ${genError instanceof Error ? genError.message : String(genError)}`, + }; + } + } + + private async generateGrammar( + requestAction: RequestAction, + ): Promise { + try { + const execAction = requestAction.actions[0]; + const schemaName = execAction.action.schemaName; + const actionName = execAction.action.actionName; + const parameters = execAction.action.parameters ?? {}; + + debug(`Grammar gen starting for ${schemaName}.${actionName}`); + debug( + `_getParsedActionSchema is ${this._getParsedActionSchema ? "configured" : "NOT configured"}`, + ); + if (!this._getParsedActionSchema) { + debug(`Parsed action schema getter not configured`); + return { + success: false, + message: "Parsed action schema getter not configured", + }; + } + return await this.generateGrammarForAction( + requestAction, + schemaName, + actionName, + parameters, + ); + } catch (error) { + const message = + error instanceof Error ? error.message : String(error); + debug(`Unexpected error: %O`, error); + this.logger?.logEvent( + "grammarGeneration", + { + request: requestAction.request, + success: false, + error: message, + }, + "error", + ); + return { + success: false, + message: `Grammar generation error: ${message}`, + }; + } + } + public async processRequestAction( requestAction: RequestAction, cache: boolean = true, @@ -217,19 +403,14 @@ export class AgentCache { `processRequestAction: "${requestAction.request}" for actions: ${executableActions.map((a) => `${a.action.schemaName}.${a.action.actionName}`).join(", ")}`, ); - if (cache) { - for (const action of executableActions) { - const cacheAction = doCacheAction( - action, - this.schemaInfoProvider, - ); - - if (!cacheAction) { - return getFailedResult( - `Caching disabled in schema config for action '${getFullActionName(action)}'`, - ); - } - } + const cachingDisabledMessage = cache + ? getCachingDisabledMessage( + executableActions, + this.schemaInfoProvider, + ) + : undefined; + if (cachingDisabledMessage) { + return getFailedResult(cachingDisabledMessage); } const namespaceKeys = this.getNamespaceKeys( @@ -335,200 +516,14 @@ export class AgentCache { constructionResult = { added, message }; } - // Generate grammar rules if using NFA system and explanation succeeded - let grammarResult: - | { success: boolean; message: string; generatedRule?: string } - | undefined = undefined; + let grammarResult: GrammarGenerationResult | undefined; if ( cache && this._useNFAGrammar && explanation.success && executableActions.length === 1 ) { - try { - const execAction = executableActions[0]; - const schemaName = execAction.action.schemaName; - const actionName = execAction.action.actionName; - const parameters = execAction.action.parameters ?? {}; - - debug( - `Grammar gen starting for ${schemaName}.${actionName}`, - ); - debug( - `_getParsedActionSchema is ${this._getParsedActionSchema ? "configured" : "NOT configured"}`, - ); - - // Check if we have the required components - if (!this._getParsedActionSchema) { - debug(`Parsed action schema getter not configured`); - grammarResult = { - success: false, - message: - "Parsed action schema getter not configured", - }; - } else { - try { - // Get parsed action schema - debug( - `Calling getParsedActionSchema("${schemaName}")...`, - ); - const parsedSchema = - this._getParsedActionSchema(schemaName); - debug(`Parsed schema loaded for ${schemaName}`); - - // Import populateCache dynamically to avoid circular dependencies - debug(`Importing populateCache...`); - const { populateCache } = await import( - "@typeagent/action-grammar/generation" - ); - debug(`populateCache imported successfully`); - - debug( - `Calling populateCache for request: "${requestAction.request}"`, - ); - // Generate grammar rule - const genResult = await populateCache({ - request: requestAction.request, - schemaName, - action: { - actionName, - parameters, - }, - parsedSchema, - }); - if (genResult.success && genResult.generatedRule) { - debug( - `Grammar rule generated for ${schemaName}.${actionName}: ${genResult.generatedRule}`, - ); - - const schemaHash = - this.schemaInfoProvider?.getActionSchemaFileHash( - schemaName, - ); - const actionBinding = - this.schemaInfoProvider?.getActionCacheBinding?.( - schemaName, - actionName, - ); - const storedRule = - await this._persistedGrammarStore.addRule({ - schemaName, - grammarText: genResult.generatedRule, - actionName, - sourceRequest: requestAction.request, - ...(schemaHash === undefined - ? {} - : { schemaHash }), - ...(actionBinding === undefined - ? {} - : { actionBinding }), - }); - - // Add rule to agent grammar registry (in-memory) - const agentGrammar = - this._agentGrammarRegistry.getAgent( - schemaName, - ); - if (agentGrammar) { - debug( - `Adding rule to agent grammar registry...`, - ); - const addResult = - agentGrammar.addGeneratedRules( - genResult.generatedRule, - genResult.checkedVariables, - ); - if (addResult.success) { - // Sync to the grammar store used for matching - this.syncAgentGrammar(schemaName); - debug( - `Grammar rule added for ${schemaName}.${actionName}`, - ); - grammarResult = { - success: true, - message: `Grammar rule added for ${schemaName}.${actionName}`, - generatedRule: - genResult.generatedRule, - }; - } else { - await this._persistedGrammarStore.deleteRuleById( - storedRule.id, - ); - debug( - `Failed to add rule to registry: ${addResult.errors.join(", ")}`, - ); - grammarResult = { - success: false, - message: `Failed to add rule to agent registry: ${addResult.errors.join(", ")}`, - ...(genResult.generatedRule && { - generatedRule: - genResult.generatedRule, - }), - }; - } - } else { - await this._persistedGrammarStore.deleteRuleById( - storedRule.id, - ); - debug( - `Agent grammar not found for ${schemaName}`, - ); - grammarResult = { - success: false, - message: `Agent grammar not found for ${schemaName}`, - ...(genResult.generatedRule && { - generatedRule: - genResult.generatedRule, - }), - }; - } - } else { - debug( - `Grammar generation rejected: ${genResult.rejectionReason || "unknown reason"}`, - ); - grammarResult = { - success: false, - message: - genResult.rejectionReason || - "Grammar generation failed", - ...(genResult.generatedRule && { - generatedRule: genResult.generatedRule, - }), - }; - } - - this.logger?.logEvent("grammarGeneration", { - request: requestAction.request, - schemaName, - actionName, - success: grammarResult?.success, - message: grammarResult?.message, - }); - } catch (genError) { - debug(`Error during generation: %O`, genError); - grammarResult = { - success: false, - message: `Generation error: ${genError instanceof Error ? genError.message : String(genError)}`, - }; - } - } - } catch (error: any) { - debug(`Unexpected error: %O`, error); - grammarResult = { - success: false, - message: `Grammar generation error: ${error.message}`, - }; - - this.logger?.logEvent( - "grammarGeneration", - { - request: requestAction.request, - success: false, - error: error.message, - }, - "error", - ); - } + grammarResult = await this.generateGrammar(requestAction); } if (grammarResult && !grammarResult.success) {