Skip to content

Commit abf435f

Browse files
opencode-agent[bot]opencode-agent[bot]
committed
Apply PR #17640: refactor(file-time): effectify FileTimeService with Semaphore locks
2 parents 7ccd91c + 86aa331 commit abf435f

File tree

13 files changed

+360
-221
lines changed

13 files changed

+360
-221
lines changed

‎packages/opencode/src/effect/instance-context.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { ServiceMap } from "effect"
2+
import type { Project } from "@/project/project"
3+
4+
export declare namespace InstanceContext {
5+
export interface Shape {
6+
readonly directory: string
7+
readonly project: Project.Info
8+
}
9+
}
10+
11+
export class InstanceContext extends ServiceMap.Service<InstanceContext, InstanceContext.Shape>()(
12+
"opencode/InstanceContext",
13+
) {}

‎packages/opencode/src/effect/instances.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,23 @@
11
import { Effect, Layer, LayerMap, ServiceMap } from "effect"
22
import { registerDisposer } from "./instance-registry"
3+
import { InstanceContext } from "./instance-context"
34
import { ProviderAuthService } from "@/provider/auth-service"
45
import { QuestionService } from "@/question/service"
56
import { PermissionService } from "@/permission/service"
67
import { FileWatcherService } from "@/file/watcher"
8+
import { VcsService } from "@/project/vcs"
9+
import { FileTimeService } from "@/file/time"
710
import { Instance } from "@/project/instance"
8-
import type { Project } from "@/project/project"
911

10-
export declare namespace InstanceContext {
11-
export interface Shape {
12-
readonly directory: string
13-
readonly project: Project.Info
14-
}
15-
}
16-
17-
export class InstanceContext extends ServiceMap.Service<InstanceContext, InstanceContext.Shape>()(
18-
"opencode/InstanceContext",
19-
) {}
12+
export { InstanceContext } from "./instance-context"
2013

21-
export type InstanceServices = QuestionService | PermissionService | ProviderAuthService | FileWatcherService
14+
export type InstanceServices =
15+
| QuestionService
16+
| PermissionService
17+
| ProviderAuthService
18+
| FileWatcherService
19+
| VcsService
20+
| FileTimeService
2221

2322
function lookup(directory: string) {
2423
const project = Instance.project
@@ -27,7 +26,9 @@ function lookup(directory: string) {
2726
Layer.fresh(QuestionService.layer),
2827
Layer.fresh(PermissionService.layer),
2928
Layer.fresh(ProviderAuthService.layer),
30-
Layer.fresh(FileWatcherService.layer),
29+
Layer.fresh(FileWatcherService.layer).pipe(Layer.orDie),
30+
Layer.fresh(VcsService.layer),
31+
Layer.fresh(FileTimeService.layer).pipe(Layer.orDie),
3132
).pipe(Layer.provide(ctx))
3233
}
3334

‎packages/opencode/src/file/time.ts‎

Lines changed: 73 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,88 @@
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"
56

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
7+
const log = Log.create({ service: "file.time" })
8+
9+
export namespace FileTimeService {
10+
export interface Service {
11+
readonly read: (sessionID: string, file: string) => Effect.Effect<void>
12+
readonly get: (sessionID: string, file: string) => Effect.Effect<Date | undefined>
13+
readonly assert: (sessionID: string, filepath: string) => Effect.Effect<void>
14+
readonly withLock: <T>(filepath: string, fn: () => Promise<T>) => Effect.Effect<T>
15+
}
16+
}
17+
18+
export class FileTimeService extends ServiceMap.Service<FileTimeService, FileTimeService.Service>()(
19+
"@opencode/FileTime",
20+
) {
21+
static readonly layer = Layer.effect(
22+
FileTimeService,
23+
Effect.gen(function* () {
24+
const disableCheck = yield* Flag.OPENCODE_DISABLE_FILETIME_CHECK
25+
const reads: { [sessionID: string]: { [path: string]: Date | undefined } } = {}
26+
const locks = new Map<string, Semaphore.Semaphore>()
27+
28+
function getLock(filepath: string) {
29+
let lock = locks.get(filepath)
30+
if (!lock) {
31+
lock = Semaphore.makeUnsafe(1)
32+
locks.set(filepath, lock)
33+
}
34+
return lock
1635
}
17-
} = {}
18-
const locks = new Map<string, Promise<void>>()
19-
return {
20-
read,
21-
locks,
22-
}
23-
})
2436

37+
return FileTimeService.of({
38+
read: Effect.fn("FileTimeService.read")(function* (sessionID: string, file: string) {
39+
log.info("read", { sessionID, file })
40+
reads[sessionID] = reads[sessionID] || {}
41+
reads[sessionID][file] = new Date()
42+
}),
43+
44+
get: Effect.fn("FileTimeService.get")(function* (sessionID: string, file: string) {
45+
return reads[sessionID]?.[file]
46+
}),
47+
48+
assert: Effect.fn("FileTimeService.assert")(function* (sessionID: string, filepath: string) {
49+
if (disableCheck) return
50+
51+
const time = reads[sessionID]?.[filepath]
52+
if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`)
53+
const mtime = Filesystem.stat(filepath)?.mtime
54+
if (mtime && mtime.getTime() > time.getTime() + 50) {
55+
throw new Error(
56+
`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.`,
57+
)
58+
}
59+
}),
60+
61+
withLock: Effect.fn("FileTimeService.withLock")(function* <T>(filepath: string, fn: () => Promise<T>) {
62+
const lock = getLock(filepath)
63+
return yield* Effect.promise(fn).pipe(lock.withPermits(1))
64+
}),
65+
})
66+
}),
67+
)
68+
}
69+
70+
// Legacy facade — callers don't need to change
71+
export namespace FileTime {
2572
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()
73+
// Fire-and-forget — callers never await this
74+
runPromiseInstance(FileTimeService.use((s) => s.read(sessionID, file)))
3075
}
3176

3277
export function get(sessionID: string, file: string) {
33-
return state().read[sessionID]?.[file]
34-
}
35-
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)
52-
}
53-
}
78+
return runPromiseInstance(FileTimeService.use((s) => s.get(sessionID, file)))
5479
}
5580

5681
export async function assert(sessionID: string, filepath: string) {
57-
if (Flag.OPENCODE_DISABLE_FILETIME_CHECK === true) {
58-
return
59-
}
82+
return runPromiseInstance(FileTimeService.use((s) => s.assert(sessionID, filepath)))
83+
}
6084

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-
}
85+
export async function withLock<T>(filepath: string, fn: () => Promise<T>): Promise<T> {
86+
return runPromiseInstance(FileTimeService.use((s) => s.withLock(filepath, fn)))
7087
}
7188
}

‎packages/opencode/src/file/watcher.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { BusEvent } from "@/bus/bus-event"
22
import { Bus } from "@/bus"
3-
import { InstanceContext } from "@/effect/instances"
3+
import { InstanceContext } from "@/effect/instance-context"
44
import { Instance } from "@/project/instance"
55
import z from "zod"
66
import { Log } from "../util/log"
@@ -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/permission/service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Bus } from "@/bus"
22
import { BusEvent } from "@/bus/bus-event"
3-
import { InstanceContext } from "@/effect/instances"
3+
import { InstanceContext } from "@/effect/instance-context"
44
import { ProjectID } from "@/project/schema"
55
import { MessageID, SessionID } from "@/session/schema"
66
import { PermissionTable } from "@/session/session.sql"

‎packages/opencode/src/project/bootstrap.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Project } from "./project"
77
import { Bus } from "../bus"
88
import { Command } from "../command"
99
import { Instance } from "./instance"
10-
import { Vcs } from "./vcs"
10+
import { VcsService } from "./vcs"
1111
import { Log } from "@/util/log"
1212
import { ShareNext } from "@/share/share-next"
1313
import { Snapshot } from "../snapshot"
@@ -22,7 +22,7 @@ export async function InstanceBootstrap() {
2222
await LSP.init()
2323
await runPromiseInstance(FileWatcherService.use((service) => service.init()))
2424
File.init()
25-
Vcs.init()
25+
await runPromiseInstance(VcsService.use((s) => s.init()))
2626
Snapshot.init()
2727
Truncate.init()
2828

‎packages/opencode/src/project/vcs.ts‎

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { BusEvent } from "@/bus/bus-event"
22
import { Bus } from "@/bus"
3-
import path from "path"
43
import z from "zod"
54
import { Log } from "@/util/log"
65
import { Instance } from "./instance"
6+
import { InstanceContext } from "@/effect/instance-context"
77
import { FileWatcher } from "@/file/watcher"
88
import { git } from "@/util/git"
9+
import { Effect, Layer, ServiceMap } from "effect"
910

1011
const log = Log.create({ service: "vcs" })
1112

@@ -27,50 +28,57 @@ export namespace Vcs {
2728
ref: "VcsInfo",
2829
})
2930
export type Info = z.infer<typeof Info>
31+
}
3032

31-
async function currentBranch() {
32-
const result = await git(["rev-parse", "--abbrev-ref", "HEAD"], {
33-
cwd: Instance.worktree,
34-
})
35-
if (result.exitCode !== 0) return
36-
const text = result.text().trim()
37-
if (!text) return
38-
return text
33+
export namespace VcsService {
34+
export interface Service {
35+
readonly init: () => Effect.Effect<void>
36+
readonly branch: () => Effect.Effect<string | undefined>
3937
}
38+
}
4039

41-
const state = Instance.state(
42-
async () => {
43-
if (Instance.project.vcs !== "git") {
44-
return { branch: async () => undefined, unsubscribe: undefined }
45-
}
46-
let current = await currentBranch()
47-
log.info("initialized", { branch: current })
40+
export class VcsService extends ServiceMap.Service<VcsService, VcsService.Service>()("@opencode/Vcs") {
41+
static readonly layer = Layer.effect(
42+
VcsService,
43+
Effect.gen(function* () {
44+
const instance = yield* InstanceContext
45+
let current: string | undefined
4846

49-
const unsubscribe = Bus.subscribe(FileWatcher.Event.Updated, async (evt) => {
50-
if (!evt.properties.file.endsWith("HEAD")) return
51-
const next = await currentBranch()
52-
if (next !== current) {
53-
log.info("branch changed", { from: current, to: next })
54-
current = next
55-
Bus.publish(Event.BranchUpdated, { branch: next })
47+
if (instance.project.vcs === "git") {
48+
const currentBranch = async () => {
49+
const result = await git(["rev-parse", "--abbrev-ref", "HEAD"], {
50+
cwd: instance.project.worktree,
51+
})
52+
if (result.exitCode !== 0) return undefined
53+
const text = result.text().trim()
54+
return text || undefined
5655
}
57-
})
5856

59-
return {
60-
branch: async () => current,
61-
unsubscribe,
62-
}
63-
},
64-
async (state) => {
65-
state.unsubscribe?.()
66-
},
67-
)
57+
current = yield* Effect.promise(() => currentBranch())
58+
log.info("initialized", { branch: current })
6859

69-
export async function init() {
70-
return state()
71-
}
60+
const unsubscribe = Bus.subscribe(
61+
FileWatcher.Event.Updated,
62+
Instance.bind(async (evt) => {
63+
if (!evt.properties.file.endsWith("HEAD")) return
64+
const next = await currentBranch()
65+
if (next !== current) {
66+
log.info("branch changed", { from: current, to: next })
67+
current = next
68+
Bus.publish(Vcs.Event.BranchUpdated, { branch: next })
69+
}
70+
}),
71+
)
7272

73-
export async function branch() {
74-
return await state().then((s) => s.branch())
75-
}
73+
yield* Effect.addFinalizer(() => Effect.sync(unsubscribe))
74+
}
75+
76+
return VcsService.of({
77+
init: Effect.fn("VcsService.init")(function* () {}),
78+
branch: Effect.fn("VcsService.branch")(function* () {
79+
return current
80+
}),
81+
})
82+
}),
83+
)
7684
}

‎packages/opencode/src/server/server.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import { LSP } from "../lsp"
1414
import { Format } from "../format"
1515
import { TuiRoutes } from "./routes/tui"
1616
import { Instance } from "../project/instance"
17-
import { Vcs } from "../project/vcs"
17+
import { Vcs, VcsService } from "../project/vcs"
18+
import { runPromiseInstance } from "@/effect/runtime"
1819
import { Agent } from "../agent/agent"
1920
import { Skill } from "../skill/skill"
2021
import { Auth } from "../auth"
@@ -337,7 +338,7 @@ export namespace Server {
337338
},
338339
}),
339340
async (c) => {
340-
const branch = await Vcs.branch()
341+
const branch = await runPromiseInstance(VcsService.use((s) => s.branch()))
341342
return c.json({
342343
branch,
343344
})

0 commit comments

Comments
 (0)