From 7ee66a5a096afd97e3b2a342c0c93706a559b8a9 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 14 Aug 2026 00:06:12 +0200 Subject: [PATCH 1/2] fix(sdk): say a connection produced no tools instead of blaming the tool Discovery can come back empty for reasons that have nothing to do with the tool being asked for. The case that surfaced this: a credential the upstream rejects breaks discovery, the catalog comes back empty, the address resolves to nothing, and the caller is told the TOOL does not exist. That sends someone looking for a renamed or removed tool, which is the one thing that is not wrong. The address really does not resolve, so ToolNotFoundError stays the right error. What was missing is the fact that separates a mistyped tool name from a connection that produced nothing, so the error now carries it when the connection exists but has no tools, and points at health, which does report the underlying cause. `reason` is optional, so an ordinary unknown tool name reads exactly as before. Both directions are pinned: the new assertion failed before this change, and emitting the reason unconditionally reddens the populated-catalog control. --- packages/core/sdk/src/errors.ts | 8 +- packages/core/sdk/src/executor.ts | 16 +++ .../src/tool-not-found-empty-catalog.test.ts | 107 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 packages/core/sdk/src/tool-not-found-empty-catalog.test.ts diff --git a/packages/core/sdk/src/errors.ts b/packages/core/sdk/src/errors.ts index 7f28ab58e..d51cd164a 100644 --- a/packages/core/sdk/src/errors.ts +++ b/packages/core/sdk/src/errors.ts @@ -50,10 +50,16 @@ export class ToolNotFoundError extends Schema.TaggedErrorClass 0 ? searchMatches : yield* findToolRowsForConnection(parsed); + // An empty catalog on a connection that DOES exist is usually not a + // wrong tool name: discovery produced nothing, most often because the + // upstream rejected the credential. Reporting only the address sends + // the reader after a tool that was never the problem, so name the + // connection and point at the surface that knows the cause. + const connectionExists = + connectionTools.length === 0 && + (yield* findConnectionRow({ + owner: parsed.owner, + integration: parsed.integration, + name: parsed.connection, + })) !== null; return yield* new ToolNotFoundError({ address, suggestions: toolSuggestions(connectionTools), + reason: connectionExists + ? `connection "${parsed.integration}/${parsed.connection}" has no tools; ` + + `check its health for why discovery produced none` + : undefined, }); } diff --git a/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts b/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts new file mode 100644 index 000000000..f136de018 --- /dev/null +++ b/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts @@ -0,0 +1,107 @@ +// --------------------------------------------------------------------------- +// When a connection produced no tools, say so. +// +// Discovery can come back empty for reasons that have nothing to do with the +// tool being asked for — most commonly a credential the upstream rejects, which +// is how this was found: a provider returning a value with a stray newline +// broke discovery, the catalog came back empty, and invoking a tool reported +// that the TOOL did not exist. Someone reading that goes looking for a renamed +// or removed tool, which is the one thing that is not wrong. +// +// The address genuinely does not resolve, so `ToolNotFoundError` is the right +// error. What was missing is that it said nothing about the connection behind +// it having no tools at all — the fact that separates "you typed the wrong tool +// name" from "this connection produced nothing". +// --------------------------------------------------------------------------- + +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { createExecutor } from "./executor"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + ProviderItemId, + ProviderKey, + ToolAddress, + ToolName, +} from "./ids"; +import { definePlugin } from "./plugin"; +import type { CredentialProvider } from "./provider"; +import { makeTestConfig } from "./test-config"; + +const STORE = ProviderKey.make("memory"); +const INTEG = IntegrationSlug.make("demo"); +const TEMPLATE = AuthTemplateSlug.make("apiKey"); +const CONN = ConnectionName.make("main"); + +const provider: CredentialProvider = { + key: STORE, + writable: true, + get: () => Effect.succeed("token"), + set: () => Effect.void, +}; + +/** `tools` is what discovery produced: empty stands in for a connection whose + * upstream rejected the credential, which yields no catalog. */ +const pluginWith = (tools: readonly { readonly name: ToolName; readonly description: string }[]) => + definePlugin(() => ({ + id: "demo" as const, + credentialProviders: [provider], + storage: () => ({}), + resolveTools: () => Effect.succeed({ tools: [...tools] }), + invokeTool: ({ toolRow }) => Effect.succeed({ ran: toolRow.name }), + extension: (ctx) => ({ + seed: () => ctx.core.integrations.register({ slug: INTEG, description: "Demo", config: {} }), + }), + }))(); + +const EMPTY = pluginWith([]); +const POPULATED = pluginWith([ + { name: ToolName.make("inspect"), description: "inspect" }, + { name: ToolName.make("deploy"), description: "deploy" }, +]); + +const failInvoking = (plugin: ReturnType, tool: string) => + Effect.gen(function* () { + const executor = yield* createExecutor({ ...makeTestConfig({ plugins: [plugin] as const }) }); + yield* executor.demo.seed(); + yield* executor.connections.create({ + owner: "org", + name: CONN, + integration: INTEG, + template: TEMPLATE, + from: { provider: STORE, id: ProviderItemId.make("item-1") }, + }); + + const exit = yield* Effect.exit( + executor.execute(ToolAddress.make(`tools.${INTEG}.org.${CONN}.${tool}`), {}), + ); + expect(exit._tag).toBe("Failure"); + if (exit._tag !== "Failure") throw new Error("expected a failure"); + return exit.cause; + }); + +describe("invoking a tool on a connection that produced no tools", () => { + it.effect("says the connection produced no tools", () => + Effect.gen(function* () { + const cause = yield* failInvoking(EMPTY, "whoami"); + + // Naming only the tool sends the reader after a tool that was never the + // problem. The message has to carry the connection's empty catalog. + expect(String(cause)).toMatch(/no tools/i); + }), + ); + + it.effect("still reports a plain unknown tool when the catalog is populated", () => + Effect.gen(function* () { + // The control, and the reason the first assertion means something: a + // message that always mentioned an empty catalog would satisfy it while + // being wrong for every ordinary typo. + const cause = yield* failInvoking(POPULATED, "nosuchtool"); + + expect(String(cause)).not.toMatch(/no tools/i); + }), + ); +}); From 768a7e8a87bd0ff7573291a77cbc7e486df170e2 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Fri, 14 Aug 2026 18:39:24 +0200 Subject: [PATCH 2/2] test(sdk): assert the typed reason field instead of a stringified cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test helper tripped four of the repo's own lint rules at once — manual _tag inspection, a raw throw, a built-in Error constructor, and stringifying an unknown — so this branch would have gone red the first time fork CI ran on it. Effect.flip removes the need for all four: the failure becomes the value, already typed, and an unexpected success fails the test by itself. The assertions now pin `reason`, the field this change actually adds, rather than regex-matching a rendered message. Mutation-checked: emitting no reason fails the empty-catalog test and leaves the populated-catalog control green. --- .../src/tool-not-found-empty-catalog.test.ts | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts b/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts index f136de018..d8c01a541 100644 --- a/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts +++ b/packages/core/sdk/src/tool-not-found-empty-catalog.test.ts @@ -15,7 +15,7 @@ // --------------------------------------------------------------------------- import { describe, expect, it } from "@effect/vitest"; -import { Effect } from "effect"; +import { Effect, Predicate } from "effect"; import { createExecutor } from "./executor"; import { @@ -75,22 +75,26 @@ const failInvoking = (plugin: ReturnType, tool: string) => from: { provider: STORE, id: ProviderItemId.make("item-1") }, }); - const exit = yield* Effect.exit( + // `Effect.flip` rather than unwrapping an Exit: the failure becomes the value, already + // typed, so nothing here inspects a `_tag`, throws, or stringifies an unknown. If the + // invocation unexpectedly SUCCEEDS, the flip fails the test on its own. + return yield* Effect.flip( executor.execute(ToolAddress.make(`tools.${INTEG}.org.${CONN}.${tool}`), {}), ); - expect(exit._tag).toBe("Failure"); - if (exit._tag !== "Failure") throw new Error("expected a failure"); - return exit.cause; }); describe("invoking a tool on a connection that produced no tools", () => { it.effect("says the connection produced no tools", () => Effect.gen(function* () { - const cause = yield* failInvoking(EMPTY, "whoami"); + const error = yield* failInvoking(EMPTY, "whoami"); // Naming only the tool sends the reader after a tool that was never the - // problem. The message has to carry the connection's empty catalog. - expect(String(cause)).toMatch(/no tools/i); + // problem. The error has to carry the connection's empty catalog. Asserting + // on the typed `reason` field rather than the rendered message pins the thing + // this change actually adds. + expect(Predicate.isTagged(error, "ToolNotFoundError")).toBe(true); + if (!Predicate.isTagged(error, "ToolNotFoundError")) return; + expect(error.reason ?? "").toMatch(/no tools/i); }), ); @@ -99,9 +103,11 @@ describe("invoking a tool on a connection that produced no tools", () => { // The control, and the reason the first assertion means something: a // message that always mentioned an empty catalog would satisfy it while // being wrong for every ordinary typo. - const cause = yield* failInvoking(POPULATED, "nosuchtool"); + const error = yield* failInvoking(POPULATED, "nosuchtool"); - expect(String(cause)).not.toMatch(/no tools/i); + expect(Predicate.isTagged(error, "ToolNotFoundError")).toBe(true); + if (!Predicate.isTagged(error, "ToolNotFoundError")) return; + expect(error.reason).toBeUndefined(); }), ); });