Skip to content

Commit 0b3e0d8

Browse files
committed
fix(core): validate model catalog responses
1 parent 5061a91 commit 0b3e0d8

2 files changed

Lines changed: 42 additions & 23 deletions

File tree

packages/core/src/models-dev.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ export const Provider = Schema.Struct({
131131

132132
export type Provider = Schema.Schema.Type<typeof Provider>
133133

134+
const Catalog = Schema.Record(Schema.String, Provider)
135+
134136
export const Event = ModelsDev.Event
135137

136138
declare const OPENCODE_MODELS_DEV: Record<string, Provider> | undefined
@@ -182,6 +184,7 @@ const layer = Layer.effect(
182184
})
183185

184186
const loadFromDisk = fs.readJson(Flag.OPENCODE_MODELS_PATH ?? filepath).pipe(
187+
Effect.flatMap(Schema.decodeUnknownEffect(Catalog)),
185188
Effect.catch((error) => {
186189
if (
187190
Flag.OPENCODE_MODELS_PATH === undefined &&
@@ -201,6 +204,7 @@ const layer = Layer.effect(
201204

202205
const fetchAndWrite = Effect.fn("ModelsDev.fetchAndWrite")(function* () {
203206
const text = yield* fetchApi()
207+
const catalog = yield* Schema.decodeUnknownEffect(Schema.fromJsonString(Catalog))(text)
204208
const tempfile = `${filepath}.${process.pid}.${Date.now()}.tmp`
205209
yield* fs.writeWithDirs(tempfile, text).pipe(
206210
Effect.andThen(fs.rename(tempfile, filepath)),
@@ -211,7 +215,7 @@ const layer = Layer.effect(
211215
}),
212216
),
213217
)
214-
return text
218+
return catalog
215219
})
216220

217221
const populate = Effect.gen(function* () {
@@ -221,13 +225,12 @@ const layer = Layer.effect(
221225
if (snapshot) return snapshot
222226
if (Flag.OPENCODE_DISABLE_MODELS_FETCH) return {}
223227
// Flock is cross-process: concurrent opencode CLIs can race on this cache file.
224-
const text = yield* Effect.scoped(
228+
return yield* Effect.scoped(
225229
Effect.gen(function* () {
226230
yield* Flock.effect(lockKey)
227231
return yield* fetchAndWrite()
228232
}),
229233
)
230-
return JSON.parse(text) as Record<string, Provider>
231234
}).pipe(
232235
Effect.withSpan("ModelsDev.populate"),
233236
Effect.catch((error) =>

packages/core/test/models.test.ts

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,18 @@ const writeCache = (data: object, mtimeMs?: number) => writeCacheText(JSON.strin
112112
const provided = <A, E>(state: Ref.Ref<MockState>, eff: Effect.Effect<A, E, ModelsDev.Service>) =>
113113
eff.pipe(Effect.provide(buildLayer(state)))
114114

115+
const withFetch = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
116+
Effect.acquireUseRelease(
117+
Effect.sync(() => {
118+
Flag.OPENCODE_DISABLE_MODELS_FETCH = false
119+
}),
120+
() => effect,
121+
() =>
122+
Effect.sync(() => {
123+
Flag.OPENCODE_DISABLE_MODELS_FETCH = true
124+
}),
125+
)
126+
115127
beforeEach(async () => {
116128
await rm(cacheFile, { force: true })
117129
})
@@ -159,16 +171,7 @@ describe("ModelsDev Service", () => {
159171
yield* writeCacheText("{")
160172
const state = yield* Ref.make({ ...initialState, body: JSON.stringify(fixture2) })
161173
const context = yield* Layer.build(buildLayer(state))
162-
const result = yield* Effect.acquireUseRelease(
163-
Effect.sync(() => {
164-
Flag.OPENCODE_DISABLE_MODELS_FETCH = false
165-
}),
166-
() => ModelsDev.Service.use((s) => s.get()).pipe(Effect.provide(context)),
167-
() =>
168-
Effect.sync(() => {
169-
Flag.OPENCODE_DISABLE_MODELS_FETCH = true
170-
}),
171-
)
174+
const result = yield* withFetch(ModelsDev.Service.use((s) => s.get()).pipe(Effect.provide(context)))
172175
expect(result).toEqual(fixture2)
173176
expect(yield* Effect.promise(() => readFile(cacheFile, "utf8"))).toBe(JSON.stringify(fixture2))
174177
const final = yield* Ref.get(state)
@@ -180,21 +183,34 @@ describe("ModelsDev Service", () => {
180183
Effect.gen(function* () {
181184
const state = yield* Ref.make({ ...initialState, status: 503 })
182185
const context = yield* Layer.build(buildLayer(state))
183-
const result = yield* Effect.acquireUseRelease(
184-
Effect.sync(() => {
185-
Flag.OPENCODE_DISABLE_MODELS_FETCH = false
186-
}),
187-
() => ModelsDev.Service.use((s) => s.get()).pipe(Effect.provide(context)),
188-
() =>
189-
Effect.sync(() => {
190-
Flag.OPENCODE_DISABLE_MODELS_FETCH = true
191-
}),
192-
)
186+
const result = yield* withFetch(ModelsDev.Service.use((s) => s.get()).pipe(Effect.provide(context)))
193187
expect(result).toEqual({})
194188
expect((yield* Ref.get(state)).calls.length).toBe(3)
195189
}),
196190
)
197191

192+
it.live("get() returns an empty catalog when the response is malformed JSON", () =>
193+
Effect.gen(function* () {
194+
const state = yield* Ref.make({ ...initialState, body: "{" })
195+
const context = yield* Layer.build(buildLayer(state))
196+
const result = yield* withFetch(ModelsDev.Service.use((s) => s.get()).pipe(Effect.provide(context)))
197+
expect(result).toEqual({})
198+
expect((yield* Ref.get(state)).calls.length).toBe(1)
199+
expect(yield* Effect.promise(() => Bun.file(cacheFile).exists())).toBe(false)
200+
}),
201+
)
202+
203+
it.live("get() returns an empty catalog when the response has an invalid shape", () =>
204+
Effect.gen(function* () {
205+
const state = yield* Ref.make({ ...initialState, body: JSON.stringify({ acme: {} }) })
206+
const context = yield* Layer.build(buildLayer(state))
207+
const result = yield* withFetch(ModelsDev.Service.use((s) => s.get()).pipe(Effect.provide(context)))
208+
expect(result).toEqual({})
209+
expect((yield* Ref.get(state)).calls.length).toBe(1)
210+
expect(yield* Effect.promise(() => Bun.file(cacheFile).exists())).toBe(false)
211+
}),
212+
)
213+
198214
it.live("get() is single-flight under concurrent calls", () =>
199215
Effect.gen(function* () {
200216
yield* writeCache(fixture)

0 commit comments

Comments
 (0)