refactor(core): isolate tool test fixture state (#45452)

This commit is contained in:
Kit Langton 2026-08-27 18:48:44 -04:00 committed by GitHub
parent ba85531931
commit 091396742e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 652 additions and 704 deletions

View file

@ -22,7 +22,7 @@ function provide(directory: string, transformFiles: EnvironmentFilesTransform =
return Effect.provide(
AppNodeBuilder.build(LayerNode.group([LocationMutation.node, FileMutation.node]), [
[Location.node, activeLocation],
[Environment.node, transformEnvironmentFiles(activeLocation, transformFiles)],
[Environment.node, transformEnvironmentFiles(transformFiles)],
]),
)
}

View file

@ -1,6 +1,4 @@
import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
import { Environment } from "@opencode-ai/core/environment/index"
import { Location } from "@opencode-ai/core/location"
import { CrossSpawnSpawner } from "@opencode-ai/util/cross-spawn-spawner"
import { LayerNode } from "@opencode-ai/util/effect/layer-node"
import { Effect, Layer } from "effect"
@ -40,10 +38,8 @@ export const recordingEnvironmentLayer = (spawns: Array<ChildProcess.Command>) =
export type EnvironmentFilesTransform = (files: Environment.Files) => Partial<Environment.Files>
export function transformEnvironmentFiles(
location: Layer.Layer<Location.Service>,
transform: EnvironmentFilesTransform = () => ({}),
) {
// Wrap real host filesystem operations without constructing workspace services.
export function transformEnvironmentFiles(transform: EnvironmentFilesTransform = () => ({})) {
return Layer.effect(
Environment.Service,
Effect.gen(function* () {
@ -53,5 +49,5 @@ export function transformEnvironmentFiles(
files: { ...current.files, ...transform(current.files) },
})
}),
).pipe(Layer.provide(AppNodeBuilder.build(Environment.node, [[Location.node, location]])))
).pipe(Layer.provide(hostEnvironmentLayer))
}

View file

@ -1,6 +1,7 @@
import fs from "fs/promises"
import { tmpdir as osTmpdir } from "os"
import path from "path"
import { Effect } from "effect"
export const tmpdir = async (prefix = "opencode-core-test-") => {
const dir = await fs.realpath(await fs.mkdtemp(path.join(osTmpdir(), prefix)))
@ -12,6 +13,13 @@ export const tmpdir = async (prefix = "opencode-core-test-") => {
}
}
export const withTempDir = <A, E, R>(body: (tmp: Awaited<ReturnType<typeof tmpdir>>) => Effect.Effect<A, E, R>) =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
body,
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
)
async function remove(dir: string, retries = 30): Promise<void> {
try {
await fs.rm(dir, { recursive: true, force: true })

File diff suppressed because it is too large Load diff

View file

@ -102,7 +102,7 @@ const withTool = <A, E, R>(
AppNodeBuilder.build(LayerNode.group([Tool.node, LocationMutation.node, FileMutation.node, patchToolNode]), [
[
Environment.node,
transformEnvironmentFiles(activeLocation, (files) => ({
transformEnvironmentFiles((files) => ({
read: (target, range) =>
Effect.sync(() => {
if (!editApproved) readsBeforeEditApproval++

View file

@ -16,7 +16,7 @@ import { Tool } from "@opencode-ai/core/tool"
import { WriteTool } from "@opencode-ai/core/tool/plugin/write"
import { transformEnvironmentFiles } from "./fixture/environment"
import { location } from "./fixture/location"
import { tmpdir } from "./fixture/tmpdir"
import { tmpdir, withTempDir } from "./fixture/tmpdir"
import { makeLocationNode } from "@opencode-ai/util/effect/app-node"
import { testEffect } from "./lib/effect"
import { permissionLayer } from "./lib/permission"
@ -29,63 +29,68 @@ const writeToolNode = makeLocationNode({
})
const sessionID = Session.ID.make("ses_write_tool_test")
const assertions: Permission.AssertInput[] = []
const writes: string[] = []
let formatFile = (_target: string): Effect.Effect<boolean> => Effect.succeed(false)
let denyAction: string | undefined
const makeWriteFixture = () => {
const fixture: {
assertions: Permission.AssertInput[]
writes: string[]
denyAction?: string
formatFile: (target: string) => Effect.Effect<boolean>
} = {
assertions: [],
writes: [],
formatFile: () => Effect.succeed(false),
}
const permission = permissionLayer({
assert: (input) =>
Effect.sync(() => assertions.push(input)).pipe(
Effect.andThen(
input.action === denyAction
? Effect.fail(
new Permission.BlockedError({
rules: [],
permission: input.action,
resources: input.resources,
}),
)
: Effect.void,
const permission = permissionLayer({
assert: (input) =>
Effect.sync(() => fixture.assertions.push(input)).pipe(
Effect.andThen(
input.action === fixture.denyAction
? Effect.fail(
new Permission.BlockedError({
rules: [],
permission: input.action,
resources: input.resources,
}),
)
: Effect.void,
),
),
),
})
})
const formatter = Layer.mock(Formatter.Service, {
file: (target) => formatFile(target),
})
const formatter = Layer.mock(Formatter.Service, {
file: (target) => fixture.formatFile(target),
})
const reset = () => {
assertions.length = 0
writes.length = 0
formatFile = () => Effect.succeed(false)
denyAction = undefined
return Object.assign(fixture, { permission, formatter })
}
const withTool = <A, E, R>(directory: string, body: (registry: Tool.Interface) => Effect.Effect<A, E, R>) => {
const withTool = <A, E, R>(
directory: string,
fixture: ReturnType<typeof makeWriteFixture>,
body: (registry: Tool.Interface) => Effect.Effect<A, E, R>,
) => {
const activeLocation = Layer.succeed(
Location.Service,
Location.Service.of(location({ directory: AbsolutePath.make(directory) })),
)
return Effect.gen(function* () {
return yield* body(yield* Tool.Service)
const registry = yield* Tool.Service
return yield* body(registry)
}).pipe(
Effect.provide(
AppNodeBuilder.build(
LayerNode.group([Tool.node, Tool.node, LocationMutation.node, FileMutation.node, writeToolNode]),
AppNodeBuilder.build(LayerNode.group([Tool.node, LocationMutation.node, FileMutation.node, writeToolNode]), [
[
[
Environment.node,
transformEnvironmentFiles(activeLocation, (files) => ({
write: (target, content) =>
Effect.sync(() => writes.push(target)).pipe(Effect.andThen(files.write(target, content))),
})),
],
[Location.node, activeLocation],
[Formatter.node, formatter],
[Permission.node, permission],
Environment.node,
transformEnvironmentFiles((files) => ({
write: (target, content) =>
Effect.sync(() => fixture.writes.push(target)).pipe(Effect.andThen(files.write(target, content))),
})),
],
),
[Location.node, activeLocation],
[Formatter.node, fixture.formatter],
[Permission.node, fixture.permission],
]),
),
)
}
@ -100,175 +105,158 @@ const it = testEffect(Layer.empty)
describe("WriteTool", () => {
it.live("registers and creates a relative file through FileMutation once", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
return withTool(tmp.path, (registry) =>
Effect.gen(function* () {
expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["write", "execute"])
const settled = yield* executeTool(registry, call({ path: "src/new.txt", content: "created" }))
expect(settled).toEqual({
status: "completed",
output: {
operation: "write",
target: path.join(yield* Effect.promise(() => fs.realpath(tmp.path)), "src", "new.txt"),
resource: "src/new.txt",
existed: false,
withTempDir((tmp) => {
const fixture = makeWriteFixture()
return withTool(tmp.path, fixture, (registry) =>
Effect.gen(function* () {
expect((yield* toolDefinitions(registry)).map((tool) => tool.name)).toEqual(["write", "execute"])
const settled = yield* executeTool(registry, call({ path: "src/new.txt", content: "created" }))
expect(settled).toEqual({
status: "completed",
output: {
operation: "write",
target: path.join(yield* Effect.promise(() => fs.realpath(tmp.path)), "src", "new.txt"),
resource: "src/new.txt",
existed: false,
},
content: [{ type: "text", text: "Created file successfully: src/new.txt" }],
})
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "src", "new.txt"), "utf8"))).toBe(
"created",
)
expect(fixture.assertions).toMatchObject([
{ sessionID, action: "edit", resources: ["src/new.txt"], save: ["*"] },
])
expect(fixture.assertions[0]?.metadata).toMatchObject({
files: [
{
file: "src/new.txt",
status: "added",
additions: 1,
deletions: 0,
patch: expect.stringContaining("+created"),
},
content: [{ type: "text", text: "Created file successfully: src/new.txt" }],
})
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "src", "new.txt"), "utf8"))).toBe(
"created",
)
expect(assertions).toMatchObject([{ sessionID, action: "edit", resources: ["src/new.txt"], save: ["*"] }])
expect(assertions[0]?.metadata).toMatchObject({
files: [
{
file: "src/new.txt",
status: "added",
additions: 1,
deletions: 0,
patch: expect.stringContaining("+created"),
},
],
})
expect(writes).toEqual([path.join(yield* Effect.promise(() => fs.realpath(tmp.path)), "src", "new.txt")])
}),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
],
})
expect(fixture.writes).toEqual([
path.join(yield* Effect.promise(() => fs.realpath(tmp.path)), "src", "new.txt"),
])
}),
)
}),
)
it.live("formats the committed file", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const target = path.join(tmp.path, "formatted.txt")
formatFile = (file) =>
Effect.promise(async () => {
await fs.writeFile(file, (await fs.readFile(file, "utf8")).toUpperCase())
return true
withTempDir((tmp) => {
const fixture = makeWriteFixture()
const target = path.join(tmp.path, "formatted.txt")
fixture.formatFile = (file) =>
Effect.promise(async () => {
await fs.writeFile(file, (await fs.readFile(file, "utf8")).toUpperCase())
return true
})
return withTool(tmp.path, fixture, (registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call({ path: "formatted.txt", content: "format me" }))).toMatchObject({
status: "completed",
})
return withTool(tmp.path, (registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call({ path: "formatted.txt", content: "format me" }))).toMatchObject({
status: "completed",
})
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("FORMAT ME")
}),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("FORMAT ME")
}),
)
}),
)
it.live("overwrites a relative existing file and reports that it wrote the file", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
return Effect.promise(() => fs.writeFile(path.join(tmp.path, "existing.txt"), "before")).pipe(
Effect.andThen(
withTool(tmp.path, (registry) => executeTool(registry, call({ path: "existing.txt", content: "after" }))),
withTempDir((tmp) => {
const fixture = makeWriteFixture()
return Effect.promise(() => fs.writeFile(path.join(tmp.path, "existing.txt"), "before")).pipe(
Effect.andThen(
withTool(tmp.path, fixture, (registry) =>
executeTool(registry, call({ path: "existing.txt", content: "after" })),
),
Effect.andThen((settled) =>
Effect.gen(function* () {
expect(settled.status).toBe("completed")
if (settled.status !== "completed") return
expect(settled.content).toEqual([{ type: "text", text: "Wrote file successfully: existing.txt" }])
expect(settled.output).toMatchObject({ resource: "existing.txt", existed: true })
expect(assertions[0]?.metadata).toMatchObject({
files: [
{
file: "existing.txt",
status: "modified",
additions: 1,
deletions: 1,
patch: expect.stringMatching(/-before[\s\S]*\+after/),
},
],
})
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "existing.txt"), "utf8"))).toBe(
"after",
)
expect(writes).toHaveLength(1)
}),
),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
),
Effect.andThen((settled) =>
Effect.gen(function* () {
expect(settled.status).toBe("completed")
if (settled.status !== "completed") return
expect(settled.content).toEqual([{ type: "text", text: "Wrote file successfully: existing.txt" }])
expect(settled.output).toMatchObject({ resource: "existing.txt", existed: true })
expect(fixture.assertions[0]?.metadata).toMatchObject({
files: [
{
file: "existing.txt",
status: "modified",
additions: 1,
deletions: 1,
patch: expect.stringMatching(/-before[\s\S]*\+after/),
},
],
})
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "existing.txt"), "utf8"))).toBe("after")
expect(fixture.writes).toHaveLength(1)
}),
),
)
}),
)
it.live("preserves exactly one BOM when overwriting existing files", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const preserved = path.join(tmp.path, "preserved.txt")
const deduplicated = path.join(tmp.path, "deduplicated.txt")
formatFile = (target) =>
Effect.promise(async () => {
await fs.writeFile(
target,
`\uFEFF\uFEFF\uFEFF${(await fs.readFile(target, "utf8")).replace(/^\uFEFF+/, "")}`,
)
return true
})
return Effect.promise(() =>
Promise.all([fs.writeFile(preserved, "\uFEFFbefore"), fs.writeFile(deduplicated, "\uFEFFbefore")]),
).pipe(
Effect.andThen(
withTool(tmp.path, (registry) =>
Effect.gen(function* () {
yield* executeTool(registry, call({ path: "preserved.txt", content: "after" }, "call-preserved"))
yield* executeTool(
registry,
call({ path: "deduplicated.txt", content: "\uFEFFafter" }, "call-deduplicated"),
)
withTempDir((tmp) => {
const fixture = makeWriteFixture()
const preserved = path.join(tmp.path, "preserved.txt")
const deduplicated = path.join(tmp.path, "deduplicated.txt")
fixture.formatFile = (target) =>
Effect.promise(async () => {
await fs.writeFile(target, `\uFEFF\uFEFF\uFEFF${(await fs.readFile(target, "utf8")).replace(/^\uFEFF+/, "")}`)
return true
})
return Effect.promise(() =>
Promise.all([fs.writeFile(preserved, "\uFEFFbefore"), fs.writeFile(deduplicated, "\uFEFFbefore")]),
).pipe(
Effect.andThen(
withTool(tmp.path, fixture, (registry) =>
Effect.gen(function* () {
yield* executeTool(registry, call({ path: "preserved.txt", content: "after" }, "call-preserved"))
yield* executeTool(
registry,
call({ path: "deduplicated.txt", content: "\uFEFFafter" }, "call-deduplicated"),
)
expect(yield* Effect.promise(() => fs.readFile(preserved, "utf8"))).toBe("\uFEFFafter")
expect(yield* Effect.promise(() => fs.readFile(deduplicated, "utf8"))).toBe("\uFEFFafter")
}),
),
expect(yield* Effect.promise(() => fs.readFile(preserved, "utf8"))).toBe("\uFEFFafter")
expect(yield* Effect.promise(() => fs.readFile(deduplicated, "utf8"))).toBe("\uFEFFafter")
}),
),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
),
)
}),
)
it.live("accepts an absolute file path inside the active Location", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const target = path.join(tmp.path, "absolute.txt")
return withTool(tmp.path, (registry) => executeTool(registry, call({ path: target, content: "inside" }))).pipe(
Effect.andThen((result) =>
Effect.gen(function* () {
expect(result).toMatchObject({
status: "completed",
content: [{ type: "text", text: "Created file successfully: absolute.txt" }],
})
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("inside")
}),
),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
withTempDir((tmp) => {
const fixture = makeWriteFixture()
const target = path.join(tmp.path, "absolute.txt")
return withTool(tmp.path, fixture, (registry) =>
executeTool(registry, call({ path: target, content: "inside" })),
).pipe(
Effect.andThen((result) =>
Effect.gen(function* () {
expect(result).toMatchObject({
status: "completed",
content: [{ type: "text", text: "Created file successfully: absolute.txt" }],
})
expect(fixture.assertions.map((input) => input.action)).toEqual(["edit"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("inside")
}),
),
)
}),
)
it.live("writes an external symlink target with only its in-location permission", () =>
Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => {
reset()
const fixture = makeWriteFixture()
if (process.platform === "win32") return Effect.void
const target = path.join(outside.path, "external.txt")
const link = path.join(active.path, "link.txt")
@ -277,13 +265,15 @@ describe("WriteTool", () => {
await fs.symlink(target, link)
}).pipe(
Effect.andThen(
withTool(active.path, (registry) => executeTool(registry, call({ path: "link.txt", content: "after" }))),
withTool(active.path, fixture, (registry) =>
executeTool(registry, call({ path: "link.txt", content: "after" })),
),
),
Effect.andThen((result) =>
Effect.sync(() => {
expect(result.status).toBe("completed")
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(assertions[0]?.resources).toEqual(["link.txt"])
expect(fixture.assertions.map((input) => input.action)).toEqual(["edit"])
expect(fixture.assertions[0]?.resources).toEqual(["link.txt"])
}),
),
Effect.andThen(Effect.promise(() => fs.readFile(target, "utf8"))),
@ -301,19 +291,22 @@ describe("WriteTool", () => {
Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => {
reset()
const fixture = makeWriteFixture()
const target = path.join(outside.path, "external.txt")
return withTool(active.path, (registry) =>
return withTool(active.path, fixture, (registry) =>
executeTool(registry, call({ path: target, content: "external" })),
).pipe(
Effect.andThen((settled) =>
Effect.gen(function* () {
const absoluteTarget = target
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
expect(assertions[0]).toMatchObject({
expect(fixture.assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
expect(fixture.assertions[0]).toMatchObject({
resources: [path.join(outside.path, "*").replaceAll("\\", "/")],
})
expect(assertions[1]).toMatchObject({ resources: [absoluteTarget.replaceAll("\\", "/")], save: ["*"] })
expect(fixture.assertions[1]).toMatchObject({
resources: [absoluteTarget.replaceAll("\\", "/")],
save: ["*"],
})
expect(settled).toMatchObject({
status: "completed",
output: {
@ -323,7 +316,7 @@ describe("WriteTool", () => {
},
})
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("external")
expect(writes).toEqual([absoluteTarget])
expect(fixture.writes).toEqual([absoluteTarget])
}),
),
)
@ -339,7 +332,7 @@ describe("WriteTool", () => {
Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => {
reset()
const fixture = makeWriteFixture()
const repo = path.join(outside.path, "repo")
const nested = path.join(repo, "packages", "app")
const target = path.join(nested, "external.txt")
@ -347,11 +340,13 @@ describe("WriteTool", () => {
Promise.all([fs.mkdir(path.join(repo, ".git"), { recursive: true }), fs.mkdir(nested, { recursive: true })]),
).pipe(
Effect.andThen(
withTool(active.path, (registry) => executeTool(registry, call({ path: target, content: "external" }))),
withTool(active.path, fixture, (registry) =>
executeTool(registry, call({ path: target, content: "external" })),
),
),
Effect.andThen(
Effect.gen(function* () {
expect(assertions[0]).toMatchObject({
expect(fixture.assertions[0]).toMatchObject({
action: "external_directory",
resources: [path.join(nested, "*").replaceAll("\\", "/")],
save: [path.join(repo, "*").replaceAll("\\", "/")],
@ -373,31 +368,31 @@ describe("WriteTool", () => {
([active, outside]) =>
Effect.gen(function* () {
const external = path.join(outside.path, "denied.txt")
reset()
denyAction = "external_directory"
const fixture = makeWriteFixture()
fixture.denyAction = "external_directory"
expect(
yield* withTool(active.path, (registry) =>
yield* withTool(active.path, fixture, (registry) =>
executeTool(registry, call({ path: external, content: "blocked" })),
),
).toEqual({
status: "error",
error: { type: "permission.rejected", message: "Permission denied: external_directory" },
})
expect(assertions.map((input) => input.action)).toEqual(["external_directory"])
expect(writes).toEqual([])
expect(fixture.assertions.map((input) => input.action)).toEqual(["external_directory"])
expect(fixture.writes).toEqual([])
reset()
denyAction = "edit"
const deniedEdit = makeWriteFixture()
deniedEdit.denyAction = "edit"
expect(
yield* withTool(active.path, (registry) =>
yield* withTool(active.path, deniedEdit, (registry) =>
executeTool(registry, call({ path: "denied.txt", content: "blocked" })),
),
).toEqual({
status: "error",
error: { type: "permission.rejected", message: "Permission denied: edit" },
})
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(writes).toEqual([])
expect(deniedEdit.assertions.map((input) => input.action)).toEqual(["edit"])
expect(deniedEdit.writes).toEqual([])
}),
([active, outside]) =>
Effect.promise(() =>