feat(core): improve read model output (#39146)

This commit is contained in:
Aiden Cline 2026-07-27 14:52:50 -05:00 committed by GitHub
commit 6da2f3c38f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 147 additions and 54 deletions

View file

@ -5,12 +5,13 @@ import { Config } from "@opencode-ai/core/config"
import { ConfigAttachments } from "@opencode-ai/core/config/attachments"
import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
import { LayerNode } from "@opencode-ai/util/effect/layer-node"
import { FileSystem } from "@opencode-ai/core/filesystem"
import { FSUtil } from "@opencode-ai/util/fs-util"
import { Location } from "@opencode-ai/core/location"
import { Image } from "@opencode-ai/core/image"
import { Permission } from "@opencode-ai/core/permission"
import { Session } from "@opencode-ai/core/session"
import { AbsolutePath } from "@opencode-ai/core/schema"
import { AbsolutePath, RelativePath } from "@opencode-ai/core/schema"
import { Global } from "@opencode-ai/util/global"
import { LocationMutation } from "@opencode-ai/core/location-mutation"
import { location } from "./fixture/location"
@ -40,13 +41,23 @@ const readToolNode = makeLocationNode({
const assertions: Permission.AssertInput[] = []
const missingPath = "__missing_read_target__.txt"
const missingAbsolutePath = path.join(process.cwd(), missingPath)
const notFound = (target: string) =>
PlatformError.systemError({
_tag: "NotFound",
module: "FileSystem",
method: "stat",
pathOrDescriptor: target,
})
const readCalls: {
input: AbsolutePath
page: ReadToolFileSystem.PageInput
}[] = []
const listCalls: ReadToolFileSystem.PageInput[] = []
let listResult = new ReadToolFileSystem.ListPage({ type: "list-page", entries: [], truncated: false })
let resolvedType: "file" | "directory" = "file"
let resolveFailure: unknown
let inspectFailure: ReadToolFileSystem.InspectError | undefined
let directoryEntries: string[] = []
let readResult: ReadToolFileSystem.FileContent | ReadToolFileSystem.TextPage = {
type: "file",
uri: "file:///README.md",
@ -59,7 +70,12 @@ let readFailure: ReadToolFileSystem.ReadError | undefined
const reader = Layer.succeed(
ReadToolFileSystem.Service,
ReadToolFileSystem.Service.of({
inspect: () => (resolveFailure === undefined ? Effect.succeed(resolvedType) : Effect.die(resolveFailure)),
inspect: () =>
resolveFailure !== undefined
? Effect.die(resolveFailure)
: inspectFailure !== undefined
? Effect.fail(inspectFailure)
: Effect.succeed(resolvedType),
read: (input, _resource, page = {}) => {
readCalls.push({ input, page })
if (readFailure !== undefined) return Effect.fail(readFailure)
@ -68,7 +84,7 @@ const reader = Layer.succeed(
list: (_path, input = {}) =>
Effect.sync(() => {
listCalls.push(input)
return new ReadToolFileSystem.ListPage({ type: "list-page", entries: [], truncated: false })
return listResult
}),
}),
)
@ -107,6 +123,7 @@ const testFileSystem = Layer.effect(
Effect.succeed(
FSUtil.Service.of({
...fs,
readDirectory: () => Effect.succeed(directoryEntries),
realPath: (path) =>
path === missingAbsolutePath
? Effect.fail(
@ -130,8 +147,6 @@ const mutation = Layer.succeed(
LocationMutation.Service,
LocationMutation.Service.of({
resolve: (input) => {
if (input.path === missingPath)
return Effect.fail(new LocationMutation.PathError({ path: input.path, reason: "non_directory_ancestor" }))
const canonical = path.resolve(process.cwd(), input.path)
const external = path.isAbsolute(input.path) && !FSUtil.contains(process.cwd(), canonical)
const resource = external ? canonical.replaceAll("\\", "/") : path.relative(process.cwd(), canonical) || "."
@ -183,6 +198,8 @@ describe("ReadTool", () => {
allow = true
resolvedType = "file"
resolveFailure = undefined
inspectFailure = undefined
directoryEntries = []
readResult = {
type: "file",
uri: "file:///README.md",
@ -192,6 +209,7 @@ describe("ReadTool", () => {
mime: "text/plain",
}
readFailure = undefined
listResult = new ReadToolFileSystem.ListPage({ type: "list-page", entries: [], truncated: false })
})
it.effect("registers, authorizes, and reads through the location filesystem", () =>
@ -219,6 +237,7 @@ describe("ReadTool", () => {
encoding: "utf8",
mime: "text/plain",
})
expect(execution.content).toEqual([{ type: "text", text: "Read file README.md, lines 1-1\n1: hello" }])
expect(assertions).toMatchObject([{ sessionID, action: "read", resources: ["README.md"], save: ["*"] }])
expect(readCalls).toEqual([
{
@ -658,6 +677,14 @@ describe("ReadTool", () => {
it.effect("returns missing paths as model-visible tool failures", () =>
Effect.gen(function* () {
inspectFailure = notFound(missingAbsolutePath)
directoryEntries = [
"__missing_read_target__.txt.bak",
"copy___missing_read_target__.txt",
"old___missing_read_target__.txt",
"other___missing_read_target__.txt",
"unrelated.txt",
]
const registry = yield* Tool.Service
expect(
@ -666,10 +693,14 @@ describe("ReadTool", () => {
...toolIdentity,
call: { type: "tool-call", id: "call-missing-path", name: "read", input: { path: missingPath } },
}),
// The message-less PathError cause must not erase the tool's curated
// failure message; the canonical error is the sole authority.
).toEqual({ status: "error", error: { type: "tool.execution", message: `Unable to read ${missingPath}` } })
expect(assertions).toEqual([])
).toEqual({
status: "error",
error: {
type: "tool.execution",
message: `File not found: ${missingPath}\n\nDid you mean one of these?\n__missing_read_target__.txt.bak\ncopy___missing_read_target__.txt\nold___missing_read_target__.txt`,
},
})
expect(assertions).toMatchObject([{ sessionID, action: "read", resources: [missingPath], save: ["*"] }])
expect(readCalls).toEqual([])
}),
)
@ -677,10 +708,18 @@ describe("ReadTool", () => {
it.effect("lists a bounded directory page through read", () =>
Effect.gen(function* () {
resolvedType = "directory"
listResult = new ReadToolFileSystem.ListPage({
type: "list-page",
entries: [
FileSystem.Entry.make({ path: RelativePath.make("components/"), type: "directory" }),
FileSystem.Entry.make({ path: RelativePath.make("index.ts"), type: "file" }),
],
truncated: true,
next: 4,
})
const registry = yield* Tool.Service
expect(
yield* executeTool(registry, {
const result = yield* executeTool(registry, {
sessionID,
...toolIdentity,
call: {
@ -689,8 +728,15 @@ describe("ReadTool", () => {
name: "read",
input: { path: "src", offset: 2, limit: 10 },
},
}),
).toMatchObject({ status: "completed", output: { entries: [], truncated: false } })
})
expect(result).toMatchObject({ status: "completed", output: { entries: listResult.entries, truncated: true, next: 4 } })
if (result.status !== "completed") return
expect(result.content).toEqual([
{
type: "text",
text: "Read directory src, entries 2-3\ncomponents/\nindex.ts\n[Output truncated. Continue reading with offset: 4]",
},
])
expect(assertions).toMatchObject([{ sessionID, action: "read", resources: ["src"], save: ["*"] }])
expect(listCalls).toEqual([{ offset: 2, limit: 10 }])
}),
@ -744,21 +790,27 @@ describe("ReadTool", () => {
})
const registry = yield* Tool.Service
expect(
yield* executeTool(registry, {
sessionID,
...toolIdentity,
call: {
type: "tool-call",
id: "call-large",
name: "read",
input: { path: "large.txt", offset: 2, limit: 1 },
},
}),
).toMatchObject({
const result = yield* executeTool(registry, {
sessionID,
...toolIdentity,
call: {
type: "tool-call",
id: "call-large",
name: "read",
input: { path: "large.txt", offset: 2, limit: 1 },
},
})
expect(result).toMatchObject({
status: "completed",
output: { type: "text-page", content: "hello", mime: "text/plain", offset: 2, truncated: true, next: 3 },
})
if (result.status !== "completed") return
expect(result.content).toEqual([
{
type: "text",
text: "Read file large.txt, lines 2-2\n2: hello\n[Output truncated. Continue reading with offset: 3]",
},
])
expect(readCalls).toEqual([
{ input: AbsolutePath.make(path.join(process.cwd(), "large.txt")), page: { offset: 2, limit: 1 } },
])