Apply PR #17640: refactor(file-time): effectify FileTimeService with Semaphore locks

Author: opencode-agent[bot]

packages/opencode/src/effect/instances.ts

@@ -6,6 +6,7 @@ import { QuestionService } from "@/question/service"
 import { PermissionService } from "@/permission/service"
 import { FileWatcherService } from "@/file/watcher"
 import { VcsService } from "@/project/vcs"
+import { FileTimeService } from "@/file/time"
 import { Instance } from "@/project/instance"
 
 export { InstanceContext } from "./instance-context"
@@ -16,6 +17,7 @@ export type InstanceServices =
   | ProviderAuthService
   | FileWatcherService
   | VcsService
+  | FileTimeService
 
 function lookup(directory: string) {
   const project = Instance.project
@@ -24,8 +26,9 @@ function lookup(directory: string) {
     Layer.fresh(QuestionService.layer),
     Layer.fresh(PermissionService.layer),
     Layer.fresh(ProviderAuthService.layer),
-    Layer.fresh(FileWatcherService.layer),
+    Layer.fresh(FileWatcherService.layer).pipe(Layer.orDie),
     Layer.fresh(VcsService.layer),
+    Layer.fresh(FileTimeService.layer).pipe(Layer.orDie),
   ).pipe(Layer.provide(ctx))
 }
 

packages/opencode/src/file/time.ts

@@ -1,71 +1,88 @@
-import { Instance } from "../project/instance"
 import { Log } from "../util/log"
-import { Flag } from "../flag/flag"
+import { Flag } from "@/flag/flag"
 import { Filesystem } from "../util/filesystem"
+import { Effect, Layer, ServiceMap, Semaphore } from "effect"
+import { runPromiseInstance } from "@/effect/runtime"
 
-export namespace FileTime {
-  const log = Log.create({ service: "file.time" })
-  // Per-session read times plus per-file write locks.
-  // All tools that overwrite existing files should run their
-  // assert/read/write/update sequence inside withLock(filepath, ...)
-  // so concurrent writes to the same file are serialized.
-  export const state = Instance.state(() => {
-    const read: {
-      [sessionID: string]: {
-        [path: string]: Date | undefined
+const log = Log.create({ service: "file.time" })
+
+export namespace FileTimeService {
+  export interface Service {
+    readonly read: (sessionID: string, file: string) => Effect.Effect<void>
+    readonly get: (sessionID: string, file: string) => Effect.Effect<Date | undefined>
+    readonly assert: (sessionID: string, filepath: string) => Effect.Effect<void>
+    readonly withLock: <T>(filepath: string, fn: () => Promise<T>) => Effect.Effect<T>
+  }
+}
+
+export class FileTimeService extends ServiceMap.Service<FileTimeService, FileTimeService.Service>()(
+  "@opencode/FileTime",
+) {
+  static readonly layer = Layer.effect(
+    FileTimeService,
+    Effect.gen(function* () {
+      const disableCheck = yield* Flag.OPENCODE_DISABLE_FILETIME_CHECK
+      const reads: { [sessionID: string]: { [path: string]: Date | undefined } } = {}
+      const locks = new Map<string, Semaphore.Semaphore>()
+
+      function getLock(filepath: string) {
+        let lock = locks.get(filepath)
+        if (!lock) {
+          lock = Semaphore.makeUnsafe(1)
+          locks.set(filepath, lock)
+        }
+        return lock
       }
-    } = {}
-    const locks = new Map<string, Promise<void>>()
-    return {
-      read,
-      locks,
-    }
-  })
 
+      return FileTimeService.of({
+        read: Effect.fn("FileTimeService.read")(function* (sessionID: string, file: string) {
+          log.info("read", { sessionID, file })
+          reads[sessionID] = reads[sessionID] || {}
+          reads[sessionID][file] = new Date()
+        }),
+
+        get: Effect.fn("FileTimeService.get")(function* (sessionID: string, file: string) {
+          return reads[sessionID]?.[file]
+        }),
+
+        assert: Effect.fn("FileTimeService.assert")(function* (sessionID: string, filepath: string) {
+          if (disableCheck) return
+
+          const time = reads[sessionID]?.[filepath]
+          if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`)
+          const mtime = Filesystem.stat(filepath)?.mtime
+          if (mtime && mtime.getTime() > time.getTime() + 50) {
+            throw new Error(
+              `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.`,
+            )
+          }
+        }),
+
+        withLock: Effect.fn("FileTimeService.withLock")(function* <T>(filepath: string, fn: () => Promise<T>) {
+          const lock = getLock(filepath)
+          return yield* Effect.promise(fn).pipe(lock.withPermits(1))
+        }),
+      })
+    }),
+  )
+}
+
+// Legacy facade — callers don't need to change
+export namespace FileTime {
   export function read(sessionID: string, file: string) {
-    log.info("read", { sessionID, file })
-    const { read } = state()
-    read[sessionID] = read[sessionID] || {}
-    read[sessionID][file] = new Date()
+    // Fire-and-forget — callers never await this
+    runPromiseInstance(FileTimeService.use((s) => s.read(sessionID, file)))
   }
 
   export function get(sessionID: string, file: string) {
-    return state().read[sessionID]?.[file]
-  }
-
-  export async function withLock<T>(filepath: string, fn: () => Promise<T>): Promise<T> {
-    const current = state()
-    const currentLock = current.locks.get(filepath) ?? Promise.resolve()
-    let release: () => void = () => {}
-    const nextLock = new Promise<void>((resolve) => {
-      release = resolve
-    })
-    const chained = currentLock.then(() => nextLock)
-    current.locks.set(filepath, chained)
-    await currentLock
-    try {
-      return await fn()
-    } finally {
-      release()
-      if (current.locks.get(filepath) === chained) {
-        current.locks.delete(filepath)
-      }
-    }
+    return runPromiseInstance(FileTimeService.use((s) => s.get(sessionID, file)))
   }
 
   export async function assert(sessionID: string, filepath: string) {
-    if (Flag.OPENCODE_DISABLE_FILETIME_CHECK === true) {
-      return
-    }
+    return runPromiseInstance(FileTimeService.use((s) => s.assert(sessionID, filepath)))
+  }
 
-    const time = get(sessionID, filepath)
-    if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`)
-    const mtime = Filesystem.stat(filepath)?.mtime
-    // Allow a 50ms tolerance for Windows NTFS timestamp fuzziness / async flushing
-    if (mtime && mtime.getTime() > time.getTime() + 50) {
-      throw new Error(
-        `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.`,
-      )
-    }
+  export async function withLock<T>(filepath: string, fn: () => Promise<T>): Promise<T> {
+    return runPromiseInstance(FileTimeService.use((s) => s.withLock(filepath, fn)))
   }
 }

packages/opencode/src/file/watcher.ts

@@ -72,7 +72,8 @@ export class FileWatcherService extends ServiceMap.Service<FileWatcherService, F
     FileWatcherService,
     Effect.gen(function* () {
       const instance = yield* InstanceContext
-      if (yield* Flag.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER) return FileWatcherService.of({ init })
+      if (yield* Flag.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER)
+        return FileWatcherService.of({ init })
 
       log.info("init", { directory: instance.directory })
 

packages/opencode/src/flag/flag.ts

@@ -61,7 +61,9 @@ export namespace Flag {
   export const OPENCODE_EXPERIMENTAL_OXFMT = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_OXFMT")
   export const OPENCODE_EXPERIMENTAL_LSP_TY = truthy("OPENCODE_EXPERIMENTAL_LSP_TY")
   export const OPENCODE_EXPERIMENTAL_LSP_TOOL = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_LSP_TOOL")
-  export const OPENCODE_DISABLE_FILETIME_CHECK = truthy("OPENCODE_DISABLE_FILETIME_CHECK")
+  export const OPENCODE_DISABLE_FILETIME_CHECK = Config.boolean("OPENCODE_DISABLE_FILETIME_CHECK").pipe(
+    Config.withDefault(false),
+  )
   export const OPENCODE_EXPERIMENTAL_PLAN_MODE = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_PLAN_MODE")
   export const OPENCODE_EXPERIMENTAL_WORKSPACES = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_WORKSPACES")
   export const OPENCODE_EXPERIMENTAL_MARKDOWN = !falsy("OPENCODE_EXPERIMENTAL_MARKDOWN")