Skip to content

Commit d39ed27

Browse files
committed
fix(app): never resolve a provider from a list that failed to load
The previous guard only covered a job that named a provider, which left the same hole one step over. A job that names none still has a choice behind it: the configured default, whose id lives in extension storage but whose model and credentials live in the list. So an unreachable list sent those runs to the built-in provider and recorded them completed, which is the case this guard existed to prevent. The condition drops to the list itself, which also states the invariant plainly. An empty list keeps the fallback, because that is the server answering that it genuinely has no providers rather than not answering.
1 parent 12d9afe commit d39ed27

3 files changed

Lines changed: 33 additions & 11 deletions

File tree

packages/browseros-agent/apps/app/lib/schedules/getChatServerResponse.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,22 @@ const resolveProvider = async (
8080
// storage lookup, and the explicit-provider path used to fetch it twice.
8181
const loaded = await listProvidersOrNull()
8282

83-
// A job that named a provider must not quietly run on a different one. The
84-
// list being unreachable says nothing about whether that provider exists, so
85-
// substituting the built-in would spend the wrong credentials on the wrong
86-
// model and still record the run as completed.
87-
if (providerId && loaded === null) {
83+
// Never resolve a provider from a list that failed to load. A job that named
84+
// one must not quietly run on a different one, and a job that named none
85+
// still has a choice behind it: the configured default, whose id lives in
86+
// extension storage but whose credentials and model live in that list. Either
87+
// way, substituting the built-in would spend the wrong credentials on the
88+
// wrong model and still record the run as completed.
89+
//
90+
// An empty list is a different answer and keeps the fallback: the server
91+
// answered, and it really has no providers.
92+
if (loaded === null) {
8893
throw new Error(
8994
'Cannot reach the BrowserOS server to load the selected provider',
9095
)
9196
}
9297

93-
const providers = loaded ?? []
98+
const providers = loaded
9499

95100
if (providerId) {
96101
const match = findChatProviderById(providers, providerId)

packages/browseros-agent/apps/app/lib/schedules/provider-resolution.test.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,26 @@ describe('provider resolution when the server is unreachable', () => {
201201
).rejects.toThrow('Cannot reach the BrowserOS server')
202202
})
203203

204-
// No provider was named, so there is no choice to betray and the built-in
205-
// fallback is the behaviour this always had.
206-
it('falls back to the built-in provider when none was named', async () => {
204+
// A job that named nothing still has a choice behind it: the configured
205+
// default. Its id is in extension storage but its model and credentials are
206+
// in the list that failed to load, so the built-in is not a safe stand-in.
207+
it('fails a scheduled job that relies on the configured default', async () => {
207208
storageValues.set('unreachable', true)
208209
const { getChatServerResponse } = await import('./getChatServerResponse')
209210

211+
await expect(
212+
getChatServerResponse({ message: 'Run my schedule' }),
213+
).rejects.toThrow('Cannot reach the BrowserOS server')
214+
215+
expect(fetchBodies).toHaveLength(0)
216+
})
217+
218+
// An empty list is a different answer from an unreachable one: the server
219+
// replied and really has no providers, so the built-in is correct.
220+
it('still falls back to the built-in provider when the server has none', async () => {
221+
storageValues.set('providers', [])
222+
const { getChatServerResponse } = await import('./getChatServerResponse')
223+
210224
await getChatServerResponse({ message: 'Run my schedule' })
211225

212226
expect(fetchBodies[0]).toMatchObject({ provider: 'browseros' })

packages/browseros-agent/apps/app/lib/schedules/refine-prompt.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@ const resolveProvider = async (
1414
providerId?: string,
1515
): Promise<LlmProviderConfig> => {
1616
const loaded = await listProvidersOrNull()
17-
if (providerId && loaded === null) {
17+
// Same rule as the scheduled run: the configured default is a choice too, and
18+
// its model and credentials are in the list that failed to load. Callers here
19+
// already catch and surface this.
20+
if (loaded === null) {
1821
throw new Error(
1922
'Cannot reach the BrowserOS server to load the selected provider',
2023
)
2124
}
2225

23-
const providers = loaded ?? []
26+
const providers = loaded
2427
if (providers.length) {
2528
const explicitProvider = findChatProviderById(providers, providerId)
2629
if (explicitProvider) return explicitProvider

0 commit comments

Comments
 (0)