effect(patch,tool): migrate patch/index and tool/read to AppFileSystem (#27155)

2026-05-13 20:32:19 -04:00
parent 3f33be1928
commit aa8a41d1b8
5 changed files with 268 additions and 236 deletions
--- a/packages/opencode/src/patch/index.ts
+++ b/packages/opencode/src/patch/index.ts
@@ -1,7 +1,6 @@
-import { Schema } from "effect"
+import { Effect, Schema } from "effect"
 import * as path from "path"
-import * as fs from "fs/promises"
-import { readFileSync } from "fs"
+import { AppFileSystem } from "@opencode-ai/core/filesystem"
 import * as Log from "@opencode-ai/core/util/log"
 import * as Bom from "../util/bom"

@@ -308,14 +307,12 @@ interface ApplyPatchFileUpdate {
  bom: boolean
 }

-export function deriveNewContentsFromChunks(filePath: string, chunks: UpdateFileChunk[]): ApplyPatchFileUpdate {
-  // Read original file content
-  let originalContent: ReturnType<typeof Bom.split>
-  try {
-    originalContent = Bom.split(readFileSync(filePath, "utf-8"))
-  } catch (error) {
-    throw new Error(`Failed to read file ${filePath}: ${error}`, { cause: error })
-  }
+export function deriveNewContentsFromChunks(
+  filePath: string,
+  chunks: UpdateFileChunk[],
+  originalText: string,
+): ApplyPatchFileUpdate {
+  const originalContent = Bom.split(originalText)

  let originalLines = originalContent.text.split("\n")

@@ -423,11 +420,11 @@ function applyReplacements(lines: string[], replacements: Array<[number, number,
 // Normalize Unicode punctuation to ASCII equivalents (like Rust's normalize_unicode)
 function normalizeUnicode(str: string): string {
  return str
-    .replace(/[\u2018\u2019\u201A\u201B]/g, "'") // single quotes
-    .replace(/[\u201C\u201D\u201E\u201F]/g, '"') // double quotes
-    .replace(/[\u2010\u2011\u2012\u2013\u2014\u2015]/g, "-") // dashes
-    .replace(/\u2026/g, "...") // ellipsis
-    .replace(/\u00A0/g, " ") // non-breaking space
+    .replace(/[‘’‚‛]/g, "'") // single quotes
+    .replace(/[“”„‟]/g, '"') // double quotes
+    .replace(/[‐‑‒–—―]/g, "-") // dashes
+    .replace(/…/g, "...") // ellipsis
+    .replace(/ /g, " ") // non-breaking space
 }

 type Comparator = (a: string, b: string) => boolean
@@ -517,77 +514,71 @@ function generateUnifiedDiff(oldContent: string, newContent: string): string {
 }

 // Apply hunks to filesystem
-export async function applyHunksToFiles(hunks: Hunk[]): Promise<AffectedPaths> {
+export const applyHunksToFiles = Effect.fn("Patch.applyHunksToFiles")(function* (hunks: Hunk[]) {
  if (hunks.length === 0) {
-    throw new Error("No files were modified.")
+    return yield* Effect.fail(new Error("No files were modified."))
  }

+  const fs = yield* AppFileSystem.Service
+
  const added: string[] = []
  const modified: string[] = []
  const deleted: string[] = []

  for (const hunk of hunks) {
    switch (hunk.type) {
-      case "add":
-        // Create parent directories
-        const addDir = path.dirname(hunk.path)
-        if (addDir !== "." && addDir !== "/") {
-          await fs.mkdir(addDir, { recursive: true })
-        }
-
-        await fs.writeFile(hunk.path, hunk.contents, "utf-8")
+      case "add": {
+        yield* fs.writeWithDirs(hunk.path, hunk.contents)
        added.push(hunk.path)
        log.info(`Added file: ${hunk.path}`)
        break
+      }

-      case "delete":
-        await fs.unlink(hunk.path)
+      case "delete": {
+        yield* fs.remove(hunk.path)
        deleted.push(hunk.path)
        log.info(`Deleted file: ${hunk.path}`)
        break
+      }

-      case "update":
-        const fileUpdate = deriveNewContentsFromChunks(hunk.path, hunk.chunks)
+      case "update": {
+        const originalText = yield* fs.readFileString(hunk.path)
+        const fileUpdate = deriveNewContentsFromChunks(hunk.path, hunk.chunks, originalText)

        if (hunk.move_path) {
-          // Handle file move
-          const moveDir = path.dirname(hunk.move_path)
-          if (moveDir !== "." && moveDir !== "/") {
-            await fs.mkdir(moveDir, { recursive: true })
-          }
-
-          await fs.writeFile(hunk.move_path, Bom.join(fileUpdate.content, fileUpdate.bom), "utf-8")
-          await fs.unlink(hunk.path)
+          yield* fs.writeWithDirs(hunk.move_path, Bom.join(fileUpdate.content, fileUpdate.bom))
+          yield* fs.remove(hunk.path)
          modified.push(hunk.move_path)
          log.info(`Moved file: ${hunk.path} -> ${hunk.move_path}`)
        } else {
-          // Regular update
-          await fs.writeFile(hunk.path, Bom.join(fileUpdate.content, fileUpdate.bom), "utf-8")
+          yield* fs.writeWithDirs(hunk.path, Bom.join(fileUpdate.content, fileUpdate.bom))
          modified.push(hunk.path)
          log.info(`Updated file: ${hunk.path}`)
        }
        break
+      }
    }
  }

-  return { added, modified, deleted }
-}
+  return { added, modified, deleted } satisfies AffectedPaths
+})

 // Main patch application function
-export async function applyPatch(patchText: string): Promise<AffectedPaths> {
+export const applyPatch = Effect.fn("Patch.applyPatch")(function* (patchText: string) {
  const { hunks } = parsePatch(patchText)
-  return applyHunksToFiles(hunks)
-}
+  return yield* applyHunksToFiles(hunks)
+})

-// Async version of maybeParseApplyPatchVerified
-export async function maybeParseApplyPatchVerified(
-  argv: string[],
-  cwd: string,
-): Promise<
+type MaybeApplyPatchVerifiedResult =
  | { type: MaybeApplyPatchVerified.Body; action: ApplyPatchAction }
  | { type: MaybeApplyPatchVerified.CorrectnessError; error: Error }
  | { type: MaybeApplyPatchVerified.NotApplyPatch }
-> {
+
+// Effectful verified-parse: needs AppFileSystem.Service to read existing files
+export const maybeParseApplyPatchVerified = Effect.fn("Patch.maybeParseApplyPatchVerified")(function* (
+  argv: string[],
+  cwd: string,
+) {
  // Detect implicit patch invocation (raw patch without apply_patch command)
  if (argv.length === 1) {
    try {
@@ -595,7 +586,7 @@ export async function maybeParseApplyPatchVerified(
      return {
        type: MaybeApplyPatchVerified.CorrectnessError,
        error: new Error(ApplyPatchError.ImplicitInvocation),
-      }
+      } satisfies MaybeApplyPatchVerifiedResult
    } catch {
      // Not a patch, continue
    }
@@ -604,8 +595,9 @@ export async function maybeParseApplyPatchVerified(
  const result = maybeParseApplyPatch(argv)

  switch (result.type) {
-    case MaybeApplyPatch.Body:
-      const { args } = result
+    case MaybeApplyPatch.Body: {
+      const fs = yield* AppFileSystem.Service
+      const args = result.args
      const effectiveCwd = args.workdir ? path.resolve(cwd, args.workdir) : cwd
      const changes = new Map<string, ApplyPatchFileChange>()

@@ -623,27 +615,37 @@ export async function maybeParseApplyPatchVerified(
            })
            break

-          case "delete":
-            // For delete, we need to read the current content
+          case "delete": {
            const deletePath = path.resolve(effectiveCwd, hunk.path)
-            try {
-              const content = await fs.readFile(deletePath, "utf-8")
-              changes.set(resolvedPath, {
-                type: "delete",
-                content,
-              })
-            } catch {
+            const content = yield* fs.readFileString(deletePath).pipe(Effect.catch(() => Effect.succeed(undefined)))
+            if (content === undefined) {
              return {
                type: MaybeApplyPatchVerified.CorrectnessError,
                error: new Error(`Failed to read file for deletion: ${deletePath}`),
-              }
+              } satisfies MaybeApplyPatchVerifiedResult
            }
+            changes.set(resolvedPath, {
+              type: "delete",
+              content,
+            })
            break
+          }

-          case "update":
+          case "update": {
            const updatePath = path.resolve(effectiveCwd, hunk.path)
+            const originalText = yield* fs.readFileString(updatePath).pipe(
+              Effect.catch((cause) =>
+                Effect.succeed(new Error(`Failed to read file ${updatePath}: ${cause}`, { cause })),
+              ),
+            )
+            if (originalText instanceof Error) {
+              return {
+                type: MaybeApplyPatchVerified.CorrectnessError,
+                error: originalText,
+              } satisfies MaybeApplyPatchVerifiedResult
+            }
            try {
-              const fileUpdate = deriveNewContentsFromChunks(updatePath, hunk.chunks)
+              const fileUpdate = deriveNewContentsFromChunks(updatePath, hunk.chunks, originalText)
              changes.set(resolvedPath, {
                type: "update",
                unified_diff: fileUpdate.unified_diff,
@@ -654,9 +656,10 @@ export async function maybeParseApplyPatchVerified(
              return {
                type: MaybeApplyPatchVerified.CorrectnessError,
                error: error as Error,
-              }
+              } satisfies MaybeApplyPatchVerifiedResult
            }
            break
+          }
        }
      }

@@ -667,17 +670,18 @@ export async function maybeParseApplyPatchVerified(
          patch: args.patch,
          cwd: effectiveCwd,
        },
-      }
+      } satisfies MaybeApplyPatchVerifiedResult
+    }

    case MaybeApplyPatch.PatchParseError:
      return {
        type: MaybeApplyPatchVerified.CorrectnessError,
        error: result.error,
-      }
+      } satisfies MaybeApplyPatchVerifiedResult

    case MaybeApplyPatch.NotApplyPatch:
-      return { type: MaybeApplyPatchVerified.NotApplyPatch }
+      return { type: MaybeApplyPatchVerified.NotApplyPatch } satisfies MaybeApplyPatchVerifiedResult
  }
-}
+})

 export * as Patch from "."
--- a/packages/opencode/src/tool/apply_patch.ts
+++ b/packages/opencode/src/tool/apply_patch.ts
@@ -119,7 +119,7 @@ export const ApplyPatchTool = Tool.define(

            // Apply the update chunks to get new content
            try {
-              const fileUpdate = Patch.deriveNewContentsFromChunks(filePath, hunk.chunks)
+              const fileUpdate = Patch.deriveNewContentsFromChunks(filePath, hunk.chunks, Bom.join(source.text, source.bom))
              newContent = fileUpdate.content
              bom = fileUpdate.bom
            } catch (error) {
--- a/packages/opencode/src/tool/read.ts
+++ b/packages/opencode/src/tool/read.ts
@@ -1,8 +1,6 @@
-import { Effect, Option, Schema, Scope } from "effect"
+import { Effect, Option, Schema, Scope, Stream } from "effect"
 import { NonNegativeInt } from "@opencode-ai/core/schema"
-import { createReadStream } from "fs"
 import * as path from "path"
-import { createInterface } from "readline"
 import * as Tool from "./tool"
 import { AppFileSystem } from "@opencode-ai/core/filesystem"
 import { LSP } from "@/lsp/lsp"
@@ -105,6 +103,49 @@ export const ReadTool = Tool.define(
      )
    })

+    const lines = Effect.fn("ReadTool.lines")(function* (filepath: string, opts: { limit: number; offset: number }) {
+      const start = opts.offset - 1
+      const raw: string[] = []
+      const flags = { bytes: 0, count: 0, cut: false, more: false, done: false }
+
+      // Note: prefer manual TextDecoder over Stream.decodeText — when the source stream
+      // ends without flushing, decodeText drops the final unterminated line. We also
+      // avoid Stream.runForEachWhile (it currently swallows the final unterminated
+      // line of the upstream splitLines pipeline) and instead toggle a `done` flag
+      // and ignore subsequent lines.
+      const decoder = new TextDecoder("utf-8")
+      yield* fs.stream(filepath).pipe(
+        Stream.map((bytes) => decoder.decode(bytes, { stream: true })),
+        Stream.splitLines,
+        Stream.runForEach((text) =>
+          Effect.sync(() => {
+            if (flags.done) return
+            flags.count += 1
+            if (flags.count <= start) return
+
+            if (raw.length >= opts.limit) {
+              flags.more = true
+              return
+            }
+
+            const line = text.length > MAX_LINE_LENGTH ? text.substring(0, MAX_LINE_LENGTH) + MAX_LINE_SUFFIX : text
+            const size = Buffer.byteLength(line, "utf-8") + (raw.length > 0 ? 1 : 0)
+            if (flags.bytes + size > MAX_BYTES) {
+              flags.cut = true
+              flags.more = true
+              flags.done = true
+              return
+            }
+
+            raw.push(line)
+            flags.bytes += size
+          }),
+        ),
+      )
+
+      return { raw, count: flags.count, cut: flags.cut, more: flags.more, offset: opts.offset }
+    })
+
    const isBinaryFile = (filepath: string, bytes: Uint8Array) => {
      const ext = path.extname(filepath).toLowerCase()
      switch (ext) {
@@ -247,9 +288,7 @@ export const ReadTool = Tool.define(
        return yield* Effect.fail(new Error(`Cannot read binary file: ${filepath}`))
      }

-      const file = yield* Effect.promise(() =>
-        lines(filepath, { limit: params.limit ?? DEFAULT_READ_LIMIT, offset: params.offset || 1 }),
-      )
+      const file = yield* lines(filepath, { limit: params.limit ?? DEFAULT_READ_LIMIT, offset: params.offset || 1 })
      if (file.count < file.offset && !(file.count === 0 && file.offset === 1)) {
        return yield* Effect.fail(
          new Error(`Offset ${file.offset} is out of range for this file (${file.count} lines)`),
@@ -296,47 +335,3 @@ export const ReadTool = Tool.define(
    }
  }),
 )
-
-async function lines(filepath: string, opts: { limit: number; offset: number }) {
-  const stream = createReadStream(filepath, { encoding: "utf8" })
-  const rl = createInterface({
-    input: stream,
-    // Note: we use the crlfDelay option to recognize all instances of CR LF
-    // ('\r\n') in file as a single line break.
-    crlfDelay: Infinity,
-  })
-
-  const start = opts.offset - 1
-  const raw: string[] = []
-  let bytes = 0
-  let count = 0
-  let cut = false
-  let more = false
-  try {
-    for await (const text of rl) {
-      count += 1
-      if (count <= start) continue
-
-      if (raw.length >= opts.limit) {
-        more = true
-        continue
-      }
-
-      const line = text.length > MAX_LINE_LENGTH ? text.substring(0, MAX_LINE_LENGTH) + MAX_LINE_SUFFIX : text
-      const size = Buffer.byteLength(line, "utf-8") + (raw.length > 0 ? 1 : 0)
-      if (bytes + size > MAX_BYTES) {
-        cut = true
-        more = true
-        break
-      }
-
-      raw.push(line)
-      bytes += size
-    }
-  } finally {
-    rl.close()
-    stream.destroy()
-  }
-
-  return { raw, count, cut, more, offset: opts.offset }
-}