Merge d5dc6eb443 into da71eaebd2

2026-01-30 17:05:54 +05:30 · 2026-01-30 17:05:54 +05:30 · 6e346b7ac4
commit 6e346b7ac4
parent da71eaebd2 d5dc6eb443
3 changed files with 107 additions and 52 deletions
--- a/src/agents/tools/web-fetch.ts
+++ b/src/agents/tools/web-fetch.ts
@ -227,7 +227,7 @@ async function fetchWithRedirects(params: {
        throw new Error("Redirect loop detected");
      }
      visited.add(nextUrl);
-      void res.body?.cancel();
+      res.body?.cancel().catch(() => {});
      await closeDispatcher(dispatcher);
      currentUrl = nextUrl;
      continue;
@ -282,15 +282,21 @@ export async function fetchFirecrawlContent(params: {
    storeInCache: params.storeInCache,
  };

-  const res = await fetch(endpoint, {
-    method: "POST",
-    headers: {
-      Authorization: `Bearer ${params.apiKey}`,
-      "Content-Type": "application/json",
-    },
-    body: JSON.stringify(body),
-    signal: withTimeout(undefined, params.timeoutSeconds * 1000),
-  });
+  let res: Response;
+  try {
+    res = await fetch(endpoint, {
+      method: "POST",
+      headers: {
+        Authorization: `Bearer ${params.apiKey}`,
+        "Content-Type": "application/json",
+      },
+      body: JSON.stringify(body),
+      signal: withTimeout(undefined, params.timeoutSeconds * 1000),
+    });
+  } catch (err) {
+    const detail = err instanceof Error ? err.message : String(err);
+    throw new Error(`Firecrawl fetch failed (network): ${detail}`);
+  }

  const payload = (await res.json()) as {
    success?: boolean;
@ -600,25 +606,36 @@ export function createWebFetchTool(options?: {
      const url = readStringParam(params, "url", { required: true });
      const extractMode = readStringParam(params, "extractMode") === "text" ? "text" : "markdown";
      const maxChars = readNumberParam(params, "maxChars", { integer: true });
-      const result = await runWebFetch({
-        url,
-        extractMode,
-        maxChars: resolveMaxChars(maxChars ?? fetch?.maxChars, DEFAULT_FETCH_MAX_CHARS),
-        maxRedirects: resolveMaxRedirects(fetch?.maxRedirects, DEFAULT_FETCH_MAX_REDIRECTS),
-        timeoutSeconds: resolveTimeoutSeconds(fetch?.timeoutSeconds, DEFAULT_TIMEOUT_SECONDS),
-        cacheTtlMs: resolveCacheTtlMs(fetch?.cacheTtlMinutes, DEFAULT_CACHE_TTL_MINUTES),
-        userAgent,
-        readabilityEnabled,
-        firecrawlEnabled,
-        firecrawlApiKey,
-        firecrawlBaseUrl,
-        firecrawlOnlyMainContent,
-        firecrawlMaxAgeMs,
-        firecrawlProxy: "auto",
-        firecrawlStoreInCache: true,
-        firecrawlTimeoutSeconds,
-      });
-      return jsonResult(result);
+      try {
+        const result = await runWebFetch({
+          url,
+          extractMode,
+          maxChars: resolveMaxChars(maxChars ?? fetch?.maxChars, DEFAULT_FETCH_MAX_CHARS),
+          maxRedirects: resolveMaxRedirects(fetch?.maxRedirects, DEFAULT_FETCH_MAX_REDIRECTS),
+          timeoutSeconds: resolveTimeoutSeconds(fetch?.timeoutSeconds, DEFAULT_TIMEOUT_SECONDS),
+          cacheTtlMs: resolveCacheTtlMs(fetch?.cacheTtlMinutes, DEFAULT_CACHE_TTL_MINUTES),
+          userAgent,
+          readabilityEnabled,
+          firecrawlEnabled,
+          firecrawlApiKey,
+          firecrawlBaseUrl,
+          firecrawlOnlyMainContent,
+          firecrawlMaxAgeMs,
+          firecrawlProxy: "auto",
+          firecrawlStoreInCache: true,
+          firecrawlTimeoutSeconds,
+        });
+        return jsonResult(result);
+      } catch (err) {
+        const message =
+          err instanceof Error ? err.message : typeof err === "string" ? err : "Unknown error";
+        return jsonResult({
+          status: "error",
+          tool: "web_fetch",
+          url,
+          error: `Web fetch failed: ${message}`,
+        });
+      }
    },
  };
 }
--- a/src/agents/tools/web-tools.fetch.test.ts
+++ b/src/agents/tools/web-tools.fetch.test.ts
@ -125,7 +125,7 @@ describe("web_fetch extraction fallbacks", () => {
    expect(details.text).toContain("firecrawl content");
  });

-  it("throws when readability is disabled and firecrawl is unavailable", async () => {
+  it("returns error result when readability is disabled and firecrawl is unavailable", async () => {
    const mockFetch = vi.fn((input: RequestInfo) =>
      Promise.resolve(htmlResponse("<html><body>hi</body></html>", requestUrl(input))),
    );
@ -143,12 +143,15 @@ describe("web_fetch extraction fallbacks", () => {
      sandboxed: false,
    });

-    await expect(
-      tool?.execute?.("call", { url: "https://example.com/readability-off" }),
-    ).rejects.toThrow("Readability disabled");
+    const result = await tool?.execute?.("call", {
+      url: "https://example.com/readability-off",
+    });
+    const details = result?.details as { status?: string; error?: string };
+    expect(details.status).toBe("error");
+    expect(details.error).toContain("Readability disabled");
  });

-  it("throws when readability is empty and firecrawl fails", async () => {
+  it("returns error result when readability is empty and firecrawl fails", async () => {
    const mockFetch = vi.fn((input: RequestInfo) => {
      const url = requestUrl(input);
      if (url.includes("api.firecrawl.dev")) {
@ -172,9 +175,12 @@ describe("web_fetch extraction fallbacks", () => {
      sandboxed: false,
    });

-    await expect(
-      tool?.execute?.("call", { url: "https://example.com/readability-empty" }),
-    ).rejects.toThrow("Readability and Firecrawl returned no content");
+    const result = await tool?.execute?.("call", {
+      url: "https://example.com/readability-empty",
+    });
+    const details = result?.details as { status?: string; error?: string };
+    expect(details.status).toBe("error");
+    expect(details.error).toContain("Readability and Firecrawl returned no content");
  });

  it("uses firecrawl when direct fetch fails", async () => {
@ -232,17 +238,14 @@ describe("web_fetch extraction fallbacks", () => {
      sandboxed: false,
    });

-    let message = "";
-    try {
-      await tool?.execute?.("call", { url: "https://example.com/missing" });
-    } catch (error) {
-      message = (error as Error).message;
-    }
-
-    expect(message).toContain("Web fetch failed (404):");
-    expect(message).toContain("Not Found");
-    expect(message).not.toContain("<html");
-    expect(message.length).toBeLessThan(5_000);
+    const result = await tool?.execute?.("call", { url: "https://example.com/missing" });
+    const details = result?.details as { status?: string; error?: string };
+    expect(details.status).toBe("error");
+    expect(details.error).toContain("Web fetch failed");
+    expect(details.error).toContain("404");
+    expect(details.error).toContain("Not Found");
+    expect(details.error).not.toContain("<html");
+    expect(details.error!.length).toBeLessThan(5_000);
  });

  it("strips HTML errors when content-type is missing", async () => {
@ -265,8 +268,32 @@ describe("web_fetch extraction fallbacks", () => {
      sandboxed: false,
    });

-    await expect(tool?.execute?.("call", { url: "https://example.com/oops" })).rejects.toThrow(
-      /Web fetch failed \(500\):.*Oops/,
-    );
+    const result = await tool?.execute?.("call", { url: "https://example.com/oops" });
+    const details = result?.details as { status?: string; error?: string };
+    expect(details.status).toBe("error");
+    expect(details.error).toMatch(/Web fetch failed.*500.*Oops/);
+  });
+
+  it("returns error result for network-level TypeError instead of crashing", async () => {
+    const mockFetch = vi.fn(() => Promise.reject(new TypeError("fetch failed")));
+    // @ts-expect-error mock fetch
+    global.fetch = mockFetch;
+
+    const tool = createWebFetchTool({
+      config: {
+        tools: {
+          web: {
+            fetch: { cacheTtlMinutes: 0, firecrawl: { enabled: false } },
+          },
+        },
+      },
+      sandboxed: false,
+    });
+
+    const result = await tool?.execute?.("call", { url: "https://unreachable.example.com/" });
+    const details = result?.details as { status?: string; error?: string; url?: string };
+    expect(details.status).toBe("error");
+    expect(details.error).toContain("fetch failed");
+    expect(details.url).toBe("https://unreachable.example.com/");
  });
 });
--- a/src/hooks/llm-slug-generator.ts
+++ b/src/hooks/llm-slug-generator.ts
@ -12,6 +12,7 @@ import {
  resolveAgentWorkspaceDir,
  resolveAgentDir,
 } from "../agents/agent-scope.js";
+import { resolveDefaultModelForAgent } from "../agents/model-selection.js";

 /**
 * Generate a short 1-2 word filename slug from session content using LLM
@ -38,6 +39,11 @@ ${params.sessionContent.slice(0, 2000)}

 Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design", "bug-fix"`;

+    // Resolve user's configured default model instead of hardcoded Opus
+    const { provider, model } = resolveDefaultModelForAgent({
+      cfg: params.cfg,
+    });
+
    const result = await runEmbeddedPiAgent({
      sessionId: `slug-generator-${Date.now()}`,
      sessionKey: "temp:slug-generator",
@ -46,6 +52,8 @@ Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design",
      agentDir,
      config: params.cfg,
      prompt,
+      provider,
+      model,
      timeoutMs: 15_000, // 15 second timeout
      runId: `slug-gen-${Date.now()}`,
    });
@ -75,7 +83,10 @@ Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design",
    // Clean up temporary session file
    if (tempSessionFile) {
      try {
-        await fs.rm(path.dirname(tempSessionFile), { recursive: true, force: true });
+        await fs.rm(path.dirname(tempSessionFile), {
+          recursive: true,
+          force: true,
+        });
      } catch {
        // Ignore cleanup errors
      }