Skip to content

Commit add731c

Browse files
committed
fix(acp): resolve cold-start race with belt-and-suspenders defense
Two complementary fixes for the ACP cold-start race condition where DB migration delays provider loading, causing defaultModel() to fall through to Provider.sort() and pick a paid model. Proactive fix (acp.ts): - Poll providers endpoint (max 10x, 500ms intervals) before accepting requests, ensuring providers are loaded post-migration - Log warning if providers don't load within 5s Defensive fix (agent.ts): - When user specifies a model in config.json but the provider is not yet loaded (or provider loaded but model not indexed), return the user's explicit choice instead of falling through to Provider.sort() Extracted testable modules: - resolve-model.ts: pure resolution logic (6 unit tests) - wait-for-providers.ts: polling loop with configurable retry (4 unit tests) Fixes #865, Fixes #866
1 parent 6ff8ee7 commit add731c

6 files changed

Lines changed: 136 additions & 6 deletions

File tree

packages/opencode/src/acp/agent.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import { ACPSessionManager } from "./session"
3838
import type { ACPConfig } from "./types"
3939
import { Provider } from "../provider"
4040
import { ModelID, ProviderID } from "../provider/schema"
41+
import { resolveDefaultModel } from "./resolve-model"
4142
import { Agent as AgentModule } from "../agent/agent"
4243
import { AppRuntime } from "@/effect/app-runtime"
4344
import { Installation } from "@/installation"
@@ -1563,12 +1564,8 @@ async function defaultModel(config: ACPConfig, cwd?: string): Promise<{ provider
15631564
return []
15641565
})
15651566

1566-
if (specified && providers.length) {
1567-
const provider = providers.find((p) => p.id === specified.providerID)
1568-
if (provider && provider.models[specified.modelID]) return specified
1569-
}
1570-
1571-
if (specified && !providers.length) return specified
1567+
const resolved = resolveDefaultModel(specified, providers)
1568+
if (resolved) return resolved
15721569

15731570
const opencodeProvider = providers.find((p) => p.id === "opencode")
15741571
if (opencodeProvider) {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { ModelID, ProviderID } from "../provider/schema"
2+
3+
/**
4+
* Pure resolution logic for choosing the default model.
5+
* Separated for testability — no SDK calls, no side effects.
6+
*/
7+
export function resolveDefaultModel(
8+
specified: { providerID: ProviderID; modelID: ModelID } | undefined,
9+
providers: Array<{ id: string; models: Record<string, unknown> }>,
10+
): { providerID: ProviderID; modelID: ModelID } | undefined {
11+
if (specified && providers.length) {
12+
const provider = providers.find((p) => p.id === specified.providerID)
13+
if (provider && provider.models[specified.modelID]) return specified
14+
// Provider not yet loaded or model not found — still honor the user's explicit choice
15+
// rather than falling through to Provider.sort() which may pick a paid model
16+
if (!provider || !provider.models[specified.modelID]) return specified
17+
}
18+
19+
if (specified && !providers.length) return specified
20+
21+
return undefined // let caller handle fallback
22+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { Log } from "../util"
2+
3+
const log = Log.create({ service: "acp-command" })
4+
5+
/**
6+
* Wait for at least one provider to be loaded.
7+
* Returns true if providers loaded, false if timed out.
8+
*/
9+
export async function waitForProviders(
10+
poll: () => Promise<{ data?: { providers?: unknown[] } }>,
11+
opts: { maxAttempts?: number; intervalMs?: number } = {},
12+
): Promise<boolean> {
13+
const { maxAttempts = 10, intervalMs = 500 } = opts
14+
for (let i = 0; i < maxAttempts; i++) {
15+
const resp = await poll()
16+
if (resp.data?.providers?.length) return true
17+
await new Promise((r) => setTimeout(r, intervalMs))
18+
}
19+
log.warn("providers not loaded within timeout, proceeding anyway")
20+
return false
21+
}

packages/opencode/src/cli/cmd/acp.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { ACP } from "@/acp/agent"
66
import { Server } from "@/server/server"
77
import { createOpencodeClient } from "@mimo-ai/sdk/v2"
88
import { withNetworkOptions, resolveNetworkOptions } from "../network"
9+
import { waitForProviders } from "@/acp/wait-for-providers"
910

1011
const log = Log.create({ service: "acp-command" })
1112

@@ -29,6 +30,13 @@ export const AcpCommand = cmd({
2930
baseUrl: `http://${server.hostname}:${server.port}`,
3031
})
3132

33+
// Wait for providers to be fully loaded after potential DB migration.
34+
// Without this, the first request may race with provider initialization
35+
// and fall back to Provider.sort() which picks a paid model.
36+
await waitForProviders(() =>
37+
sdk.config.providers({ directory: args.cwd }, { throwOnError: false }),
38+
)
39+
3240
const input = new WritableStream<Uint8Array>({
3341
write(chunk) {
3442
return new Promise<void>((resolve, reject) => {
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { test, expect, describe } from "bun:test"
2+
import { resolveDefaultModel } from "../../src/acp/resolve-model"
3+
import { ProviderID, ModelID } from "../../src/provider/schema"
4+
5+
describe("resolveDefaultModel", () => {
6+
const mimo = {
7+
providerID: ProviderID.make("mimo"),
8+
modelID: ModelID.make("mimo-auto"),
9+
}
10+
11+
test("returns specified when provider is not in providers list", () => {
12+
const result = resolveDefaultModel(mimo, [
13+
{ id: "xiaomi", models: { "some-model": {} } },
14+
])
15+
expect(result).toEqual(mimo)
16+
})
17+
18+
test("returns specified when provider exists but model not indexed", () => {
19+
const result = resolveDefaultModel(mimo, [
20+
{ id: "mimo", models: {} },
21+
])
22+
expect(result).toEqual(mimo)
23+
})
24+
25+
test("returns specified when providers list is empty", () => {
26+
const result = resolveDefaultModel(mimo, [])
27+
expect(result).toEqual(mimo)
28+
})
29+
30+
test("returns specified when provider and model both present", () => {
31+
const result = resolveDefaultModel(mimo, [
32+
{ id: "mimo", models: { "mimo-auto": { id: "mimo-auto" } } },
33+
])
34+
expect(result).toEqual(mimo)
35+
})
36+
37+
test("returns undefined when no specified model", () => {
38+
const result = resolveDefaultModel(undefined, [
39+
{ id: "mimo", models: { "mimo-auto": {} } },
40+
])
41+
expect(result).toBeUndefined()
42+
})
43+
44+
test("returns undefined when no specified and no providers", () => {
45+
const result = resolveDefaultModel(undefined, [])
46+
expect(result).toBeUndefined()
47+
})
48+
})
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { test, expect, describe } from "bun:test"
2+
import { waitForProviders } from "../../src/acp/wait-for-providers"
3+
4+
describe("waitForProviders", () => {
5+
test("returns true immediately when providers are available", async () => {
6+
const poll = () => Promise.resolve({ data: { providers: [{ id: "mimo" }] } })
7+
const result = await waitForProviders(poll, { maxAttempts: 3, intervalMs: 10 })
8+
expect(result).toBe(true)
9+
})
10+
11+
test("retries and returns true when providers load on Nth attempt", async () => {
12+
let calls = 0
13+
const poll = () => {
14+
calls++
15+
if (calls < 3) return Promise.resolve({ data: { providers: [] } })
16+
return Promise.resolve({ data: { providers: [{ id: "mimo" }] } })
17+
}
18+
const result = await waitForProviders(poll, { maxAttempts: 5, intervalMs: 10 })
19+
expect(result).toBe(true)
20+
expect(calls).toBe(3)
21+
})
22+
23+
test("returns false after exhausting all attempts", async () => {
24+
const poll = () => Promise.resolve({ data: { providers: [] } })
25+
const result = await waitForProviders(poll, { maxAttempts: 3, intervalMs: 10 })
26+
expect(result).toBe(false)
27+
})
28+
29+
test("handles undefined data gracefully", async () => {
30+
const poll = () => Promise.resolve({ data: undefined })
31+
const result = await waitForProviders(poll, { maxAttempts: 2, intervalMs: 10 })
32+
expect(result).toBe(false)
33+
})
34+
})

0 commit comments

Comments
 (0)