Преглед изворни кода

refactor(tool): convert webfetch tool internals to Effect (#21809)

Kit Langton пре 5 месеци
родитељ
комит
e83404367c

+ 5 - 1
packages/opencode/src/tool/registry.ts

@@ -30,6 +30,7 @@ import { Glob } from "../util/glob"
 import path from "path"
 import { pathToFileURL } from "url"
 import { Effect, Layer, ServiceMap } from "effect"
+import { FetchHttpClient, HttpClient } from "effect/unstable/http"
 import { InstanceState } from "@/effect/instance-state"
 import { makeRuntime } from "@/effect/run-service"
 import { Env } from "../env"
@@ -84,6 +85,7 @@ export namespace ToolRegistry {
     | FileTime.Service
     | Instruction.Service
     | AppFileSystem.Service
+    | HttpClient.HttpClient
   > = Layer.effect(
     Service,
     Effect.gen(function* () {
@@ -98,6 +100,7 @@ export namespace ToolRegistry {
       const todo = yield* TodoWriteTool
       const lsptool = yield* LspTool
       const plan = yield* PlanExitTool
+      const webfetch = yield* WebFetchTool
 
       const state = yield* InstanceState.make<State>(
         Effect.fn("ToolRegistry.state")(function* (ctx) {
@@ -163,7 +166,7 @@ export namespace ToolRegistry {
             edit: Tool.init(EditTool),
             write: Tool.init(WriteTool),
             task: Tool.init(task),
-            fetch: Tool.init(WebFetchTool),
+            fetch: Tool.init(webfetch),
             todo: Tool.init(todo),
             search: Tool.init(WebSearchTool),
             code: Tool.init(CodeSearchTool),
@@ -309,6 +312,7 @@ export namespace ToolRegistry {
       Layer.provide(FileTime.defaultLayer),
       Layer.provide(Instruction.defaultLayer),
       Layer.provide(AppFileSystem.defaultLayer),
+      Layer.provide(FetchHttpClient.layer),
     ),
   )
 

+ 142 - 148
packages/opencode/src/tool/webfetch.ts

@@ -1,169 +1,163 @@
 import z from "zod"
+import { Effect } from "effect"
+import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"
 import { Tool } from "./tool"
 import TurndownService from "turndown"
 import DESCRIPTION from "./webfetch.txt"
-import { abortAfterAny } from "../util/abort"
-import { iife } from "@/util/iife"
 
 const MAX_RESPONSE_SIZE = 5 * 1024 * 1024 // 5MB
 const DEFAULT_TIMEOUT = 30 * 1000 // 30 seconds
 const MAX_TIMEOUT = 120 * 1000 // 2 minutes
 
-export const WebFetchTool = Tool.define("webfetch", {
-  description: DESCRIPTION,
-  parameters: z.object({
-    url: z.string().describe("The URL to fetch content from"),
-    format: z
-      .enum(["text", "markdown", "html"])
-      .default("markdown")
-      .describe("The format to return the content in (text, markdown, or html). Defaults to markdown."),
-    timeout: z.number().describe("Optional timeout in seconds (max 120)").optional(),
-  }),
-  async execute(params, ctx) {
-    // Validate URL
-    if (!params.url.startsWith("http://") && !params.url.startsWith("https://")) {
-      throw new Error("URL must start with http:// or https://")
-    }
-
-    await ctx.ask({
-      permission: "webfetch",
-      patterns: [params.url],
-      always: ["*"],
-      metadata: {
-        url: params.url,
-        format: params.format,
-        timeout: params.timeout,
-      },
-    })
-
-    const timeout = Math.min((params.timeout ?? DEFAULT_TIMEOUT / 1000) * 1000, MAX_TIMEOUT)
-
-    const { signal, clearTimeout } = abortAfterAny(timeout, ctx.abort)
-
-    // Build Accept header based on requested format with q parameters for fallbacks
-    let acceptHeader = "*/*"
-    switch (params.format) {
-      case "markdown":
-        acceptHeader = "text/markdown;q=1.0, text/x-markdown;q=0.9, text/plain;q=0.8, text/html;q=0.7, */*;q=0.1"
-        break
-      case "text":
-        acceptHeader = "text/plain;q=1.0, text/markdown;q=0.9, text/html;q=0.8, */*;q=0.1"
-        break
-      case "html":
-        acceptHeader = "text/html;q=1.0, application/xhtml+xml;q=0.9, text/plain;q=0.8, text/markdown;q=0.7, */*;q=0.1"
-        break
-      default:
-        acceptHeader =
-          "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8"
-    }
-    const headers = {
-      "User-Agent":
-        "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/143.0.0.0 Safari/537.36",
-      Accept: acceptHeader,
-      "Accept-Language": "en-US,en;q=0.9",
-    }
-
-    const response = await iife(async () => {
-      try {
-        const initial = await fetch(params.url, { signal, headers })
-
-        // Retry with honest UA if blocked by Cloudflare bot detection (TLS fingerprint mismatch)
-        return initial.status === 403 && initial.headers.get("cf-mitigated") === "challenge"
-          ? await fetch(params.url, { signal, headers: { ...headers, "User-Agent": "opencode" } })
-          : initial
-      } finally {
-        clearTimeout()
-      }
-    })
-
-    if (!response.ok) {
-      throw new Error(`Request failed with status code: ${response.status}`)
-    }
-
-    // Check content length
-    const contentLength = response.headers.get("content-length")
-    if (contentLength && parseInt(contentLength) > MAX_RESPONSE_SIZE) {
-      throw new Error("Response too large (exceeds 5MB limit)")
-    }
+const parameters = z.object({
+  url: z.string().describe("The URL to fetch content from"),
+  format: z
+    .enum(["text", "markdown", "html"])
+    .default("markdown")
+    .describe("The format to return the content in (text, markdown, or html). Defaults to markdown."),
+  timeout: z.number().describe("Optional timeout in seconds (max 120)").optional(),
+})
 
-    const arrayBuffer = await response.arrayBuffer()
-    if (arrayBuffer.byteLength > MAX_RESPONSE_SIZE) {
-      throw new Error("Response too large (exceeds 5MB limit)")
-    }
+export const WebFetchTool = Tool.defineEffect(
+  "webfetch",
+  Effect.gen(function* () {
+    const http = yield* HttpClient.HttpClient
+    const httpOk = HttpClient.filterStatusOk(http)
+
+    return {
+      description: DESCRIPTION,
+      parameters,
+      execute: (params: z.infer<typeof parameters>, ctx: Tool.Context) =>
+        Effect.gen(function* () {
+          if (!params.url.startsWith("http://") && !params.url.startsWith("https://")) {
+            throw new Error("URL must start with http:// or https://")
+          }
 
-    const contentType = response.headers.get("content-type") || ""
-    const mime = contentType.split(";")[0]?.trim().toLowerCase() || ""
-    const title = `${params.url} (${contentType})`
-
-    // Check if response is an image
-    const isImage = mime.startsWith("image/") && mime !== "image/svg+xml" && mime !== "image/vnd.fastbidsheet"
-
-    if (isImage) {
-      const base64Content = Buffer.from(arrayBuffer).toString("base64")
-      return {
-        title,
-        output: "Image fetched successfully",
-        metadata: {},
-        attachments: [
-          {
-            type: "file",
-            mime,
-            url: `data:${mime};base64,${base64Content}`,
-          },
-        ],
-      }
-    }
+          yield* Effect.promise(() =>
+            ctx.ask({
+              permission: "webfetch",
+              patterns: [params.url],
+              always: ["*"],
+              metadata: {
+                url: params.url,
+                format: params.format,
+                timeout: params.timeout,
+              },
+            }),
+          )
+
+          const timeout = Math.min((params.timeout ?? DEFAULT_TIMEOUT / 1000) * 1000, MAX_TIMEOUT)
+
+          // Build Accept header based on requested format with q parameters for fallbacks
+          let acceptHeader = "*/*"
+          switch (params.format) {
+            case "markdown":
+              acceptHeader =
+                "text/markdown;q=1.0, text/x-markdown;q=0.9, text/plain;q=0.8, text/html;q=0.7, */*;q=0.1"
+              break
+            case "text":
+              acceptHeader = "text/plain;q=1.0, text/markdown;q=0.9, text/html;q=0.8, */*;q=0.1"
+              break
+            case "html":
+              acceptHeader =
+                "text/html;q=1.0, application/xhtml+xml;q=0.9, text/plain;q=0.8, text/markdown;q=0.7, */*;q=0.1"
+              break
+            default:
+              acceptHeader =
+                "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8"
+          }
+          const headers = {
+            "User-Agent":
+              "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/143.0.0.0 Safari/537.36",
+            Accept: acceptHeader,
+            "Accept-Language": "en-US,en;q=0.9",
+          }
 
-    const content = new TextDecoder().decode(arrayBuffer)
-
-    // Handle content based on requested format and actual content type
-    switch (params.format) {
-      case "markdown":
-        if (contentType.includes("text/html")) {
-          const markdown = convertHTMLToMarkdown(content)
-          return {
-            output: markdown,
-            title,
-            metadata: {},
+          const request = HttpClientRequest.get(params.url).pipe(HttpClientRequest.setHeaders(headers))
+
+          // Retry with honest UA if blocked by Cloudflare bot detection (TLS fingerprint mismatch)
+          const response = yield* httpOk.execute(request).pipe(
+            Effect.catchIf(
+              (err) =>
+                err.reason._tag === "StatusCodeError" &&
+                err.reason.response.status === 403 &&
+                err.reason.response.headers["cf-mitigated"] === "challenge",
+              () =>
+                httpOk.execute(
+                  HttpClientRequest.get(params.url).pipe(
+                    HttpClientRequest.setHeaders({ ...headers, "User-Agent": "opencode" }),
+                  ),
+                ),
+            ),
+            Effect.timeoutOrElse({ duration: timeout, orElse: () => Effect.die(new Error("Request timed out")) }),
+          )
+
+          // Check content length
+          const contentLength = response.headers["content-length"]
+          if (contentLength && parseInt(contentLength) > MAX_RESPONSE_SIZE) {
+            throw new Error("Response too large (exceeds 5MB limit)")
           }
-        }
-        return {
-          output: content,
-          title,
-          metadata: {},
-        }
 
-      case "text":
-        if (contentType.includes("text/html")) {
-          const text = await extractTextFromHTML(content)
-          return {
-            output: text,
-            title,
-            metadata: {},
+          const arrayBuffer = yield* response.arrayBuffer
+          if (arrayBuffer.byteLength > MAX_RESPONSE_SIZE) {
+            throw new Error("Response too large (exceeds 5MB limit)")
           }
-        }
-        return {
-          output: content,
-          title,
-          metadata: {},
-        }
 
-      case "html":
-        return {
-          output: content,
-          title,
-          metadata: {},
-        }
+          const contentType = response.headers["content-type"] || ""
+          const mime = contentType.split(";")[0]?.trim().toLowerCase() || ""
+          const title = `${params.url} (${contentType})`
+
+          // Check if response is an image
+          const isImage = mime.startsWith("image/") && mime !== "image/svg+xml" && mime !== "image/vnd.fastbidsheet"
+
+          if (isImage) {
+            const base64Content = Buffer.from(arrayBuffer).toString("base64")
+            return {
+              title,
+              output: "Image fetched successfully",
+              metadata: {},
+              attachments: [
+                {
+                  type: "file" as const,
+                  mime,
+                  url: `data:${mime};base64,${base64Content}`,
+                },
+              ],
+            }
+          }
 
-      default:
-        return {
-          output: content,
-          title,
-          metadata: {},
-        }
+          const content = new TextDecoder().decode(arrayBuffer)
+
+          // Handle content based on requested format and actual content type
+          switch (params.format) {
+            case "markdown":
+              if (contentType.includes("text/html")) {
+                const markdown = convertHTMLToMarkdown(content)
+                return {
+                  output: markdown,
+                  title,
+                  metadata: {},
+                }
+              }
+              return { output: content, title, metadata: {} }
+
+            case "text":
+              if (contentType.includes("text/html")) {
+                const text = yield* Effect.promise(() => extractTextFromHTML(content))
+                return { output: text, title, metadata: {} }
+              }
+              return { output: content, title, metadata: {} }
+
+            case "html":
+              return { output: content, title, metadata: {} }
+
+            default:
+              return { output: content, title, metadata: {} }
+          }
+        }).pipe(Effect.runPromise),
     }
-  },
-})
+  }),
+)
 
 async function extractTextFromHTML(html: string) {
   let text = ""

+ 7 - 1
packages/opencode/test/memory/abort-leak.test.ts

@@ -1,5 +1,7 @@
 import { describe, test, expect } from "bun:test"
 import path from "path"
+import { Effect } from "effect"
+import { FetchHttpClient } from "effect/unstable/http"
 import { Instance } from "../../src/project/instance"
 import { WebFetchTool } from "../../src/tool/webfetch"
 import { SessionID, MessageID } from "../../src/session/schema"
@@ -30,7 +32,11 @@ describe("memory: abort controller leak", () => {
     await Instance.provide({
       directory: projectRoot,
       fn: async () => {
-        const tool = await WebFetchTool.init()
+        const tool = await WebFetchTool.pipe(
+          Effect.flatMap((info) => Effect.promise(() => info.init())),
+          Effect.provide(FetchHttpClient.layer),
+          Effect.runPromise,
+        )
 
         // Warm up
         await tool.execute({ url: "https://example.com", format: "text" }, ctx).catch(() => {})

+ 2 - 0
packages/opencode/test/session/prompt-effect.test.ts

@@ -1,6 +1,7 @@
 import { NodeFileSystem } from "@effect/platform-node"
 import { expect } from "bun:test"
 import { Cause, Effect, Exit, Fiber, Layer } from "effect"
+import { FetchHttpClient } from "effect/unstable/http"
 import path from "path"
 import z from "zod"
 import { Agent as AgentSvc } from "../../src/agent/agent"
@@ -169,6 +170,7 @@ function makeHttp() {
   const todo = Todo.layer.pipe(Layer.provideMerge(deps))
   const registry = ToolRegistry.layer.pipe(
     Layer.provide(Skill.defaultLayer),
+    Layer.provide(FetchHttpClient.layer),
     Layer.provideMerge(todo),
     Layer.provideMerge(question),
     Layer.provideMerge(deps),

+ 3 - 2
packages/opencode/test/session/snapshot-tool-race.test.ts

@@ -12,7 +12,8 @@
  * tools internally during multi-step processing before emitting events.
  */
 import { expect } from "bun:test"
-import { Effect } from "effect"
+import { Effect, Layer } from "effect"
+import { FetchHttpClient } from "effect/unstable/http"
 import fs from "fs/promises"
 import path from "path"
 import { Session } from "../../src/session"
@@ -28,7 +29,6 @@ import { TestLLMServer } from "../lib/llm-server"
 
 // Same layer setup as prompt-effect.test.ts
 import { NodeFileSystem } from "@effect/platform-node"
-import { Layer } from "effect"
 import { Agent as AgentSvc } from "../../src/agent/agent"
 import { Bus } from "../../src/bus"
 import { Command } from "../../src/command"
@@ -134,6 +134,7 @@ function makeHttp() {
   const todo = Todo.layer.pipe(Layer.provideMerge(deps))
   const registry = ToolRegistry.layer.pipe(
     Layer.provide(Skill.defaultLayer),
+    Layer.provide(FetchHttpClient.layer),
     Layer.provideMerge(todo),
     Layer.provideMerge(question),
     Layer.provideMerge(deps),

+ 13 - 3
packages/opencode/test/tool/webfetch.test.ts

@@ -1,5 +1,7 @@
 import { describe, expect, test } from "bun:test"
 import path from "path"
+import { Effect } from "effect"
+import { FetchHttpClient } from "effect/unstable/http"
 import { Instance } from "../../src/project/instance"
 import { WebFetchTool } from "../../src/tool/webfetch"
 import { SessionID, MessageID } from "../../src/session/schema"
@@ -22,6 +24,14 @@ async function withFetch(fetch: (req: Request) => Response | Promise<Response>,
   await fn(server.url)
 }
 
+function initTool() {
+  return WebFetchTool.pipe(
+    Effect.flatMap((info) => Effect.promise(() => info.init())),
+    Effect.provide(FetchHttpClient.layer),
+    Effect.runPromise,
+  )
+}
+
 describe("tool.webfetch", () => {
   test("returns image responses as file attachments", async () => {
     const bytes = new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10])
@@ -31,7 +41,7 @@ describe("tool.webfetch", () => {
         await Instance.provide({
           directory: projectRoot,
           fn: async () => {
-            const webfetch = await WebFetchTool.init()
+            const webfetch = await initTool()
             const result = await webfetch.execute(
               { url: new URL("/image.png", url).toString(), format: "markdown" },
               ctx,
@@ -63,7 +73,7 @@ describe("tool.webfetch", () => {
         await Instance.provide({
           directory: projectRoot,
           fn: async () => {
-            const webfetch = await WebFetchTool.init()
+            const webfetch = await initTool()
             const result = await webfetch.execute({ url: new URL("/image.svg", url).toString(), format: "html" }, ctx)
             expect(result.output).toContain("<svg")
             expect(result.attachments).toBeUndefined()
@@ -84,7 +94,7 @@ describe("tool.webfetch", () => {
         await Instance.provide({
           directory: projectRoot,
           fn: async () => {
-            const webfetch = await WebFetchTool.init()
+            const webfetch = await initTool()
             const result = await webfetch.execute({ url: new URL("/file.txt", url).toString(), format: "text" }, ctx)
             expect(result.output).toBe("hello from webfetch")
             expect(result.attachments).toBeUndefined()