refactor(effect): keep read path handling in app filesystem

Move the repaired Windows path normalization and not-found handling back behind AppFileSystem helpers so the read tool stays on the service abstraction end-to-end. Keep external-directory checks on the same path helper family for consistency.
pull/21016/head
Kit Langton 2026-04-04 12:14:11 -04:00
parent b15f1593c0
commit baff53b759
3 changed files with 36 additions and 22 deletions

View File

@ -188,13 +188,23 @@ export namespace AppFileSystem {
export function normalizePath(p: string): string { export function normalizePath(p: string): string {
if (process.platform !== "win32") return p if (process.platform !== "win32") return p
const resolved = pathResolve(windowsPath(p))
try { try {
return realpathSync.native(p) return realpathSync.native(resolved)
} catch { } catch {
return p return resolved
} }
} }
export function normalizePathPattern(p: string): string {
if (process.platform !== "win32") return p
if (p === "*") return p
const match = p.match(/^(.*)[\\/]\*$/)
if (!match) return normalizePath(p)
const dir = /^[A-Za-z]:$/.test(match[1]) ? match[1] + "\\" : match[1]
return join(normalizePath(dir), "*")
}
export function resolve(p: string): string { export function resolve(p: string): string {
const resolved = pathResolve(windowsPath(p)) const resolved = pathResolve(windowsPath(p))
try { try {

View File

@ -1,7 +1,8 @@
import path from "path" import path from "path"
import { Effect } from "effect"
import type { Tool } from "./tool" import type { Tool } from "./tool"
import { Instance } from "../project/instance" import { Instance } from "../project/instance"
import { Filesystem } from "@/util/filesystem" import { AppFileSystem } from "../filesystem"
type Kind = "file" | "directory" type Kind = "file" | "directory"
@ -15,14 +16,14 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string
if (options?.bypass) return if (options?.bypass) return
const full = process.platform === "win32" ? Filesystem.normalizePath(target) : target const full = process.platform === "win32" ? AppFileSystem.normalizePath(target) : target
if (Instance.containsPath(full)) return if (Instance.containsPath(full)) return
const kind = options?.kind ?? "file" const kind = options?.kind ?? "file"
const dir = kind === "directory" ? full : path.dirname(full) const dir = kind === "directory" ? full : path.dirname(full)
const glob = const glob =
process.platform === "win32" process.platform === "win32"
? Filesystem.normalizePathPattern(path.join(dir, "*")) ? AppFileSystem.normalizePathPattern(path.join(dir, "*"))
: path.join(dir, "*").replaceAll("\\", "/") : path.join(dir, "*").replaceAll("\\", "/")
await ctx.ask({ await ctx.ask({
@ -35,3 +36,11 @@ export async function assertExternalDirectory(ctx: Tool.Context, target?: string
}, },
}) })
} }
export const assertExternalDirectoryEffect = Effect.fn("Tool.assertExternalDirectory")(function* (
ctx: Tool.Context,
target?: string,
options?: Options,
) {
yield* Effect.promise(() => assertExternalDirectory(ctx, target, options))
})

View File

@ -10,7 +10,7 @@ import { LSP } from "../lsp"
import { FileTime } from "../file/time" import { FileTime } from "../file/time"
import DESCRIPTION from "./read.txt" import DESCRIPTION from "./read.txt"
import { Instance } from "../project/instance" import { Instance } from "../project/instance"
import { assertExternalDirectory } from "./external-directory" import { assertExternalDirectoryEffect } from "./external-directory"
import { Instruction } from "../session/instruction" import { Instruction } from "../session/instruction"
const DEFAULT_READ_LIMIT = 2000 const DEFAULT_READ_LIMIT = 2000
@ -96,15 +96,18 @@ export const ReadTool = Tool.defineEffect(
} }
const title = path.relative(Instance.worktree, filepath) const title = path.relative(Instance.worktree, filepath)
const stat = yield* fs.stat(filepath).pipe(Effect.catch(() => Effect.succeed(undefined))) const stat = yield* fs.stat(filepath).pipe(
Effect.catchIf(
yield* Effect.promise(() => (err) => "reason" in err && err.reason._tag === "NotFound",
assertExternalDirectory(ctx, filepath, { () => Effect.succeed(undefined),
bypass: Boolean(ctx.extra?.["bypassCwdCheck"]), ),
kind: stat?.type === "Directory" ? "directory" : "file",
}),
) )
yield* assertExternalDirectoryEffect(ctx, filepath, {
bypass: Boolean(ctx.extra?.["bypassCwdCheck"]),
kind: stat?.type === "Directory" ? "directory" : "file",
})
yield* Effect.promise(() => yield* Effect.promise(() =>
ctx.ask({ ctx.ask({
permission: "read", permission: "read",
@ -218,15 +221,7 @@ export const ReadTool = Tool.defineEffect(
description: DESCRIPTION, description: DESCRIPTION,
parameters, parameters,
async execute(params: z.infer<typeof parameters>, ctx) { async execute(params: z.infer<typeof parameters>, ctx) {
return Effect.runPromise( return Effect.runPromise(run(params, ctx).pipe(Effect.orDie))
run(params, ctx).pipe(
Effect.catch((err) =>
Effect.sync(() => {
throw err
}),
),
),
)
}, },
} }
}), }),