fix(connectors): return tool errors as results so the model can self-correct (#427)

src/connectors/tools.ts

@@ -200,7 +200,42 @@ export function createOpenWikiConnectorTools(): StructuredToolInterface[] {
           ),
         ),
     }),
-  ];
+  ].map(withToolErrorsAsResults);
+}
+
+/**
+ * Return thrown tool errors as the tool *result* instead of rejecting (#427).
+ *
+ * A thrown tool error aborts the agent run, and even when the run survives, the
+ * model never sees the message. That matters because these messages are written
+ * for the model — `callMcpTool` answers a wrong tool name with
+ *
+ *   MCP tool notion-get-page-content was not returned by tools/list for notion.
+ *   Run openwiki_list_mcp_tools first and use an exact discovered name.
+ *
+ * which is precisely the hint needed to retry correctly. Returning it lets the
+ * model self-correct; throwing it just ends the turn.
+ *
+ * Errors are surfaced, not swallowed: the result is prefixed `Tool error:` so a
+ * failure is never mistaken for data, and the run transcript still shows it.
+ */
+export function withToolErrorsAsResults(
+  tool: DynamicStructuredTool,
+): DynamicStructuredTool {
+  const originalFunc = tool.func.bind(tool) as (
+    ...args: Parameters<DynamicStructuredTool["func"]>
+  ) => Promise<unknown>;
+
+  tool.func = (async (...args: Parameters<DynamicStructuredTool["func"]>) => {
+    try {
+      return await originalFunc(...args);
+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      return `Tool error: ${message}`;
+    }
+  }) as DynamicStructuredTool["func"];
+
+  return tool;
 }
 
 async function listConnectors() {

test/connector-tool-errors.test.ts

@@ -0,0 +1,115 @@
+import { DynamicStructuredTool } from "@langchain/core/tools";
+import { describe, expect, test } from "vitest";
+
+import {
+  createOpenWikiConnectorTools,
+  withToolErrorsAsResults,
+} from "../src/connectors/tools.ts";
+
+// The connector tools threw on error instead of returning the message as the
+// tool result (issue #427). A thrown tool error aborts the agent run, and even
+// when it doesn't, the model never sees the text — which matters because these
+// messages are written *for* the model. `callMcpTool` answers a wrong tool name
+// with "Run openwiki_list_mcp_tools first and use an exact discovered name",
+// exactly the hint needed to retry, and the model never received it.
+//
+// Note on scope: schema violations are rejected by LangChain before `func` runs
+// and are a different layer. These tests cover errors thrown *inside* the tool,
+// which is what the issue is about.
+
+function toolNamed(name: string) {
+  const tool = createOpenWikiConnectorTools().find(
+    (candidate) => candidate.name === name,
+  );
+  if (!tool) {
+    throw new Error(`tool ${name} not registered`);
+  }
+  return tool;
+}
+
+describe("withToolErrorsAsResults", () => {
+  test("a thrown Error becomes a labelled result", async () => {
+    const tool = withToolErrorsAsResults(
+      new DynamicStructuredTool({
+        description: "d",
+        func: () => {
+          throw new Error("the model should read this");
+        },
+        name: "boom",
+        schema: { additionalProperties: false, properties: {}, type: "object" },
+      }),
+    );
+
+    await expect(tool.invoke({})).resolves.toBe(
+      "Tool error: the model should read this",
+    );
+  });
+
+  test("a rejected promise becomes a labelled result", async () => {
+    const tool = withToolErrorsAsResults(
+      new DynamicStructuredTool({
+        description: "d",
+        func: () => Promise.reject(new Error("async failure")),
+        name: "boom-async",
+        schema: { additionalProperties: false, properties: {}, type: "object" },
+      }),
+    );
+
+    await expect(tool.invoke({})).resolves.toBe("Tool error: async failure");
+  });
+
+  test("a non-Error throw is still reported", async () => {
+    const tool = withToolErrorsAsResults(
+      new DynamicStructuredTool({
+        description: "d",
+        // A provider that rejects with a bare string, which the wrapper must
+        // still report rather than losing.
+        // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors
+        func: () => Promise.reject("rate limited"),
+        name: "boom-string",
+        schema: { additionalProperties: false, properties: {}, type: "object" },
+      }),
+    );
+
+    await expect(tool.invoke({})).resolves.toBe("Tool error: rate limited");
+  });
+
+  test("a successful result passes through untouched", async () => {
+    const tool = withToolErrorsAsResults(
+      new DynamicStructuredTool({
+        description: "d",
+        func: () => Promise.resolve("real output"),
+        name: "fine",
+        schema: { additionalProperties: false, properties: {}, type: "object" },
+      }),
+    );
+
+    await expect(tool.invoke({})).resolves.toBe("real output");
+  });
+});
+
+describe("registered connector tools", () => {
+  test("a wrong MCP tool name returns the self-correction hint instead of throwing", async () => {
+    // The issue's headline case: the model guesses `notion-get-page-content`
+    // instead of a discovered name. Schema-valid, so it reaches the tool body.
+    const tool = toolNamed("openwiki_call_mcp_tool");
+
+    const result = String(
+      await tool.invoke({
+        args: {},
+        connectorId: "notion",
+        toolName: "notion-get-page-content",
+      }),
+    );
+
+    expect(result.startsWith("Tool error:")).toBe(true);
+    expect(result.length).toBeGreaterThan("Tool error: ".length);
+  });
+
+  test("a successful connector tool call is unaffected", async () => {
+    const result = String(await toolNamed("openwiki_list_connectors").invoke({}));
+
+    expect(result).not.toContain("Tool error:");
+    expect(() => JSON.parse(result) as unknown).not.toThrow();
+  });
+});

test/raw-connector-tools.test.ts

@@ -85,37 +85,43 @@ describe("raw connector tools", () => {
     const linkRelativePath = await createSymlinkRawItem(home, "x", runId);
     const tools = await loadConnectorTools(home);
 
-    await expect(
-      getTool(tools, "openwiki_read_raw_item").invoke({
+    // Surfaced as a labelled tool result rather than a rejection (#427);
+    // the symlink is still refused, and the reason still reaches the model.
+    const result = String(await getTool(tools, "openwiki_read_raw_item").invoke({
         connectorId: "x",
         maxBytes: 100,
         path: linkRelativePath,
-      }),
-    ).rejects.toThrow(/symbolic links/u);
+      }));
+    expect(result).toMatch(/^Tool error:/u);
+    expect(result).toMatch(/symbolic links/u);
   });
 
   test("rejects symlink raw directories before listing", async () => {
     const home = await createTempHome();
     await createSymlinkRawDir(home, "x");
     const tools = await loadConnectorTools(home);
 
-    await expect(
-      getTool(tools, "openwiki_list_raw_items").invoke({ connectorId: "x" }),
-    ).rejects.toThrow(/symbolic links/u);
+    // Surfaced as a labelled tool result rather than a rejection (#427);
+    // the symlink is still refused, and the reason still reaches the model.
+    const result = String(await getTool(tools, "openwiki_list_raw_items").invoke({ connectorId: "x" }));
+    expect(result).toMatch(/^Tool error:/u);
+    expect(result).toMatch(/symbolic links/u);
   });
 
   test("rejects symlink raw directories before reading", async () => {
     const home = await createTempHome();
     const rawItemPath = await createSymlinkRawDir(home, "x");
     const tools = await loadConnectorTools(home);
 
-    await expect(
-      getTool(tools, "openwiki_read_raw_item").invoke({
+    // Surfaced as a labelled tool result rather than a rejection (#427);
+    // the symlink is still refused, and the reason still reaches the model.
+    const result = String(await getTool(tools, "openwiki_read_raw_item").invoke({
         connectorId: "x",
         maxBytes: 100,
         path: rawItemPath,
-      }),
-    ).rejects.toThrow(/symbolic links/u);
+      }));
+    expect(result).toMatch(/^Tool error:/u);
+    expect(result).toMatch(/symbolic links/u);
   });
 });