From d9a07b5d966d6ff00d5336da15c1b312e0c315a7 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 3 Apr 2026 22:00:59 -0400 Subject: [PATCH] refactor(effect): effectify read tool execution Move the read tool body onto a named Effect path and keep a single Promise bridge at execute(). This keeps the service graph wiring from the previous commit while reducing runPromise islands inside the tool implementation. --- packages/opencode/src/tool/read.ts | 407 +++++++++++++++-------------- 1 file changed, 215 insertions(+), 192 deletions(-) diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index e9890c91a1..6a913549c5 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -34,225 +34,248 @@ export const ReadTool = Tool.defineEffect( const lsp = yield* LSP.Service const time = yield* FileTime.Service - return { - description: DESCRIPTION, - parameters, - async execute(params: z.infer, ctx) { - if (params.offset !== undefined && params.offset < 1) { - throw new Error("offset must be greater than or equal to 1") - } - let filepath = params.filePath - if (!path.isAbsolute(filepath)) { - filepath = path.resolve(Instance.directory, filepath) - } - if (process.platform === "win32") { - filepath = Filesystem.normalizePath(filepath) - } - const title = path.relative(Instance.worktree, filepath) + const miss = Effect.fn("ReadTool.miss")(function* (filepath: string) { + const dir = path.dirname(filepath) + const base = path.basename(filepath) + const items = yield* fs.readDirectory(dir).pipe( + Effect.map((items) => + items + .filter( + (item) => + item.toLowerCase().includes(base.toLowerCase()) || base.toLowerCase().includes(item.toLowerCase()), + ) + .map((item) => path.join(dir, item)) + .slice(0, 3), + ), + Effect.catch(() => Effect.succeed([] as string[])), + ) - const stat = await Effect.runPromise(fs.stat(filepath).pipe(Effect.catch(() => Effect.succeed(undefined)))) + if (items.length > 0) { + return yield* Effect.fail( + new Error(`File not found: ${filepath}\n\nDid you mean one of these?\n${items.join("\n")}`), + ) + } - await assertExternalDirectory(ctx, filepath, { + return yield* Effect.fail(new Error(`File not found: ${filepath}`)) + }) + + const list = Effect.fn("ReadTool.list")(function* (filepath: string) { + const items = yield* fs.readDirectoryEntries(filepath) + return yield* Effect.forEach( + items, + Effect.fnUntraced(function* (item) { + if (item.type === "directory") return item.name + "/" + if (item.type !== "symlink") return item.name + + const target = yield* fs + .stat(path.join(filepath, item.name)) + .pipe(Effect.catch(() => Effect.succeed(undefined))) + if (target?.type === "Directory") return item.name + "/" + return item.name + }), + { concurrency: "unbounded" }, + ).pipe(Effect.map((items: string[]) => items.sort((a, b) => a.localeCompare(b)))) + }) + + const warm = Effect.fn("ReadTool.warm")(function* (filepath: string, sessionID: Tool.Context["sessionID"]) { + yield* lsp.touchFile(filepath, false) + yield* time.read(sessionID, filepath) + }) + + const run = Effect.fn("ReadTool.execute")(function* (params: z.infer, ctx: Tool.Context) { + if (params.offset !== undefined && params.offset < 1) { + return yield* Effect.fail(new Error("offset must be greater than or equal to 1")) + } + + let filepath = params.filePath + if (!path.isAbsolute(filepath)) { + filepath = path.resolve(Instance.directory, filepath) + } + if (process.platform === "win32") { + filepath = Filesystem.normalizePath(filepath) + } + const title = path.relative(Instance.worktree, filepath) + + const stat = yield* fs.stat(filepath).pipe(Effect.catch(() => Effect.succeed(undefined))) + + yield* Effect.promise(() => + assertExternalDirectory(ctx, filepath, { bypass: Boolean(ctx.extra?.["bypassCwdCheck"]), kind: stat?.type === "Directory" ? "directory" : "file", - }) + }), + ) - await ctx.ask({ + yield* Effect.promise(() => + ctx.ask({ permission: "read", patterns: [filepath], always: ["*"], metadata: {}, - }) + }), + ) - if (!stat) { - const dir = path.dirname(filepath) - const base = path.basename(filepath) + if (!stat) return yield* miss(filepath) - const suggestions = await Effect.runPromise( - fs.readDirectory(dir).pipe( - Effect.map((entries) => - entries - .filter( - (entry) => - entry.toLowerCase().includes(base.toLowerCase()) || - base.toLowerCase().includes(entry.toLowerCase()), - ) - .map((entry) => path.join(dir, entry)) - .slice(0, 3), - ), - Effect.catch(() => Effect.succeed([] as string[])), - ), - ) + if (stat.type === "Directory") { + const items = yield* list(filepath) + const limit = params.limit ?? DEFAULT_READ_LIMIT + const offset = params.offset ?? 1 + const start = offset - 1 + const sliced = items.slice(start, start + limit) + const truncated = start + sliced.length < items.length - if (suggestions.length > 0) { - throw new Error(`File not found: ${filepath}\n\nDid you mean one of these?\n${suggestions.join("\n")}`) - } - - throw new Error(`File not found: ${filepath}`) - } - - if (stat.type === "Directory") { - const dirents = await Effect.runPromise(fs.readDirectoryEntries(filepath)) - const entries = await Promise.all( - dirents.map(async (dirent) => { - if (dirent.type === "directory") return dirent.name + "/" - if (dirent.type === "symlink") { - const target = await Effect.runPromise( - fs.stat(path.join(filepath, dirent.name)).pipe(Effect.catch(() => Effect.succeed(undefined))), - ) - if (target?.type === "Directory") return dirent.name + "/" - } - return dirent.name - }), - ) - entries.sort((a, b) => a.localeCompare(b)) - - const limit = params.limit ?? DEFAULT_READ_LIMIT - const offset = params.offset ?? 1 - const start = offset - 1 - const sliced = entries.slice(start, start + limit) - const truncated = start + sliced.length < entries.length - - const output = [ + return { + title, + output: [ `${filepath}`, `directory`, ``, sliced.join("\n"), truncated - ? `\n(Showing ${sliced.length} of ${entries.length} entries. Use 'offset' parameter to read beyond entry ${offset + sliced.length})` - : `\n(${entries.length} entries)`, + ? `\n(Showing ${sliced.length} of ${items.length} entries. Use 'offset' parameter to read beyond entry ${offset + sliced.length})` + : `\n(${items.length} entries)`, ``, - ].join("\n") - - return { - title, - output, - metadata: { - preview: sliced.slice(0, 20).join("\n"), - truncated, - loaded: [] as string[], - }, - } - } - - const instructions = await Effect.runPromise(instruction.resolve(ctx.messages, filepath, ctx.messageID)) - - // Exclude SVG (XML-based) and vnd.fastbidsheet (.fbs extension, commonly FlatBuffers schema files) - const mime = Filesystem.mimeType(filepath) - const isImage = mime.startsWith("image/") && mime !== "image/svg+xml" && mime !== "image/vnd.fastbidsheet" - const isPdf = mime === "application/pdf" - if (isImage || isPdf) { - const msg = `${isImage ? "Image" : "PDF"} read successfully` - return { - title, - output: msg, - metadata: { - preview: msg, - truncated: false, - loaded: instructions.map((i) => i.filepath), - }, - attachments: [ - { - type: "file", - mime, - url: `data:${mime};base64,${Buffer.from(await Effect.runPromise(fs.readFile(filepath))).toString("base64")}`, - }, - ], - } - } - - const isBinary = await isBinaryFile(filepath, Number(stat.size)) - if (isBinary) throw new Error(`Cannot read binary file: ${filepath}`) - - 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 limit = params.limit ?? DEFAULT_READ_LIMIT - const offset = params.offset ?? 1 - const start = offset - 1 - const raw: string[] = [] - let bytes = 0 - let lines = 0 - let truncatedByBytes = false - let hasMoreLines = false - try { - for await (const text of rl) { - lines += 1 - if (lines <= start) continue - - if (raw.length >= limit) { - hasMoreLines = 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) { - truncatedByBytes = true - hasMoreLines = true - break - } - - raw.push(line) - bytes += size - } - } finally { - rl.close() - stream.destroy() - } - - if (lines < offset && !(lines === 0 && offset === 1)) { - throw new Error(`Offset ${offset} is out of range for this file (${lines} lines)`) - } - - const content = raw.map((line, index) => { - return `${index + offset}: ${line}` - }) - const preview = raw.slice(0, 20).join("\n") - - let output = [`${filepath}`, `file`, ""].join("\n") - output += content.join("\n") - - const totalLines = lines - const lastReadLine = offset + raw.length - 1 - const nextOffset = lastReadLine + 1 - const truncated = hasMoreLines || truncatedByBytes - - if (truncatedByBytes) { - output += `\n\n(Output capped at ${MAX_BYTES_LABEL}. Showing lines ${offset}-${lastReadLine}. Use offset=${nextOffset} to continue.)` - } else if (hasMoreLines) { - output += `\n\n(Showing lines ${offset}-${lastReadLine} of ${totalLines}. Use offset=${nextOffset} to continue.)` - } else { - output += `\n\n(End of file - total ${totalLines} lines)` - } - output += "\n" - - await Effect.runPromise(lsp.touchFile(filepath, false)) - await Effect.runPromise(time.read(ctx.sessionID, filepath)) - - if (instructions.length > 0) { - output += `\n\n\n${instructions.map((i) => i.content).join("\n\n")}\n` - } - - return { - title, - output, + ].join("\n"), metadata: { - preview, + preview: sliced.slice(0, 20).join("\n"), truncated, - loaded: instructions.map((i) => i.filepath), + loaded: [] as string[], }, } + } + + const loaded = yield* instruction.resolve(ctx.messages, filepath, ctx.messageID) + + const mime = Filesystem.mimeType(filepath) + const isImage = mime.startsWith("image/") && mime !== "image/svg+xml" && mime !== "image/vnd.fastbidsheet" + const isPdf = mime === "application/pdf" + if (isImage || isPdf) { + const msg = `${isImage ? "Image" : "PDF"} read successfully` + return { + title, + output: msg, + metadata: { + preview: msg, + truncated: false, + loaded: loaded.map((item) => item.filepath), + }, + attachments: [ + { + type: "file" as const, + mime, + url: `data:${mime};base64,${Buffer.from(yield* fs.readFile(filepath)).toString("base64")}`, + }, + ], + } + } + + if (yield* Effect.promise(() => isBinaryFile(filepath, Number(stat.size)))) { + 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 }), + ) + 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)`), + ) + } + + let output = [`${filepath}`, `file`, ""].join("\n") + output += file.raw.map((line, i) => `${i + file.offset}: ${line}`).join("\n") + + const last = file.offset + file.raw.length - 1 + const next = last + 1 + const truncated = file.more || file.cut + if (file.cut) { + output += `\n\n(Output capped at ${MAX_BYTES_LABEL}. Showing lines ${file.offset}-${last}. Use offset=${next} to continue.)` + } else if (file.more) { + output += `\n\n(Showing lines ${file.offset}-${last} of ${file.count}. Use offset=${next} to continue.)` + } else { + output += `\n\n(End of file - total ${file.count} lines)` + } + output += "\n" + + yield* warm(filepath, ctx.sessionID) + + if (loaded.length > 0) { + output += `\n\n\n${loaded.map((item) => item.content).join("\n\n")}\n` + } + + return { + title, + output, + metadata: { + preview: file.raw.slice(0, 20).join("\n"), + truncated, + loaded: loaded.map((item) => item.filepath), + }, + } + }) + + return { + description: DESCRIPTION, + parameters, + async execute(params: z.infer, ctx) { + return Effect.runPromise( + run(params, ctx).pipe( + Effect.catch((err) => + Effect.sync(() => { + throw err + }), + ), + ), + ) }, } }), ) +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 } +} + async function isBinaryFile(filepath: string, fileSize: number): Promise { const ext = path.extname(filepath).toLowerCase() // binary check for common non-text extensions