vrr/opencode
2
0
Fork 0
mirror of https://github.com/anomalyco/opencode.git synced 2026-08-24 09:33:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

fix(core): serialize edit and patch transactions (#40641)

Browse source 
Co-authored-by: Aiden Cline <63023139+rekram1-node@users.noreply.github.com>
Co-authored-by: Aiden Cline <aidenpcline@gmail.com>
This commit is contained in:
opencode-agent[bot] 2026-08-07 10:07:48 -05:00 • committed by GitHub
parent e6d20440f9
commit bc47030d4d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 160 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

13
packages/core/src/file-mutation.ts
View file

@ -29,6 +29,10 @@ export interface WriteResult {
}
export interface Interface {
/** Serialize a complete read/prepare/write mutation transaction by resolved path. */
readonly withLock: (
targets: ReadonlyArray<string>,
) => <A, E, R>(effect: Effect.Effect<A, E, R>) => Effect.Effect<A, E, R>
readonly write: (input: WriteInput) => Effect.Effect<WriteResult, FSUtil.Error>
/** Write text while retaining an existing UTF-8 BOM and emitting at most one BOM. */
readonly writeTextPreservingBom: (input: TextWriteInput) => Effect.Effect<WriteResult, FSUtil.Error>
@ -36,6 +40,9 @@ export interface Interface {
export class Service extends Context.Service<Service, Interface>()("@opencode/FileMutation") {}
/** Share transaction locks across Location graphs that address the same file. */
const transactionLocks = KeyedMutex.makeUnsafe<string>()
/**
* Serialize file changes by absolute target. Conditional writes compare and
* write under the same process-local lock so cooperating OpenCode mutations do
@ -46,6 +53,10 @@ const layer = Layer.effect(
Effect.gen(function* () {
const fs = yield* FSUtil.Service
const locks = KeyedMutex.makeUnsafe<string>()
const withLock: Interface["withLock"] = (targets) => (effect) =>
[...new Set(targets.map(FSUtil.resolve))]
.sort()
.reduceRight((result, target) => transactionLocks.withLock(target)(result), effect)
const withTargetLock =
(target: Target) =>
<A, E, R>(effect: Effect.Effect<A, E, R>) =>
@ -84,7 +95,7 @@ const layer = Layer.effect(
),
)
return Service.of({ write, writeTextPreservingBom })
return Service.of({ withLock, write, writeTextPreservingBom })
}),
)

4
packages/core/src/tool/plugin/edit.ts
View file

@ -11,9 +11,11 @@ import { ToolFailure } from "@opencode-ai/ai"
import { FileDiff } from "@opencode-ai/schema/file-diff"
import { Bom } from "@opencode-ai/util/bom"
import { Effect, Schema } from "effect"
import path from "path"
import { FileMutation } from "../../file-mutation"
import { Formatter } from "../../formatter"
import { FSUtil } from "@opencode-ai/util/fs-util"
import { Location } from "../../location"
import { LocationMutation } from "../../location-mutation"
import { Permission } from "../../permission"
import { fileDiff } from "./file-diff"
@ -112,6 +114,7 @@ export const Plugin = {
const files = yield* FileMutation.Service
const formatter = yield* Formatter.Service
const fs = yield* FSUtil.Service
const location = yield* Location.Service
const permission = yield* Permission.Service
yield* ctx.tool
@ -217,6 +220,7 @@ export const Plugin = {
replacements,
} satisfies Output
}).pipe(
files.withLock([path.resolve(location.directory, input.path)]),
Effect.map((output) => ({
output,
content: `Edited ${output.files[0]?.file} (${output.replacements} replacement${output.replacements === 1 ? "" : "s"})`,

14
packages/core/src/tool/plugin/patch.ts
View file

@ -4,12 +4,13 @@ import type { Context as PluginContext } from "@opencode-ai/plugin/effect/plugin
import { ToolFailure } from "@opencode-ai/ai"
import { FileDiff } from "@opencode-ai/schema/file-diff"
import { createTwoFilesPatch, diffLines } from "diff"
import { Effect, Schema } from "effect"
import { Effect, Result, Schema } from "effect"
import { PlatformError } from "effect/PlatformError"
import path from "path"
import { Bom } from "@opencode-ai/util/bom"
import { FSUtil } from "@opencode-ai/util/fs-util"
import { Formatter } from "../../formatter"
import { FileMutation } from "../../file-mutation"
import { Location } from "../../location"
import { Patch } from "@opencode-ai/util/patch"
import { Permission } from "../../permission"
@ -70,6 +71,7 @@ export const Plugin = {
id: "opencode.tool.patch",
effect: Effect.fn("PatchTool.Plugin")(function* (ctx: PluginContext) {
const fs = yield* FSUtil.Service
const mutation = yield* FileMutation.Service
const formatter = yield* Formatter.Service
const location = yield* Location.Service
const permission = yield* Permission.Service
@ -84,6 +86,15 @@ export const Plugin = {
output: Output,
execute: (input, context) => {
const applied: Array<typeof Applied.Type> = []
const parsed = Patch.parse(input.patchText)
const lockTargets = Result.isSuccess(parsed)
? parsed.success.flatMap((hunk) => [
path.resolve(location.directory, hunk.path),
...(hunk.type === "update" && hunk.movePath
? [path.resolve(location.directory, hunk.movePath)]
: []),
])
: []
const fail = (operation: string, error: unknown) => {
const completed = applied.map((item) => item.resource).join(", ")
return new ToolFailure({
@ -315,6 +326,7 @@ export const Plugin = {
})
return { applied, files }
}).pipe(
mutation.withLock(lockTargets),
Effect.map((output) => ({
output,
content: toModelOutput(output),

51
packages/core/test/file-mutation.test.ts
View file

@ -152,6 +152,57 @@ describe("FileMutation", () => {
),
)
it.live("shares transaction locks across Location service instances", () =>
withTmp((directory) =>
Effect.gen(function* () {
const firstStarted = yield* Deferred.make<void>()
const releaseFirst = yield* Deferred.make<void>()
const secondStarted = yield* Deferred.make<void>()
const target = path.join(directory, "shared.txt")
const first = yield* Effect.gen(function* () {
const files = yield* FileMutation.Service
yield* files.withLock([target])(
Deferred.succeed(firstStarted, undefined).pipe(Effect.andThen(Deferred.await(releaseFirst))),
)
}).pipe(provide(directory), Effect.forkChild)
yield* Deferred.await(firstStarted)
const second = yield* Effect.gen(function* () {
const files = yield* FileMutation.Service
yield* files.withLock([target])(Deferred.succeed(secondStarted, undefined))
}).pipe(provide(directory), Effect.forkChild)
yield* Effect.yieldNow
expect(yield* Deferred.isDone(secondStarted)).toBe(false)
yield* Deferred.succeed(releaseFirst, undefined)
yield* Deferred.await(secondStarted)
yield* Fiber.join(first)
yield* Fiber.join(second)
}),
),
)
it.live("allows transaction locks for distinct resolved paths to proceed independently", () =>
withTmp((directory) =>
Effect.gen(function* () {
const firstStarted = yield* Deferred.make<void>()
const releaseFirst = yield* Deferred.make<void>()
const secondFinished = yield* Deferred.make<void>()
const files = yield* FileMutation.Service
const first = yield* files
.withLock([path.join(directory, "first.txt")])(
Deferred.succeed(firstStarted, undefined).pipe(Effect.andThen(Deferred.await(releaseFirst))),
)
.pipe(Effect.forkChild)
yield* Deferred.await(firstStarted)
yield* files.withLock([path.join(directory, "second.txt")])(Deferred.succeed(secondFinished, undefined))
expect(yield* Deferred.isDone(secondFinished)).toBe(true)
yield* Deferred.succeed(releaseFirst, undefined)
yield* Fiber.join(first)
}).pipe(provide(directory)),
),
)
it.live("allows distinct absolute targets to proceed independently", () =>
withTmp((directory) =>
Effect.gen(function* () {

39
packages/core/test/tool-edit.test.ts
View file

@ -23,7 +23,7 @@ import { toolIdentity, executeTool, registerToolPlugin, toolDefinitions } from "
const editToolNode = makeLocationNode({
name: "test/edit-tool-plugin",
layer: Layer.effectDiscard(registerToolPlugin(EditTool.Plugin)),
deps: [Tool.node, LocationMutation.node, FileMutation.node, Formatter.node, FSUtil.node, Permission.node],
deps: [Tool.node, LocationMutation.node, FileMutation.node, Formatter.node, FSUtil.node, Location.node, Permission.node],
})
const sessionID = Session.ID.make("ses_edit_tool_test")
@ -645,6 +645,43 @@ describe("EditTool", () => {
),
)
it.live("serializes concurrent edit transactions", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const target = path.join(tmp.path, "concurrent.txt")
afterRead = () => (reads === 1 ? Effect.sleep("50 millis") : Effect.void)
return Effect.promise(() => fs.writeFile(target, "one\ntwo\n")).pipe(
Effect.andThen(
withTool(tmp.path, (registry) =>
Effect.all(
[
executeTool(
registry,
call({ path: "concurrent.txt", oldString: "one", newString: "ONE" }, "call-edit-one"),
),
executeTool(
registry,
call({ path: "concurrent.txt", oldString: "two", newString: "TWO" }, "call-edit-two"),
),
],
{ concurrency: "unbounded" },
),
),
),
Effect.andThen((results) =>
Effect.gen(function* () {
expect(results.map((result) => result.status)).toEqual(["completed", "completed"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("ONE\nTWO\n")
}),
),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
)
it.live("applies the edit when content changes after matching", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),

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

@ -7,6 +7,7 @@ import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
import { LayerNode } from "@opencode-ai/util/effect/layer-node"
import { FSUtil } from "@opencode-ai/util/fs-util"
import { Formatter } from "@opencode-ai/core/formatter"
import { FileMutation } from "@opencode-ai/core/file-mutation"
import { Location } from "@opencode-ai/core/location"
import { Permission } from "@opencode-ai/core/permission"
import { AbsolutePath } from "@opencode-ai/core/schema"
@ -22,7 +23,7 @@ import { toolIdentity, executeTool, registerToolPlugin, toolDefinitions } from "
const patchToolNode = makeLocationNode({
name: "test/patch-tool-plugin",
layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)),
deps: [Tool.node, Formatter.node, FSUtil.node, Location.node, Permission.node],
deps: [Tool.node, FileMutation.node, Formatter.node, FSUtil.node, Location.node, Permission.node],
})
const sessionID = Session.ID.make("ses_patch_tool_test")
@ -139,7 +140,7 @@ const withTool = <A, E, R>(
return yield* body(yield* Tool.Service)
}).pipe(
Effect.provide(
AppNodeBuilder.build(LayerNode.group([Tool.node, patchToolNode]), [
AppNodeBuilder.build(LayerNode.group([Tool.node, FileMutation.node, patchToolNode]), [
[FSUtil.node, filesystem],
[Location.node, activeLocation],
[Formatter.node, formatter],
@ -262,6 +263,45 @@ describe("PatchTool", () => {
),
)
it.live("serializes concurrent patch transactions", () =>
withTempTool((directory, registry) => {
const target = path.join(directory, "concurrent.txt")
afterEditApproval = () =>
assertions.filter((input) => input.action === "edit").length === 1
? Effect.sleep("50 millis")
: Effect.void
return Effect.promise(() => fs.writeFile(target, "one\ntwo\n")).pipe(
Effect.andThen(
Effect.all(
[
executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: concurrent.txt\n@@\n-one\n+ONE\n*** End Patch",
"call-patch-one",
),
),
executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: concurrent.txt\n@@\n-two\n+TWO\n*** End Patch",
"call-patch-two",
),
),
],
{ concurrency: "unbounded" },
),
),
Effect.andThen((results) =>
Effect.gen(function* () {
expect(results.map((result) => result.status)).toEqual(["completed", "completed"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("ONE\nTWO\n")
}),
),
)
}),
)
it.live("returns file diffs for final formatted content", () =>
withTempTool((directory, registry) => {
const target = path.join(directory, "formatted.txt")