ceos/opencode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(tools): unify tool APIs and result handling (#38367)

Browse source
This commit is contained in:
Kit Langton 2026-07-23 17:13:31 -04:00 • committed by GitHub
commit 79c1544072
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
133 changed files with 3602 additions and 2770 deletions
Download patch file Download diff file Expand all files Collapse all files

171
packages/core/test/tool-patch.test.ts
View file

@ -16,7 +16,7 @@ import { location } from "./fixture/location"
import { tmpdir } from "./fixture/tmpdir"
import { makeLocationNode } from "@opencode-ai/util/effect/app-node"
import { testEffect } from "./lib/effect"
import { toolIdentity, executeTool, registerToolPlugin, settleTool, toolDefinitions } from "./lib/tool"
import { toolIdentity, executeTool, registerToolPlugin, toolDefinitions } from "./lib/tool"
const patchToolNode = makeLocationNode({
name: "test/patch-tool-plugin",
@ -96,29 +96,19 @@ const withTool = <A, E, R>(
const activeLocation = Layer.succeed(
Location.Service,
Location.Service.of(
location(
{ directory: AbsolutePath.make(directory) },
{ projectDirectory: AbsolutePath.make(projectDirectory) },
),
location({ directory: AbsolutePath.make(directory) }, { projectDirectory: AbsolutePath.make(projectDirectory) }),
),
)
return Effect.gen(function* () {
return yield* body(yield* ToolRegistry.Service)
}).pipe(
Effect.provide(
AppNodeBuilder.build(
LayerNode.group([
ToolRegistry.node,
ToolRegistry.toolsNode,
patchToolNode,
]),
[
[FSUtil.node, filesystem],
[Location.node, activeLocation],
[PermissionV2.node, permission],
[ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig],
],
),
AppNodeBuilder.build(LayerNode.group([ToolRegistry.node, ToolRegistry.toolsNode, patchToolNode]), [
[FSUtil.node, filesystem],
[Location.node, activeLocation],
[PermissionV2.node, permission],
[ToolOutputStore.node, ToolOutputStore.nodeWithoutConfig],
]),
),
)
}
@ -162,18 +152,23 @@ describe("PatchTool", () => {
withTool(tmp.path, (registry) =>
Effect.gen(function* () {
expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["patch"])
const settled = yield* settleTool(
const settled = yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Add File: nested/new.txt\n+created\n*** Update File: update.txt\n@@\n-before\n+after\n*** Delete File: remove.txt\n*** End Patch",
),
)
expect(settled.result).toEqual({
type: "text",
value: "Success. Updated the following files:\nA nested/new.txt\nM update.txt\nD remove.txt",
})
if (process.platform === "win32") expect(settled.result.value).not.toContain("\\")
expect(settled.output?.structured).toMatchObject({
expect(settled.status).toBe("completed")
if (settled.status !== "completed") return
expect(settled.content).toEqual([
{
type: "text",
text: "Success. Updated the following files:\nA nested/new.txt\nM update.txt\nD remove.txt",
},
])
const modelText = settled.content[0]?.type === "text" ? settled.content[0].text : ""
if (process.platform === "win32") expect(modelText).not.toContain("\\")
expect(settled.output).toMatchObject({
applied: [
{ type: "add", resource: "nested/new.txt" },
{ type: "update", resource: "update.txt" },
@ -248,9 +243,11 @@ describe("PatchTool", () => {
"*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: old.txt\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch",
),
),
).toEqual({
type: "text",
value: "Success. Updated the following files:\nA created.txt\nM moved.txt",
).toMatchObject({
status: "completed",
content: [
{ type: "text", text: "Success. Updated the following files:\nA created.txt\nM moved.txt" },
],
})
expect(yield* exists(source)).toBe(false)
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "moved.txt"), "utf8"))).toBe(
@ -278,7 +275,9 @@ describe("PatchTool", () => {
return Effect.promise(() =>
Promise.all([
fs.writeFile(source, "before\n"),
fs.mkdir(path.dirname(destination), { recursive: true }).then(() => fs.writeFile(destination, "existing\n")),
fs
.mkdir(path.dirname(destination), { recursive: true })
.then(() => fs.writeFile(destination, "existing\n")),
]),
).pipe(
Effect.andThen(
@ -291,7 +290,7 @@ describe("PatchTool", () => {
"*** Begin Patch\n*** Update File: old.txt\n*** Move to: nested/moved.txt\n@@\n-before\n+after\n*** End Patch",
),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(yield* exists(source)).toBe(false)
expect(yield* Effect.promise(() => fs.readFile(destination, "utf8"))).toBe("after\n")
}),
@ -325,19 +324,21 @@ describe("PatchTool", () => {
),
)
it.live("includes move file info in structured output", () =>
it.live("includes move file info in output and metadata", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const source = path.join(directory, "old", "name.txt")
yield* Effect.promise(() => fs.mkdir(path.dirname(source), { recursive: true }))
yield* Effect.promise(() => fs.writeFile(source, "old content\n"))
const settled = yield* settleTool(
const settled = yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch",
),
)
expect(settled.output?.structured).toMatchObject({
expect(settled.status).toBe("completed")
if (settled.status !== "completed") return
expect(settled.output).toMatchObject({
applied: [{ type: "update", resource: "renamed/dir/name.txt" }],
files: [
{
@ -393,7 +394,7 @@ describe("PatchTool", () => {
yield* Effect.promise(() => fs.mkdir(path.join(directory, "dir")))
expect(
yield* executeTool(registry, call("*** Begin Patch\n*** Delete File: dir\n*** End Patch")),
).toMatchObject({ type: "error" })
).toMatchObject({ status: "error" })
expect(yield* exists(path.join(directory, "dir"))).toBe(true)
}),
),
@ -407,11 +408,9 @@ describe("PatchTool", () => {
expect(
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: two-chunks.txt\n@@\n-b\n+B\n\n-d\n+D\n*** End Patch",
),
call("*** Begin Patch\n*** Update File: two-chunks.txt\n@@\n-b\n+B\n\n-d\n+D\n*** End Patch"),
),
).toMatchObject({ type: "error" })
).toMatchObject({ status: "error" })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("a\nb\nc\nd\n")
}),
),
@ -420,7 +419,10 @@ describe("PatchTool", () => {
it.live("requires patchText", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call(""))).toEqual({ type: "error", value: "patchText is required" })
expect(yield* executeTool(registry, call(""))).toEqual({
status: "error",
error: { type: "tool.execution", message: "patchText is required" },
})
}),
),
)
@ -429,12 +431,18 @@ describe("PatchTool", () => {
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call("invalid patch"))).toEqual({
type: "error",
value: "patch verification failed: The first line of the patch must be '*** Begin Patch'",
status: "error",
error: {
type: "tool.execution",
message: "patch verification failed: The first line of the patch must be '*** Begin Patch'",
},
})
expect(yield* executeTool(registry, call("*** Begin Patch\n*** Add File: foo\n+hello"))).toEqual({
type: "error",
value: "patch verification failed: The last line of the patch must be '*** End Patch'",
status: "error",
error: {
type: "tool.execution",
message: "patch verification failed: The last line of the patch must be '*** End Patch'",
},
})
}),
),
@ -444,8 +452,8 @@ describe("PatchTool", () => {
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call("*** Begin Patch\n*** End Patch"))).toEqual({
type: "error",
value: "patch rejected: empty patch",
status: "error",
error: { type: "tool.execution", message: "patch rejected: empty patch" },
})
}),
),
@ -454,15 +462,13 @@ describe("PatchTool", () => {
it.live("rejects an invalid hunk header", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call("*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"),
),
).toEqual({
type: "error",
value:
"patch verification failed: Invalid hunk at line 2: '*** Frobnicate File: foo' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'",
expect(yield* executeTool(registry, call("*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"))).toEqual({
status: "error",
error: {
type: "tool.execution",
message:
"patch verification failed: Invalid hunk at line 2: '*** Frobnicate File: foo' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'",
},
})
}),
),
@ -490,13 +496,13 @@ describe("PatchTool", () => {
const bom = "\uFEFF"
const target = path.join(directory, "example.cs")
yield* Effect.promise(() => fs.writeFile(target, `${bom}using System;\n\nclass Test {}\n`))
const settled = yield* settleTool(
const settled = yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: example.cs\n@@\n class Test {}\n+class Next {}\n*** End Patch",
),
call("*** Begin Patch\n*** Update File: example.cs\n@@\n class Test {}\n+class Next {}\n*** End Patch"),
)
const output = Schema.decodeUnknownSync(PatchTool.Output)(settled.output?.structured)
expect(settled.status).toBe("completed")
if (settled.status !== "completed") return
const output = Schema.decodeUnknownSync(PatchTool.Output)(settled.output)
expect(output.files[0]?.patch).not.toContain(bom)
expect(output.files[0]?.patch).not.toContain("-using System;")
expect(output.files[0]?.patch).not.toContain("+using System;")
@ -517,7 +523,10 @@ describe("PatchTool", () => {
registry,
call("*** Begin Patch\n*** Update File: unchanged.txt\n@@\n-missing\n+changed\n*** End Patch"),
),
).toMatchObject({ type: "error", value: expect.stringContaining("Failed to find expected lines") })
).toMatchObject({
status: "error",
error: { message: expect.stringContaining("Failed to find expected lines") },
})
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("line1\nline2\n")
}),
),
@ -532,10 +541,12 @@ describe("PatchTool", () => {
call("*** Begin Patch\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch"),
),
).toMatchObject({
type: "error",
value: expect.stringContaining(
`patch verification failed: Failed to read file to update ${path.join(directory, "missing.txt")}: `,
),
status: "error",
error: {
message: expect.stringContaining(
`patch verification failed: Failed to read file to update ${path.join(directory, "missing.txt")}: `,
),
},
})
}),
),
@ -548,8 +559,11 @@ describe("PatchTool", () => {
expect(
yield* executeTool(registry, call("*** Begin Patch\n*** Update File: nested\n@@\n-old\n+new\n*** End Patch")),
).toEqual({
type: "error",
value: `patch verification failed: Failed to read file to update ${path.join(directory, "nested")}: path is a directory`,
status: "error",
error: {
type: "tool.execution",
message: `patch verification failed: Failed to read file to update ${path.join(directory, "nested")}: path is a directory`,
},
})
}),
),
@ -560,7 +574,7 @@ describe("PatchTool", () => {
Effect.gen(function* () {
expect(
yield* executeTool(registry, call("*** Begin Patch\n*** Delete File: missing.txt\n*** End Patch")),
).toMatchObject({ type: "error", value: expect.stringContaining("patch verification failed") })
).toMatchObject({ status: "error", error: { message: expect.stringContaining("patch verification failed") } })
}),
),
)
@ -580,7 +594,7 @@ describe("PatchTool", () => {
registry,
call(`*** Begin Patch\n*** Update File: ${target}\n@@\n-before\n+after\n*** End Patch`),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
expect(readsBeforeEditApproval).toBe(1)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
@ -614,7 +628,7 @@ describe("PatchTool", () => {
registry,
call(`*** Begin Patch\n*** Update File: ${target}\n@@\n-before\n+after\n*** End Patch`),
),
).toMatchObject({ type: "error" })
).toMatchObject({ status: "error" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory"])
expect(readsBeforeEditApproval).toBe(0)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("before\n")
@ -649,7 +663,7 @@ describe("PatchTool", () => {
registry,
call("*** Begin Patch\n*** Update File: ../sibling.txt\n@@\n-before\n+after\n*** End Patch"),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
}),
@ -680,7 +694,7 @@ describe("PatchTool", () => {
registry,
call("*** Begin Patch\n*** Update File: link.txt\n@@\n-before\n+after\n*** End Patch"),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
}),
@ -711,7 +725,7 @@ describe("PatchTool", () => {
registry,
call(`*** Begin Patch\n*** Update File: ${relative}\n@@\n-before\n+after\n*** End Patch`),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
expect(readsBeforeEditApproval).toBe(1)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
@ -747,7 +761,7 @@ describe("PatchTool", () => {
`*** Begin Patch\n*** Update File: ${first}\n@@\n-before\n+after\n*** Update File: ${second}\n@@\n-before\n+after\n*** End Patch`,
),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(assertions.map((input) => input.action)).toEqual([
"external_directory",
"external_directory",
@ -786,8 +800,10 @@ describe("PatchTool", () => {
),
),
).toMatchObject({
type: "error",
value: expect.stringContaining("patch verification failed: Failed to read file to update"),
status: "error",
error: {
message: expect.stringContaining("patch verification failed: Failed to read file to update"),
},
})
expect(yield* exists(path.join(tmp.path, "created.txt"))).toBe(false)
}),
@ -812,7 +828,7 @@ describe("PatchTool", () => {
registry,
call("*** Begin Patch\n*** Add File: existing.txt\n+replacement\n*** End Patch"),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("replacement\n")
}),
),
@ -837,7 +853,7 @@ describe("PatchTool", () => {
registry,
call("*** Begin Patch\n*** Add File: appeared.txt\n+replacement\n*** End Patch"),
),
).toMatchObject({ type: "text" })
).toMatchObject({ status: "completed" })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("replacement\n")
}),
)
@ -876,5 +892,4 @@ describe("PatchTool", () => {
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
)
})