Skip to content

Commit 098a921

Browse files
committed
refactor(models): use Flock.effect, makeRuntime, cachedInvalidateWithTTL
Three review findings + a real bug: 1. Replace 'Effect.promise(() => Flock.withLock(... runPromise(fetchApi) ...))' with 'Effect.scoped(Effect.gen { Flock.effect(lockKey); fetchAndWrite })'. Flock.effect is the existing scoped acquire/release; the runPromise re-entry inside the Promise lock body was dropping the parent fiber's tracing span, scope, and interruption. 2. Replace hand-rolled 'ManagedRuntime.make(defaultLayer, { memoMap })' with the existing 'makeRuntime(Service, defaultLayer)' helper from src/effect/run-service.ts. Same pattern as bus, sync, instance-store, etc. 3. Use 'Effect.cachedInvalidateWithTTL(populate, Duration.infinity)' for single-flight cache + manual invalidation. Concurrent first-callers now share one populate; refresh() invalidates so the next get() re-populates. 4. Restore eager initial refresh — Schedule.fixed waits the duration before the first run, which would have lost the original 'void refresh()' on module load. Now: refresh().pipe(Effect.andThen(refresh().repeat(...))). 5. Simplify fresh() mtime check — fs.stat's info.mtime is always Option<Date>; dropped the dead Option.isOption guard. 6. Drop unnecessary Effect.fnUntraced on loadFromDisk/loadSnapshot (these are values, not traced effects). Replaced loadSnapshot's manual try/catch wrapper with Effect.tryPromise + Effect.catch. 7. Effect.orDie on populate — Service interface declares get returns Effect<Record<string, Provider>> (E = never); failures from HttpClient, FileSystem, JSON.parse must die rather than silently leak. Smoke + provider tests pass.
1 parent ca8fd0a commit 098a921

1 file changed

Lines changed: 51 additions & 51 deletions

File tree

packages/opencode/src/provider/models.ts

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { Global } from "@opencode-ai/core/global"
22
import path from "path"
3-
import { Context, Duration, Effect, Layer, ManagedRuntime, Option, Schedule, Schema } from "effect"
4-
import { memoMap } from "@opencode-ai/core/effect/memo-map"
3+
import { Context, Duration, Effect, Layer, Option, Schedule, Schema } from "effect"
54
import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http"
65
import { Installation } from "../installation"
76
import { Flag } from "@opencode-ai/core/flag/flag"
87
import { Flock } from "@opencode-ai/core/util/flock"
98
import { Hash } from "@opencode-ai/core/util/hash"
109
import { AppFileSystem } from "@opencode-ai/core/filesystem"
10+
import { makeRuntime } from "@/effect/run-service"
1111
import { withTransientReadRetry } from "@/util/effect-http-client"
1212

1313
const Cost = Schema.Struct({
@@ -111,12 +111,10 @@ export const layer: Layer.Layer<Service, never, AppFileSystem.Service | HttpClie
111111
const ttl = Duration.minutes(5)
112112
const lockKey = `models-dev:${filepath}`
113113

114-
let cached: Record<string, Provider> | undefined
115-
116114
const fresh = Effect.fnUntraced(function* () {
117115
const stat = yield* fs.stat(filepath).pipe(Effect.catch(() => Effect.succeed(undefined)))
118-
if (!stat?.mtime) return false
119-
const mtime = Option.isOption(stat.mtime) ? Number(Option.getOrElse(stat.mtime, () => new Date(0)).getTime()) : 0
116+
if (!stat) return false
117+
const mtime = Option.getOrElse(stat.mtime, () => new Date(0)).getTime()
120118
return Date.now() - mtime < Duration.toMillis(ttl)
121119
})
122120

@@ -129,67 +127,69 @@ export const layer: Layer.Layer<Service, never, AppFileSystem.Service | HttpClie
129127
)
130128
})
131129

132-
const loadFromDisk = Effect.fnUntraced(function* () {
133-
return yield* fs
134-
.readJson(Flag.OPENCODE_MODELS_PATH ?? filepath)
135-
.pipe(Effect.catch(() => Effect.succeed(undefined))) as Effect.Effect<
136-
Record<string, Provider> | undefined
137-
>
138-
})
130+
const loadFromDisk = fs
131+
.readJson(Flag.OPENCODE_MODELS_PATH ?? filepath)
132+
.pipe(
133+
Effect.catch(() => Effect.succeed(undefined)),
134+
Effect.map((v) => v as Record<string, Provider> | undefined),
135+
)
139136

140-
const loadSnapshot = Effect.promise(async () => {
141-
try {
142-
// @ts-ignore — generated at build time, may not exist in dev
143-
const m = await import("./models-snapshot.js")
144-
return m.snapshot as Record<string, Provider> | undefined
145-
} catch {
146-
return undefined
147-
}
137+
// Bundled snapshot fallback — present in built releases, missing in dev.
138+
const loadSnapshot = Effect.tryPromise({
139+
// @ts-ignore — generated at build time, may not exist in dev
140+
try: () => import("./models-snapshot.js").then((m) => m.snapshot as Record<string, Provider> | undefined),
141+
catch: () => undefined,
142+
}).pipe(Effect.catch(() => Effect.succeed(undefined)))
143+
144+
// Cross-process file lock — Flock is the right primitive (in-process
145+
// semaphores can't coordinate concurrent opencode CLIs writing the same cache).
146+
const fetchAndWrite = Effect.fn("ModelsDev.fetchAndWrite")(function* () {
147+
const text = yield* fetchApi()
148+
yield* fs.writeWithDirs(filepath, text)
149+
return text
148150
})
149151

150-
const populate = Effect.fn("ModelsDev.populate")(function* () {
151-
const fromDisk = yield* loadFromDisk()
152+
const populate = Effect.gen(function* () {
153+
const fromDisk = yield* loadFromDisk
152154
if (fromDisk) return fromDisk
153155
const snapshot = yield* loadSnapshot
154156
if (snapshot) return snapshot
155157
if (Flag.OPENCODE_DISABLE_MODELS_FETCH) return {}
156-
157-
// Cross-process file lock — Flock is the right primitive (in-process
158-
// semaphores can't coordinate concurrent opencode CLIs writing the same cache).
159-
return yield* Effect.promise(() =>
160-
Flock.withLock(lockKey, async () => {
161-
const text = await Effect.runPromise(fetchApi())
162-
await Effect.runPromise(fs.writeWithDirs(filepath, text))
158+
return yield* Effect.scoped(
159+
Effect.gen(function* () {
160+
yield* Flock.effect(lockKey)
161+
const text = yield* fetchAndWrite()
163162
return JSON.parse(text) as Record<string, Provider>
164163
}),
165164
)
166-
})
165+
}).pipe(Effect.withSpan("ModelsDev.populate"), Effect.orDie)
167166

168-
const get = Effect.fn("ModelsDev.get")(function* () {
169-
if (cached) return cached
170-
cached = yield* populate()
171-
return cached
172-
})
167+
// Single-flight cache: concurrent first-call dedupes via the cached effect.
168+
// refresh() invalidates so the next get() re-populates from disk.
169+
const [cachedGet, invalidate] = yield* Effect.cachedInvalidateWithTTL(populate, Duration.infinity)
170+
171+
const get = (): Effect.Effect<Record<string, Provider>> => cachedGet
173172

174173
const refresh = Effect.fn("ModelsDev.refresh")(function* (force = false) {
175-
if (!force && (yield* fresh())) {
176-
cached = undefined
177-
return
178-
}
179-
yield* Effect.promise(() =>
180-
Flock.withLock(lockKey, async () => {
181-
const text = await Effect.runPromise(fetchApi())
182-
await Effect.runPromise(fs.writeWithDirs(filepath, text))
174+
if (!force && (yield* fresh())) return yield* invalidate
175+
yield* Effect.scoped(
176+
Effect.gen(function* () {
177+
yield* Flock.effect(lockKey)
178+
if (!force && (yield* fresh())) return
179+
yield* fetchAndWrite()
183180
}),
184181
).pipe(
185182
Effect.tapCause((cause) => Effect.logError("Failed to fetch models.dev", { cause })),
186183
Effect.ignore,
187184
)
188-
cached = undefined
185+
yield* invalidate
189186
})
190187

191188
if (!Flag.OPENCODE_DISABLE_MODELS_FETCH && !process.argv.includes("--get-yargs-completions")) {
192-
yield* Effect.forkScoped(refresh().pipe(Effect.repeat(Schedule.fixed("60 minutes")), Effect.ignore))
189+
// Eager initial refresh + 1 hour cadence (matches pre-Service behavior).
190+
yield* Effect.forkScoped(
191+
refresh().pipe(Effect.andThen(refresh().pipe(Effect.repeat(Schedule.fixed("60 minutes")))), Effect.ignore),
192+
)
193193
}
194194

195195
return Service.of({ get, refresh })
@@ -202,10 +202,10 @@ export const defaultLayer: Layer.Layer<Service> = layer.pipe(
202202
)
203203

204204
// Promise-style compat for callers in Promise-context (Hono routes, legacy CLI handlers).
205-
// Uses the shared memoMap so this runtime's Service instance is shared with AppRuntime
206-
// — Effect callers that yield ModelsDev.Service see the same cache.
207-
const promiseRuntime = ManagedRuntime.make(defaultLayer, { memoMap })
208-
export const get = () => promiseRuntime.runPromise(Service.use((s) => s.get()))
209-
export const refresh = (force = false) => promiseRuntime.runPromise(Service.use((s) => s.refresh(force)))
205+
// makeRuntime uses the shared memoMap so this runtime's Service instance is the same one
206+
// AppRuntime sees — Effect callers and Promise callers operate on the same cache.
207+
const runtime = makeRuntime(Service, defaultLayer)
208+
export const get = () => runtime.runPromise((s) => s.get())
209+
export const refresh = (force = false) => runtime.runPromise((s) => s.refresh(force))
210210

211211
export * as ModelsDev from "./models"

0 commit comments

Comments
 (0)