Skip to content

Commit 2e1f5c0

Browse files
kitlangtonConanXu-math
authored andcommitted
refactor(file-time): effectify FileTimeService with Semaphore locks (anomalyco#17835)
1 parent 58dc1a6 commit 2e1f5c0

File tree

9 files changed

+147
-150
lines changed

9 files changed

+147
-150
lines changed

packages/opencode/src/effect/instances.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { QuestionService } from "@/question/service"
66
import { PermissionService } from "@/permission/service"
77
import { FileWatcherService } from "@/file/watcher"
88
import { VcsService } from "@/project/vcs"
9+
import { FileTimeService } from "@/file/time"
910
import { Instance } from "@/project/instance"
1011

1112
export { InstanceContext } from "./instance-context"
@@ -16,6 +17,7 @@ export type InstanceServices =
1617
| ProviderAuthService
1718
| FileWatcherService
1819
| VcsService
20+
| FileTimeService
1921

2022
function lookup(directory: string) {
2123
const project = Instance.project
@@ -24,8 +26,9 @@ function lookup(directory: string) {
2426
Layer.fresh(QuestionService.layer),
2527
Layer.fresh(PermissionService.layer),
2628
Layer.fresh(ProviderAuthService.layer),
27-
Layer.fresh(FileWatcherService.layer),
29+
Layer.fresh(FileWatcherService.layer).pipe(Layer.orDie),
2830
Layer.fresh(VcsService.layer),
31+
Layer.fresh(FileTimeService.layer).pipe(Layer.orDie),
2932
).pipe(Layer.provide(ctx))
3033
}
3134

packages/opencode/src/file/time.ts

Lines changed: 103 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,115 @@
1-
import { Instance } from "../project/instance"
21
import { Log } from "../util/log"
3-
import { Flag } from "../flag/flag"
2+
import { Flag } from "@/flag/flag"
43
import { Filesystem } from "../util/filesystem"
4+
import { Effect, Layer, ServiceMap, Semaphore } from "effect"
5+
import { runPromiseInstance } from "@/effect/runtime"
6+
import type { SessionID } from "@/session/schema"
57

6-
export namespace FileTime {
7-
const log = Log.create({ service: "file.time" })
8-
// Per-session read times plus per-file write locks.
9-
// All tools that overwrite existing files should run their
10-
// assert/read/write/update sequence inside withLock(filepath, ...)
11-
// so concurrent writes to the same file are serialized.
12-
export const state = Instance.state(() => {
13-
const read: {
14-
[sessionID: string]: {
15-
[path: string]: Date | undefined
16-
}
17-
} = {}
18-
const locks = new Map<string, Promise<void>>()
19-
return {
20-
read,
21-
locks,
22-
}
23-
})
24-
25-
export function read(sessionID: string, file: string) {
26-
log.info("read", { sessionID, file })
27-
const { read } = state()
28-
read[sessionID] = read[sessionID] || {}
29-
read[sessionID][file] = new Date()
8+
const log = Log.create({ service: "file.time" })
9+
10+
export namespace FileTimeService {
11+
export interface Service {
12+
readonly read: (sessionID: SessionID, file: string) => Effect.Effect<void>
13+
readonly get: (sessionID: SessionID, file: string) => Effect.Effect<Date | undefined>
14+
readonly assert: (sessionID: SessionID, filepath: string) => Effect.Effect<void>
15+
readonly withLock: <T>(filepath: string, fn: () => Promise<T>) => Effect.Effect<T>
3016
}
17+
}
3118

32-
export function get(sessionID: string, file: string) {
33-
return state().read[sessionID]?.[file]
19+
type Stamp = {
20+
readonly read: Date
21+
readonly mtime: number | undefined
22+
readonly ctime: number | undefined
23+
readonly size: number | undefined
24+
}
25+
26+
function stamp(file: string): Stamp {
27+
const stat = Filesystem.stat(file)
28+
const size = typeof stat?.size === "bigint" ? Number(stat.size) : stat?.size
29+
return {
30+
read: new Date(),
31+
mtime: stat?.mtime?.getTime(),
32+
ctime: stat?.ctime?.getTime(),
33+
size,
3434
}
35+
}
3536

36-
export async function withLock<T>(filepath: string, fn: () => Promise<T>): Promise<T> {
37-
const current = state()
38-
const currentLock = current.locks.get(filepath) ?? Promise.resolve()
39-
let release: () => void = () => {}
40-
const nextLock = new Promise<void>((resolve) => {
41-
release = resolve
42-
})
43-
const chained = currentLock.then(() => nextLock)
44-
current.locks.set(filepath, chained)
45-
await currentLock
46-
try {
47-
return await fn()
48-
} finally {
49-
release()
50-
if (current.locks.get(filepath) === chained) {
51-
current.locks.delete(filepath)
37+
function session(reads: Map<SessionID, Map<string, Stamp>>, sessionID: SessionID) {
38+
let value = reads.get(sessionID)
39+
if (!value) {
40+
value = new Map<string, Stamp>()
41+
reads.set(sessionID, value)
42+
}
43+
return value
44+
}
45+
46+
export class FileTimeService extends ServiceMap.Service<FileTimeService, FileTimeService.Service>()(
47+
"@opencode/FileTime",
48+
) {
49+
static readonly layer = Layer.effect(
50+
FileTimeService,
51+
Effect.gen(function* () {
52+
const disableCheck = yield* Flag.OPENCODE_DISABLE_FILETIME_CHECK
53+
const reads = new Map<SessionID, Map<string, Stamp>>()
54+
const locks = new Map<string, Semaphore.Semaphore>()
55+
56+
function getLock(filepath: string) {
57+
let lock = locks.get(filepath)
58+
if (!lock) {
59+
lock = Semaphore.makeUnsafe(1)
60+
locks.set(filepath, lock)
61+
}
62+
return lock
5263
}
53-
}
64+
65+
return FileTimeService.of({
66+
read: Effect.fn("FileTimeService.read")(function* (sessionID: SessionID, file: string) {
67+
log.info("read", { sessionID, file })
68+
session(reads, sessionID).set(file, stamp(file))
69+
}),
70+
71+
get: Effect.fn("FileTimeService.get")(function* (sessionID: SessionID, file: string) {
72+
return reads.get(sessionID)?.get(file)?.read
73+
}),
74+
75+
assert: Effect.fn("FileTimeService.assert")(function* (sessionID: SessionID, filepath: string) {
76+
if (disableCheck) return
77+
78+
const time = reads.get(sessionID)?.get(filepath)
79+
if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`)
80+
const next = stamp(filepath)
81+
const changed = next.mtime !== time.mtime || next.ctime !== time.ctime || next.size !== time.size
82+
83+
if (changed) {
84+
throw new Error(
85+
`File ${filepath} has been modified since it was last read.\nLast modification: ${new Date(next.mtime ?? next.read.getTime()).toISOString()}\nLast read: ${time.read.toISOString()}\n\nPlease read the file again before modifying it.`,
86+
)
87+
}
88+
}),
89+
90+
withLock: Effect.fn("FileTimeService.withLock")(function* <T>(filepath: string, fn: () => Promise<T>) {
91+
const lock = getLock(filepath)
92+
return yield* Effect.promise(fn).pipe(lock.withPermits(1))
93+
}),
94+
})
95+
}),
96+
)
97+
}
98+
99+
export namespace FileTime {
100+
export function read(sessionID: SessionID, file: string) {
101+
return runPromiseInstance(FileTimeService.use((s) => s.read(sessionID, file)))
102+
}
103+
104+
export function get(sessionID: SessionID, file: string) {
105+
return runPromiseInstance(FileTimeService.use((s) => s.get(sessionID, file)))
54106
}
55107

56-
export async function assert(sessionID: string, filepath: string) {
57-
if (Flag.OPENCODE_DISABLE_FILETIME_CHECK === true) {
58-
return
59-
}
60-
61-
const time = get(sessionID, filepath)
62-
if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`)
63-
const mtime = Filesystem.stat(filepath)?.mtime
64-
// Allow a 50ms tolerance for Windows NTFS timestamp fuzziness / async flushing
65-
if (mtime && mtime.getTime() > time.getTime() + 50) {
66-
throw new Error(
67-
`File ${filepath} has been modified since it was last read.\nLast modification: ${mtime.toISOString()}\nLast read: ${time.toISOString()}\n\nPlease read the file again before modifying it.`,
68-
)
69-
}
108+
export async function assert(sessionID: SessionID, filepath: string) {
109+
return runPromiseInstance(FileTimeService.use((s) => s.assert(sessionID, filepath)))
110+
}
111+
112+
export async function withLock<T>(filepath: string, fn: () => Promise<T>): Promise<T> {
113+
return runPromiseInstance(FileTimeService.use((s) => s.withLock(filepath, fn)))
70114
}
71115
}

packages/opencode/src/file/watcher.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ export class FileWatcherService extends ServiceMap.Service<FileWatcherService, F
7272
FileWatcherService,
7373
Effect.gen(function* () {
7474
const instance = yield* InstanceContext
75-
if (yield* Flag.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER) return FileWatcherService.of({ init })
75+
if (yield* Flag.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER)
76+
return FileWatcherService.of({ init })
7677

7778
log.info("init", { directory: instance.directory })
7879

packages/opencode/src/flag/flag.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ export namespace Flag {
6161
export const OPENCODE_EXPERIMENTAL_OXFMT = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_OXFMT")
6262
export const OPENCODE_EXPERIMENTAL_LSP_TY = truthy("OPENCODE_EXPERIMENTAL_LSP_TY")
6363
export const OPENCODE_EXPERIMENTAL_LSP_TOOL = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_LSP_TOOL")
64-
export const OPENCODE_DISABLE_FILETIME_CHECK = truthy("OPENCODE_DISABLE_FILETIME_CHECK")
64+
export const OPENCODE_DISABLE_FILETIME_CHECK = Config.boolean("OPENCODE_DISABLE_FILETIME_CHECK").pipe(
65+
Config.withDefault(false),
66+
)
6567
export const OPENCODE_EXPERIMENTAL_PLAN_MODE = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_PLAN_MODE")
6668
export const OPENCODE_EXPERIMENTAL_WORKSPACES = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_WORKSPACES")
6769
export const OPENCODE_EXPERIMENTAL_MARKDOWN = !falsy("OPENCODE_EXPERIMENTAL_MARKDOWN")

packages/opencode/src/session/prompt.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,7 @@ export namespace SessionPrompt {
12451245
]
12461246
}
12471247

1248-
FileTime.read(input.sessionID, filepath)
1248+
await FileTime.read(input.sessionID, filepath)
12491249
return [
12501250
{
12511251
messageID: info.id,

packages/opencode/src/tool/edit.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export const EditTool = Tool.define("edit", {
7878
file: filePath,
7979
event: existed ? "change" : "add",
8080
})
81-
FileTime.read(ctx.sessionID, filePath)
81+
await FileTime.read(ctx.sessionID, filePath)
8282
return
8383
}
8484

@@ -119,7 +119,7 @@ export const EditTool = Tool.define("edit", {
119119
diff = trimDiff(
120120
createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)),
121121
)
122-
FileTime.read(ctx.sessionID, filePath)
122+
await FileTime.read(ctx.sessionID, filePath)
123123
})
124124

125125
const filediff: Snapshot.FileDiff = {

packages/opencode/src/tool/read.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ export const ReadTool = Tool.define("read", {
214214

215215
// just warms the lsp client
216216
LSP.touchFile(filepath, false)
217-
FileTime.read(ctx.sessionID, filepath)
217+
await FileTime.read(ctx.sessionID, filepath)
218218

219219
if (instructions.length > 0) {
220220
output += `\n\n<system-reminder>\n${instructions.map((i) => i.content).join("\n\n")}\n</system-reminder>`

packages/opencode/src/tool/write.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const WriteTool = Tool.define("write", {
4949
file: filepath,
5050
event: exists ? "change" : "add",
5151
})
52-
FileTime.read(ctx.sessionID, filepath)
52+
await FileTime.read(ctx.sessionID, filepath)
5353

5454
let output = "Wrote file successfully."
5555
await LSP.touchFile(filepath, true)

0 commit comments

Comments
 (0)