diff --git a/src/agent/runtime/tool-exposure.test.ts b/src/agent/runtime/tool-exposure.test.ts index de1a81e323..1d7765152e 100644 --- a/src/agent/runtime/tool-exposure.test.ts +++ b/src/agent/runtime/tool-exposure.test.ts @@ -210,20 +210,23 @@ it("tool search normalizes ASCII case and underscores as spaces", () => { ); }); -it("tool search falls back to deterministic whitespace terms after a phrase miss", () => { +it("tool search scores fallback terms by selectivity after a phrase miss", () => { const fileCatalog = [ definition("create_file", "Create a project file"), definition("update_file", "Update a project file"), definition("sandbox_write_file", "Write only inside the sandbox filesystem"), ]; + // `file` matches every candidate and therefore says nothing about which one the + // caller meant; `create` and `update` each match exactly one. `sandbox_write_file` + // matches no term but `file`, so returning it would be filler, not a result. assertEquals( searchToolExposure({ query: "create_file update_file project file", authorized: fileCatalog, state: createToolExposureState(), }).matches.map((match) => match.name), - ["create_file", "update_file", "sandbox_write_file"], + ["create_file", "update_file"], ); }); @@ -865,3 +868,212 @@ it("tool_search is reserved only when deferred framework search is injected", () } assert(message.includes("reserved")); }); + +const VERYFRONT_LIST_TOOL_NAMES = [ + "list_accessible_agents", + "list_agent_runs", + "list_agent_templates", + "list_agent_tool_references", + "list_agent_workers", + "list_agents", + "list_child_agent_runs", + "list_child_agent_runs_by_parent_conversation", + "list_eval_runs", + "list_evals", + "list_external_files", + "list_files", + "list_input_requests", + "list_integrations", + "list_models", + "list_projects", + "list_prompts", + "list_releases", + "list_resources", + "list_sandbox_background_commands", + "list_sandbox_sessions", + "list_schedules", + "list_skills", + "list_tasks", + "list_tools", + "list_uploads", + "list_workflows", +]; + +const CATALOG_NAMESPACE_DESCRIPTION = + "Get detailed configuration, available tool IDs, and input schemas for an integration. " + + "Use this for integration tool IDs in these namespaces: confluence, github, jira, salesforce."; + +function integrationDiscoveryCatalog(): ToolDefinition[] { + return [ + ...VERYFRONT_LIST_TOOL_NAMES.map((name) => + definition(name, `List ${name.slice("list_".length).replaceAll("_", " ")}`) + ), + definition("get_integration", CATALOG_NAMESPACE_DESCRIPTION, "integration namespace name"), + ]; +} + +it("tool search ranks a rare description term above a common name term", () => { + const matches = searchToolExposure({ + query: "list github issues", + authorized: integrationDiscoveryCatalog(), + state: createToolExposureState(), + }).matches.map((match) => match.name); + + assertEquals(matches[0], "get_integration"); + assertEquals(matches.filter((name) => name.startsWith("list_agent")), []); +}); + +it("tool search resolves a canonical integration tool id to its namespace", () => { + // Every one of these word-splits onto a platform tool that would otherwise win: + // `list_projects`, `list_...`, and the `search_*` family all share the generic + // half of the id. Resolving the namespace is what keeps the wrong tool out. + for ( + const query of [ + "jira__list_projects", + "jira__list_comments", + "jira__list_sites", + "jira__search_users", + "github__list_repos", + ] + ) { + assertEquals( + searchToolExposure({ + query, + authorized: integrationDiscoveryCatalog(), + state: createToolExposureState(), + }).matches.map((match) => match.name), + ["get_integration"], + `canonical id ${query} must resolve to its namespace`, + ); + } +}); + +it("tool search prefers an authorized integration tool over its namespace catalog entry", () => { + const result = searchToolExposure({ + query: "jira__list_projects", + authorized: [ + ...integrationDiscoveryCatalog(), + definition("jira__list_projects", "List Jira projects on a site"), + ], + state: createToolExposureState(), + }); + + assertEquals(result.matches[0]?.name, "jira__list_projects"); +}); + +it("tool search does not let a same-named local tool satisfy a canonical id", () => { + // `jira__list_projects` and the local id `jira_list_projects` normalize to the + // same text, and the registry permits any local id without `__`. A phrase match + // must not hand back the local tool for a canonical query. + const result = searchToolExposure({ + query: "jira__list_projects", + authorized: [ + ...integrationDiscoveryCatalog(), + definition("jira_list_projects", "A project-local tool that is not the Jira integration"), + ], + state: createToolExposureState(), + }); + + assertEquals(result.matches.map((match) => match.name), ["get_integration"]); +}); + +it("tool search keeps a malformed namespace-shaped query off local tools", () => { + // `jira__list__projects` is not a valid canonical id, but `__` is reserved for + // the integration namespace and local ids may not contain it. Normalization + // collapses it onto the local `jira_list_projects`, which must not win. + const authorized = [ + ...integrationDiscoveryCatalog(), + definition("jira_list_projects", "A project-local tool that is not the Jira integration"), + ]; + + for (const query of ["jira__list__projects", "JIRA__LIST_PROJECTS", "jira__list_projects "]) { + assertEquals( + searchToolExposure({ query, authorized, state: createToolExposureState() }).matches.map(( + match, + ) => match.name), + ["get_integration"], + `namespace-shaped query ${JSON.stringify(query)} must not resolve to a local tool`, + ); + } +}); + +it("tool search resolves a namespace that itself contains an underscore", () => { + // The grammar permits `_` inside a namespace segment, but candidate text has + // underscores rewritten to spaces, so an un-normalized namespace never matches. + assertEquals( + searchToolExposure({ + query: "foo_bar__list_items", + authorized: [ + definition("get_integration", "Tool ids and schemas. Namespaces: foo_bar, jira."), + definition("list_items", "List items in this project"), + ], + state: createToolExposureState(), + }).matches.map((match) => match.name), + ["get_integration"], + ); +}); + +it("tool search matches a canonical namespace as a whole token", () => { + // `exa` is a real integration name. Substring evidence would admit anything + // whose text merely contains `example`, which is most of a catalog. + assertEquals( + searchToolExposure({ + query: "exa__search", + authorized: [ + definition("get_integration", "Tool ids and schemas. Namespaces: exa, jira."), + definition("run_sample", "Runs the example workflow", "an example value"), + ], + state: createToolExposureState(), + }).matches.map((match) => match.name), + ["get_integration"], + ); +}); + +it("tool search accepts canonical ids with the authorization layer's segment grammar", () => { + // The authoritative grammar allows consecutive and trailing separators; search + // must not be stricter, or it disagrees with authorization about what an id is. + assertEquals( + searchToolExposure({ + query: "github__list-issues-", + authorized: integrationDiscoveryCatalog(), + state: createToolExposureState(), + }).matches.map((match) => match.name), + ["get_integration"], + ); +}); + +it("tool search matches a full-coverage candidate in a single-tool catalog", () => { + // With one candidate every term matches everything, so no term can clear the + // selectivity floor. Full term coverage is its own evidence: there is no + // competing candidate for the floor to protect against. + assertEquals( + searchToolExposure({ + query: "create project", + authorized: [definition("create_file", "Create a project file")], + state: createToolExposureState(), + }).matches.map((match) => match.name), + ["create_file"], + ); +}); + +it("tool search reports a miss when no candidate matches a selective term", () => { + assertEquals( + searchToolExposure({ + query: "list spreadsheet macros", + authorized: integrationDiscoveryCatalog(), + state: createToolExposureState(), + }), + { matches: [], resultCount: 0, loadedCount: 0, miss: true }, + ); +}); + +it("tool search still resolves a bare platform tool id to its exact name match", () => { + assertEquals( + searchToolExposure({ + query: "list_projects", + authorized: integrationDiscoveryCatalog(), + state: createToolExposureState(), + }).matches[0]?.name, + "list_projects", + ); +}); diff --git a/src/agent/runtime/tool-exposure.ts b/src/agent/runtime/tool-exposure.ts index a339584bad..0602e9c1f3 100644 --- a/src/agent/runtime/tool-exposure.ts +++ b/src/agent/runtime/tool-exposure.ts @@ -1,4 +1,5 @@ import type { ToolDefinition } from "#veryfront/tool"; +import { parseIntegrationToolIdentity } from "#veryfront/integrations/source-policy.ts"; import type { RuntimeToolLoadingMode } from "./runtime-tool-config.ts"; import { isOwnDataPropertyDescriptor } from "./data-property-descriptor.ts"; @@ -7,6 +8,33 @@ export const TOOL_SEARCH_TOOL_NAME = "tool_search"; const DEFAULT_BOOTSTRAP_TOOL_NAMES = new Set(["load_skill"]); const TOOL_SEARCH_RESULT_LIMIT = 5; +/** Which field a query term matched on, strongest evidence first. */ +type ToolSearchMatchField = "exactName" | "name" | "description" | "parameterDescription"; + +/** Ordering for a whole-query match: lower wins. */ +const TOOL_SEARCH_FIELD_PRECEDENCE: Record = { + exactName: 0, + name: 1, + description: 2, + parameterDescription: 3, +}; + +/** Contribution to a per-term score: higher wins. Keyed by the same union as the + * precedence above so the two orderings cannot drift apart silently. */ +const TOOL_SEARCH_FIELD_WEIGHTS: Record = { + exactName: 4, + name: 3, + description: 2, + parameterDescription: 1, +}; +/** + * A term matching more than half the catalog carries no information about which + * tool the caller meant. `log(2)` is exactly the inverse-document-frequency of a + * term present in half the candidates, so it is the point where a term stops + * discriminating. Below it a term may still contribute score, but it can never + * be the sole reason a candidate is returned. + */ +const TOOL_SEARCH_MIN_SELECTIVE_IDF = Math.LN2; const TOOL_SEARCH_QUERY_MAX_BYTES = 256; const TOOL_SEARCH_CANDIDATE_LIMIT = 4_096; const TOOL_SEARCH_NAME_MAX_BYTES = 256; @@ -67,13 +95,10 @@ export type ToolSearchResult = { miss: boolean; }; -type RankedToolSearchMatch = { - rank: number; - matchCount: number; - match: ToolSearchMatch; -}; - type SearchableTool = ToolSearchMatch & { + /** Normalized once at snapshot time: matching rescans every candidate per term. */ + normalizedName: string; + normalizedDescription: string; parameterDescriptions: string[]; }; @@ -87,6 +112,62 @@ function normalizeSearchText(value: string): string { .replaceAll("_", " ").trim().replace(/\s+/g, " "); } +/** + * Recognize `__` on the raw query, before normalization + * rewrites `_` to a space and destroys the separator. + * + * Defers to the authorization layer's grammar rather than restating it: a query + * search accepts but authorization rejects, or the reverse, is a disagreement + * about what a canonical id even is. + */ +function parseCanonicalIntegrationQuery( + query: string, +): { namespace: string; canonicalName: string | null } | null { + const trimmed = query.trim().toLowerCase(); + const separator = trimmed.indexOf("__"); + if (separator <= 0) return null; + + const identity = parseIntegrationToolIdentity(trimmed); + if (identity !== null) { + return { + namespace: identity.integration, + canonicalName: `${identity.integration}__${identity.toolId}`, + }; + } + + // `__` is the reserved integration namespace separator and `assertLocalToolId` + // forbids it in local ids, so a query carrying it is asking for an integration + // tool even when the rest is malformed (`jira__list__projects`). Keep such a + // query on the namespace path: otherwise normalization collapses it onto a + // local id like `jira_list_projects`, which then wins the phrase match. + const namespace = trimmed.slice(0, separator); + return parseIntegrationToolIdentity(`${namespace}__placeholder`) === null + ? null + : { namespace, canonicalName: null }; +} + +/** + * Match a namespace as a whole token, never as a substring. + * + * Namespaces can be very short (`exa` is a real integration), so substring + * evidence would admit any tool whose text merely contains `example`. Normalized + * text is lowercase with `_` rewritten to spaces, so the boundaries are anything + * that is not alphanumeric. + */ +function createNamespaceTokenPattern(namespaceTerm: string): RegExp { + const escaped = namespaceTerm.replace(/[.*+?^${}()|[\]\\-]/g, "\\$&"); + return new RegExp(`(? right ? 1 : 0; } @@ -232,6 +313,8 @@ function snapshotSearchableTool( name: name.value, description: description.value, status, + normalizedName: normalizeSearchText(name.value), + normalizedDescription: normalizeSearchText(description.value), parameterDescriptions: parameters ? snapshotSchemaDescriptions(parameters.value, budget) ?? [] : [], @@ -241,26 +324,22 @@ function snapshotSearchableTool( } } -function getIndexedMatchRank( +function getMatchedField( query: string, tool: SearchableTool, -): number | null { - const name = normalizeSearchText(tool.name); - if (name === query) return 0; - if (name.includes(query)) return 1; - if (normalizeSearchText(tool.description).includes(query)) return 2; - return tool.parameterDescriptions.some((description) => description.includes(query)) ? 3 : null; +): ToolSearchMatchField | null { + if (tool.normalizedName === query) return "exactName"; + if (tool.normalizedName.includes(query)) return "name"; + if (tool.normalizedDescription.includes(query)) return "description"; + return tool.parameterDescriptions.some((description) => description.includes(query)) + ? "parameterDescription" + : null; } -function rankToolExposureMatches(input: { - query: string; +function collectSearchCandidates(input: { available: readonly ToolDefinition[]; authorized: readonly ToolDefinition[]; -}): RankedToolSearchMatch[] { - if (!isUtf8LengthWithin(input.query, TOOL_SEARCH_QUERY_MAX_BYTES)) return []; - const query = normalizeSearchText(input.query); - if (!query) return []; - const terms = [...new Set(query.split(/\s+/).filter(Boolean))]; +}): SearchableTool[] { const budget: SchemaSearchBudget = { nodes: 0, bytes: 0 }; const candidates: SearchableTool[] = []; let examinedCandidates = 0; @@ -274,38 +353,140 @@ function rankToolExposureMatches(input: { }; append(input.available, "available"); append(input.authorized, "loaded"); - const ranked: RankedToolSearchMatch[] = []; + return candidates; +} - for (const tool of candidates) { - const rank = getIndexedMatchRank(query, tool); - if (rank === null) continue; +/** Rank candidates against the query taken as one phrase, strongest field first. */ +function rankWholeQueryMatches( + query: string, + candidates: readonly SearchableTool[], +): ToolSearchMatch[] { + const ranked: { precedence: number; match: ToolSearchMatch }[] = []; + for (const candidate of candidates) { + const field = getMatchedField(query, candidate); + if (field === null) continue; ranked.push({ - rank, - matchCount: 1, - match: { name: tool.name, description: tool.description, status: tool.status }, + precedence: TOOL_SEARCH_FIELD_PRECEDENCE[field], + match: toSearchMatch(candidate), }); } + return ranked + .sort((left, right) => + left.precedence - right.precedence || compareToolSearchMatches(left.match, right.match) + ) + .map(({ match }) => match); +} - if (ranked.length === 0 && terms.length >= 2) { - for (const tool of candidates) { - const ranks = terms.flatMap((term) => { - const rank = getIndexedMatchRank(term, tool); - return rank === null ? [] : [rank]; - }); - if (ranks.length === 0) continue; - ranked.push({ - rank: Math.min(...ranks), - matchCount: ranks.length, - match: { name: tool.name, description: tool.description, status: tool.status }, - }); +/** + * Score candidates across independent query terms. + * + * Two properties matter, and the previous pure-OR fallback had neither. First, + * a term is weighted by how few candidates it matches, so a rare term such as an + * integration namespace outweighs a common one such as `list`. Second, a + * candidate must match at least one *selective* term to be returned at all; + * otherwise a query containing one common word returns whichever tools happen to + * sort first, and reports it as a hit. + */ +function scoreToolExposureTerms( + terms: readonly string[], + candidates: readonly SearchableTool[], +): ToolSearchMatch[] { + const total = candidates.length; + if (total === 0 || terms.length === 0) return []; + + const weightedTerms = terms.map((term) => { + let documentFrequency = 0; + for (const candidate of candidates) { + if (getMatchedField(term, candidate) !== null) documentFrequency += 1; } + return { + term, + inverseDocumentFrequency: documentFrequency === 0 + ? 0 + : Math.log((total + 1) / (documentFrequency + 0.5)), + }; + }); + + const scored: { score: number; match: ToolSearchMatch }[] = []; + for (const candidate of candidates) { + let score = 0; + let matchedTermCount = 0; + let matchedSelectiveTerm = false; + for (const { term, inverseDocumentFrequency } of weightedTerms) { + const field = getMatchedField(term, candidate); + if (field === null) continue; + matchedTermCount += 1; + score += inverseDocumentFrequency * TOOL_SEARCH_FIELD_WEIGHTS[field]; + if (inverseDocumentFrequency >= TOOL_SEARCH_MIN_SELECTIVE_IDF) matchedSelectiveTerm = true; + } + // Selectivity suppresses filler, which only means anything when there is + // something better to prefer. A candidate matching *every* term is not filler + // however common those terms are: in a one-tool catalog every term matches + // everything, so the floor alone would report a certain match as a miss. + if (!matchedSelectiveTerm && matchedTermCount < terms.length) continue; + scored.push({ score, match: toSearchMatch(candidate) }); + } + + return scored + .sort((left, right) => + right.score - left.score || compareToolSearchMatches(left.match, right.match) + ) + .map(({ match }) => match); +} + +function rankToolExposureMatches(input: { + query: string; + available: readonly ToolDefinition[]; + authorized: readonly ToolDefinition[]; +}): ToolSearchMatch[] { + if (!isUtf8LengthWithin(input.query, TOOL_SEARCH_QUERY_MAX_BYTES)) return []; + const query = normalizeSearchText(input.query); + if (!query) return []; + const candidates = collectSearchCandidates(input); + + // Canonical integration ids are classified before any normalized matching. + // Normalization maps `jira__list_projects` and the *local* id `jira_list_projects` + // onto the same text, so a phrase pass here would hand back a same-named local + // tool instead of the namespace the caller actually asked for. Only the real + // canonical name satisfies a canonical query; anything else is namespace discovery. + const canonical = parseCanonicalIntegrationQuery(input.query); + if (canonical !== null) { + const canonicalName = canonical.canonicalName; + const exact = canonicalName === null ? [] : candidates + .filter((candidate) => candidate.name.toLowerCase() === canonicalName) + .map(toSearchMatch) + .sort(compareToolSearchMatches); + if (exact.length > 0) return exact; + + // Namespace discovery accepts two kinds of evidence, and a coincidental name + // match is neither: a sibling tool in the same namespace, or a tool that + // *documents* the namespace (the catalog readers do). A local + // `jira_list_projects` merely contains the word and is not the Jira integration. + // + // Sibling identity compares raw ids; text evidence must compare normalized, + // because a namespace may itself contain `_` (`foo_bar__list_items`) and every + // candidate's text has already had underscores rewritten to spaces. + const namespaceTerm = normalizeSearchText(canonical.namespace); + const namespacePattern = createNamespaceTokenPattern(namespaceTerm); + const namespaceCandidates = candidates.filter((candidate) => { + const identity = parseIntegrationToolIdentity(candidate.name.toLowerCase()); + if (identity !== null) return identity.integration === canonical.namespace; + // A non-canonical tool carrying the namespace in its *name* is a + // normalization coincidence, not the integration, whatever its description + // happens to mention: `jira_list_projects` is a local tool, not Jira. + if (namespacePattern.test(candidate.normalizedName)) return false; + return namespacePattern.test(candidate.normalizedDescription) || + candidate.parameterDescriptions.some((description) => namespacePattern.test(description)); + }); + return rankWholeQueryMatches(namespaceTerm, namespaceCandidates); } - return ranked.sort((left, right) => - left.rank - right.rank || right.matchCount - left.matchCount || - (left.match.status === right.match.status ? 0 : left.match.status === "available" ? -1 : 1) || - compareAscii(left.match.name, right.match.name) - ); + // The query taken whole is the strongest signal for every non-canonical query. + const wholeQueryMatches = rankWholeQueryMatches(query, candidates); + if (wholeQueryMatches.length > 0) return wholeQueryMatches; + + const terms = [...new Set(query.split(/\s+/).filter(Boolean))]; + return terms.length >= 2 ? scoreToolExposureTerms(terms, candidates) : []; } /** Create fresh run-local tool exposure state. */ @@ -420,11 +601,10 @@ export function searchToolExposure(input: { available: input.available ?? [], authorized: input.authorized, }); - if (ranked[0]?.match.status === "available") { + if (ranked[0]?.status === "available") { const matches = ranked - .filter(({ match }) => match.status === "available") - .slice(0, TOOL_SEARCH_RESULT_LIMIT) - .map(({ match }) => match); + .filter((match) => match.status === "available") + .slice(0, TOOL_SEARCH_RESULT_LIMIT); return { matches, resultCount: matches.length, @@ -434,14 +614,13 @@ export function searchToolExposure(input: { } const matches = ranked - .filter(({ match }) => match.status === "loaded") + .filter((match) => match.status === "loaded") .slice( 0, input.maxLoadedTools === undefined ? TOOL_SEARCH_RESULT_LIMIT : Math.min(TOOL_SEARCH_RESULT_LIMIT, input.maxLoadedTools), - ) - .map(({ match }) => match); + ); for (const match of matches) { input.state.loadedToolNames.delete(match.name);