diff --git a/src/tools/capability-registry/register.ts b/src/tools/capability-registry/register.ts index 2a8bf5e9..9c7219b9 100644 --- a/src/tools/capability-registry/register.ts +++ b/src/tools/capability-registry/register.ts @@ -290,6 +290,36 @@ export function addCapabilityRegistryTools( return failed(message, extra); }; + /** + * A question for the user, not a failure. + * + * A product clash stops the search just as a refusal does, but it is the EXPECTED path + * for an ambiguous request, and surfacing it as a tool error made every client render + * it in red and every agent narrate "the search failed". It is returned as an ordinary + * result whose `status` says what to do; the search still does not run. Counted in + * telemetry under `ask_user_reason`, next to the refusals it used to sit with. + */ + const askUser = ( + tool: string, + reason: string, + message: string, + extra: Record, + extras?: MCPEventExtras, + ): CallToolResult => { + try { + trackMCP(tool, server.server.getClientVersion()!, undefined, config, { + ask_user_reason: reason, + ...(extras ?? {}), + }); + } catch { + // Telemetry must not decide whether a tool call succeeds. + } + return ok({ status: "ask_user", reason, message, ...extra }); + }; + + /** The product's title for anything put to a person; the key stays the handle. */ + const label = (name: string) => registry.index.products[name]?.title ?? name; + tools.listProducts = server.tool( "listProducts", "List the BrowserStack products this surface can reach: what each one does, the " + @@ -350,9 +380,6 @@ export function addCapabilityRegistryTools( : { verdict: "settled", product: ambiguity.settled }, ) : undefined; - /** The product's title for the question put to a person; the key stays the handle. */ - const label = (name: string) => - registry.index.products[name]?.title ?? name; const routing = ambiguity.products.length > 1 ? { @@ -792,12 +819,15 @@ export function addCapabilityRegistryTools( .filter(Boolean); return { product: name, + ...(registry.index.products[name]?.title + ? { title: registry.index.products[name]?.title } + : {}), summary: registry.index.products[name]?.summary, ...(senses.length ? { shared_terms: senses } : {}), }; }); const named = anchor.products.length > 0; - return refuse( + return askUser( "searchCapability", "product_ambiguous", (blank @@ -809,15 +839,15 @@ export function addCapabilityRegistryTools( (ambiguity.because?.length ? ` (${ambiguity.because.map((t) => `'${t}'`).join(", ")})` : "") + - `, so the request points at ${ambiguity.settled}, not ${product}. ` + + `, so the request points at ${label(ambiguity.settled as string)}, not ${label(product)}. ` + "Search the product the words name, or — if the user meant " + - `${product} — get them to say so and quote them. ` - : `'${subject}' could mean ${ambiguity.products.join(" or ")} — ` + + `${label(product)} — get them to say so and quote them. ` + : `'${subject}' could mean ${ambiguity.products.map(label).join(" or ")} — ` + `${ambiguity.terms.map((t) => `'${t}'`).join(", ")} ` + `${ambiguity.terms.length === 1 ? "belongs" : "belong"} to both. `) + (named ? `The words you quoted (${anchor.terms.map((t) => `'${t}'`).join(", ")}) ` + - `point at ${anchor.products.join(" and ")}, not ${product}. ` + `point at ${anchor.products.map(label).join(" and ")}, not ${label(product)}. ` : user_words ? "Nothing in the words you quoted names a product or a term only one " + "product uses. " @@ -830,7 +860,7 @@ export function addCapabilityRegistryTools( { routing_token: token, clarify: { - question: `Which product do you mean — ${choices.join(" or ")}?`, + question: `Which product do you mean — ${choices.map(label).join(" or ")}?`, shared: ambiguity.terms, options, }, diff --git a/tests/tools/capabilityRegistryE2E.test.ts b/tests/tools/capabilityRegistryE2E.test.ts index 7d8d2e43..a87ff91b 100644 --- a/tests/tools/capabilityRegistryE2E.test.ts +++ b/tests/tools/capabilityRegistryE2E.test.ts @@ -25,9 +25,15 @@ const FIXTURE_DIR = fileURLToPath( */ const SHIPPED_DIR = mkdtempSync(join(tmpdir(), "cap-registry-multi-")); for (const [from, name] of [ - [new URL("../../capability/tm.capability-index.json", import.meta.url), "tm.capability-index.json"], [ - new URL("../fixtures/capability-multi/secondproduct.capability-index.json", import.meta.url), + new URL("../../capability/tm.capability-index.json", import.meta.url), + "tm.capability-index.json", + ], + [ + new URL( + "../fixtures/capability-multi/secondproduct.capability-index.json", + import.meta.url, + ), "secondproduct.capability-index.json", ], ] as const) { @@ -148,9 +154,8 @@ describe("capability registry, end to end through the server factory", () => { it("carries each entity's one-line meaning on the routing call", async () => { const server = await buildServer(); const listed = JSON.parse( - ( - await (server.getTools().listProducts as any).handler({}, {} as any) - ).content[0].text, + (await (server.getTools().listProducts as any).handler({}, {} as any)) + .content[0].text, ); const byName = Object.fromEntries( listed.products[0].entities.map((e: any) => [e.entity, e]), @@ -214,21 +219,26 @@ describe("capability registry, end to end through the server factory", () => { const server = await buildServer(); const search = server.getTools().searchCapability as any; const run = async (args: Record) => { - const r = await search.handler( - { product: "tm", ...args }, - {} as any, - ); - return { blocked: r.isError === true, body: JSON.parse(r.content[0].text) }; + const r = await search.handler({ product: "tm", ...args }, {} as any); + const body = JSON.parse(r.content[0].text); + // The gate is a question for the user, not an error: it stops the search without + // flagging the call as failed. + return { + blocked: body.status === "ask_user" && r.isError !== true, + body, + }; }; const refused = await run({ query: "list all projects" }); expect(refused.blocked).toBe(true); // The refusal has to be actionable: which products, and on which word. - expect(refused.body.error).toContain("secondproduct or tm"); - expect(refused.body.error).toContain("'project'"); - expect(refused.body.error).toMatch(/to the USER/); + // Products are named by title where they have one — tm's is "Test Management"; + // the fixture's second product declares none and reads as its key. + expect(refused.body.message).toContain("secondproduct or Test Management"); + expect(refused.body.message).toContain("'project'"); + expect(refused.body.message).toMatch(/to the USER/); // And it must name the failure mode it exists to stop. - expect(refused.body.error).toMatch(/search each product in turn and merge/); + expect(refused.body.message).toMatch(/search each product in turn and merge/); // A word only one product claims settles the query, so there is nothing to ask — // PROVIDED it settles on the product being searched. @@ -245,10 +255,11 @@ describe("capability registry, end to end through the server factory", () => { // settled, never whether it settled on what was being asked for. const crossed = await run({ query: "check my quota" }); expect(crossed.blocked).toBe(true); - expect(crossed.body.error).toMatch(/points at secondproduct, not tm/); - expect(crossed.body.error).toMatch(/only secondproduct claims \('quota'\)/); + expect(crossed.body.message).toMatch(/points at secondproduct, not Test Management/); + expect(crossed.body.message).toMatch(/only secondproduct claims \('quota'\)/); expect( - (await run({ query: "check my quota", product: "secondproduct" })).blocked, + (await run({ query: "check my quota", product: "secondproduct" })) + .blocked, ).toBe(false); // ONE WAY OUT, AND IT IS NOT A FLAG. `product_choice: 'user_confirmed'` used to open @@ -256,15 +267,21 @@ describe("capability registry, end to end through the server factory", () => { // caller wrote it on every ambiguous prompt with nothing behind it. What opens it now // is the user's own words, checked against the index rather than taken on trust. expect( - (await run({ - query: "list all projects", - user_words: "the test management one", - })).blocked, + ( + await run({ + query: "list all projects", + user_words: "the test management one", + }) + ).blocked, ).toBe(false); // And a flag by that name buys nothing, because there is no longer such an argument. expect( - (await run({ query: "list all projects", product_choice: "user_confirmed" })) - .blocked, + ( + await run({ + query: "list all projects", + product_choice: "user_confirmed", + }) + ).blocked, ).toBe(true); // `entity` IS NOT A WAY OUT, and used to be. It narrows within a product and says @@ -286,7 +303,11 @@ describe("capability registry, end to end through the server factory", () => { process.env.CAPABILITY_REGISTRY_INDEX_DIR = SHIPPED_DIR; const server = await buildServer(); const r: any = await (server.getTools().searchCapability as any).handler( - { query: "list all projects", product: "tm", product_choice: "not_asked" }, + { + query: "list all projects", + product: "tm", + product_choice: "not_asked", + }, {} as any, ); const { clarify } = JSON.parse(r.content[0].text); @@ -302,7 +323,10 @@ describe("capability registry, end to end through the server factory", () => { // word — the two things a user needs to answer without being shown a schema. const tm = clarify.options.find((o: any) => o.product === "tm"); expect(tm.summary).toMatch(/Test Management/); - expect(tm.shared_terms[0]).toMatchObject({ term: "project", entity: "project" }); + expect(tm.shared_terms[0]).toMatchObject({ + term: "project", + entity: "project", + }); expect(tm.shared_terms[0].means).toMatch(/top-level container/); // The second product ships no entity descriptions, so its sense of `project` has no @@ -321,13 +345,13 @@ describe("capability registry, end to end through the server factory", () => { process.env.CAPABILITY_REGISTRY_INDEX_DIR = SHIPPED_DIR; const server = await buildServer(); const search = server.getTools().searchCapability as any; - const blocked = async (query: string) => - ( - await search.handler( - { query, product: "tm", product_choice: "not_asked" }, - {} as any, - ) - ).isError === true; + const blocked = async (query: string) => { + const r = await search.handler( + { query, product: "tm", product_choice: "not_asked" }, + {} as any, + ); + return JSON.parse(r.content[0].text).status === "ask_user"; + }; // QUERY side: the entry is `project`, the user says "projects". Exact containment // missed this — and it is the query that was actually reported.