Skip to content

Commit fdd01e8

Browse files
authored
feat: read and write llm providers through the server (#2537)
* feat(app): read and write llm providers through the server useLlmProviders keeps its exact interface so both consumers, AI settings and chat target selection, are untouched. Underneath it is now a react-query-kit query over Hono RPC instead of extension storage. It gains an unavailable state. Previously an empty list meant the user had no providers, and the hook seeded the built-in one in response. Over HTTP a failed load looks the same as an empty one, so seeding moved into the fetcher where it can only run on a confirmed empty response, and AI settings now says the list could not be loaded rather than showing none. Saving a single-instance provider used to collapse earlier copies as a side effect of writing the whole list. That is now an explicit plan of one PUT and the deletes it displaces. The default provider id stays in extension storage. It is a per-profile preference and every profile shares one database, so a column would make them share a default too. A stale id costs nothing because it is resolved on read. Logout no longer deletes providers or scheduled jobs. That was right while they were account data synced to the cloud; they are now local data the account does not back. * fix(app): do not substitute a provider the caller did not choose An unreachable provider list returned an empty array, so a scheduled job that named a provider found no match and fell through to the built-in one. It ran on a different model with different credentials and was recorded as completed. The list being unreachable says nothing about whether that provider exists, so the two cases are now distinguished and naming a provider that cannot be loaded fails the run instead. A provider that was genuinely deleted still falls back, as before. Deleting a provider also persisted the replacement default before attempting the delete, so a failed delete left the provider configured but no longer default with nothing to show for it. The delete goes first; a default id left pointing at a deleted provider is repaired on read. * 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. * feat: move scheduled jobs and run history to the server (#2538) * feat(server): add local storage for scheduled job runs Job definitions had a table; their run history did not, so it was the one part of the domain with nowhere to live on this side. Runs cascade on job delete, unlike the job to provider reference which is set null. A job whose provider was removed is a job needing attention, whereas a run whose job was removed means nothing, and deleting a job already removed its runs before this table existed. The tool call log is a json column. Its input field is optional here where the extension has it required: an unknown already admits undefined, so the two describe the same values, and matching the validator avoids asserting the difference away at the route boundary. * feat(server): carry the per-job run cap across with the runs The extension kept fifteen runs per job, trimming as it created each one. Now that it no longer owns the history that policy has to live here, or the table grows without bound. It applies on every write rather than only on creation, which is bounded and idempotent, so it holds however the run was written. The import path does not prune, staying purely additive; the next real run trims. * feat(app): read and write scheduled jobs and runs through the server The hooks keep their shape, so the tasks page, the results view, the card and the new tab panel are unchanged apart from where they import from. Both gain an unavailable state, since an empty list and an unreachable server are now the same shape without one. The alarm runner distinguishes them everywhere it reads. Treating a failed load as an empty list would read as nothing being scheduled: alarms would not be rebuilt on startup and schedules would quietly stop firing, with no failed run to show for it. It skips the pass instead and retries on the next startup. Extension storage no longer carries the data, but it still carries the change signal. Runs are written by the background while the side panel and new tab display them, and storage watch is what kept those in step. A revision item is bumped after a write so every mounted view refetches. Run history is imported once, under its own marker. It cannot share the provider and job marker because that import must never run twice: extension storage is frozen now, so a second pass would insert back whatever the user has since deleted. Also removes the scheduled job deletion queue, whose only reader went when sync did, and the mount-time storage read that chose the opening tab, which is now derived so it settles when the history arrives. * fix(app): record a finished run against the current job Recording that a run finished wrote back the job as it was read before the run started. A run can take minutes and the job stays editable throughout, so a rename, a schedule change, a disable or a different provider chosen while it was going would be silently reverted. The old code merged into a freshly read list; passing the job object instead was an attempt to save a read and is what lost the update. It takes an id again, so a stale snapshot cannot be handed to it, and it skips the write when the job was deleted mid-run rather than resurrecting it.
1 parent bfa7e8b commit fdd01e8

38 files changed

Lines changed: 2935 additions & 490 deletions

packages/browseros-agent/apps/app/entrypoints/background/scheduledJobRuns.ts

Lines changed: 57 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,51 @@
11
import { onScheduleMessage } from '@/lib/messaging/schedules/scheduleMessages'
22
import { createAlarmFromJob } from '@/lib/schedules/createAlarmFromJob'
33
import { getChatServerResponse } from '@/lib/schedules/getChatServerResponse'
4-
import {
5-
scheduledJobRunStorage,
6-
scheduledJobStorage,
7-
} from '@/lib/schedules/scheduleStorage'
84
import type { ScheduledJobRun } from '@/lib/schedules/scheduleTypes'
5+
import {
6+
listScheduledJobRunsOrNull,
7+
listScheduledJobsOrNull,
8+
putScheduledJob,
9+
putScheduledJobRun,
10+
} from '@/modules/schedules/schedules.api'
11+
import { applyLastRunAt } from '@/modules/schedules/schedules.helpers'
912

10-
const MAX_RUNS_PER_JOB = 15
1113
const STALE_TIMEOUT_MS = 10 * 60 * 1000 // 10 minutes
1214
const TWENTY_FOUR_HOURS_MS = 24 * 60 * 60 * 1000
1315

1416
const runAbortControllers = new Map<string, AbortController>()
1517

1618
export const scheduledJobRuns = async () => {
19+
// Every read below distinguishes an unreachable server from an empty list.
20+
// Treating the two alike would look like "nothing is scheduled": alarms would
21+
// not be rebuilt on startup and schedules would quietly stop firing, with no
22+
// failed run to show for it. Skipping the pass instead leaves the next
23+
// startup to retry.
1724
const cleanupStaleJobRuns = async () => {
18-
const current = (await scheduledJobRunStorage.getValue()) ?? []
25+
const current = await listScheduledJobRunsOrNull()
26+
if (current === null) return
1927
const now = Date.now()
2028

21-
const updated = current.map((run) => {
22-
if (run.status !== 'running') return run
23-
24-
const startedAt = new Date(run.startedAt).getTime()
25-
if (now - startedAt > STALE_TIMEOUT_MS) {
26-
return {
27-
...run,
28-
status: 'failed' as const,
29-
completedAt: new Date().toISOString(),
30-
result: 'Job timed out!',
31-
}
32-
}
33-
return run
34-
})
29+
const stale = current.filter(
30+
(run) =>
31+
run.status === 'running' &&
32+
now - new Date(run.startedAt).getTime() > STALE_TIMEOUT_MS,
33+
)
3534

36-
await scheduledJobRunStorage.setValue(updated)
35+
for (const run of stale) {
36+
await putScheduledJobRun({
37+
...run,
38+
status: 'failed',
39+
completedAt: new Date().toISOString(),
40+
result: 'Job timed out!',
41+
})
42+
}
3743
}
3844

3945
const syncAlarmState = async () => {
40-
const jobs = (await scheduledJobStorage.getValue()).filter(
41-
(each) => each.enabled,
42-
)
46+
const loaded = await listScheduledJobsOrNull()
47+
if (loaded === null) return
48+
const jobs = loaded.filter((each) => each.enabled)
4349

4450
for (let i = 0; i < jobs.length; i++) {
4551
const job = jobs[i]
@@ -56,55 +62,46 @@ export const scheduledJobRuns = async () => {
5662
jobId: string,
5763
status: ScheduledJobRun['status'],
5864
): Promise<ScheduledJobRun> => {
65+
// Trimming to the per-job cap happens on the server now, so creating a run
66+
// no longer has to rewrite the job's whole history to stay bounded.
5967
const jobRun: ScheduledJobRun = {
6068
id: crypto.randomUUID(),
6169
jobId,
6270
startedAt: new Date().toISOString(),
6371
status,
6472
}
6573

66-
const current = (await scheduledJobRunStorage.getValue()) ?? []
67-
const otherJobRuns = current.filter((r) => r.jobId !== jobId)
68-
const thisJobRuns = current
69-
.filter((r) => r.jobId === jobId)
70-
.sort(
71-
(a, b) =>
72-
new Date(b.startedAt).getTime() - new Date(a.startedAt).getTime(),
73-
)
74-
.slice(0, MAX_RUNS_PER_JOB - 1)
75-
76-
await scheduledJobRunStorage.setValue([
77-
...otherJobRuns,
78-
...thisJobRuns,
79-
jobRun,
80-
])
74+
await putScheduledJobRun(jobRun)
8175
return jobRun
8276
}
8377

78+
// Takes the run rather than its id: the caller already holds it, and merging
79+
// locally avoids re-reading a list to update one row.
8480
const updateJobRun = async (
85-
runId: string,
81+
run: ScheduledJobRun,
8682
updates: Partial<Omit<ScheduledJobRun, 'id' | 'jobId' | 'startedAt'>>,
8783
) => {
88-
const current = (await scheduledJobRunStorage.getValue()) ?? []
89-
await scheduledJobRunStorage.setValue(
90-
current.map((r) => (r.id === runId ? { ...r, ...updates } : r)),
91-
)
84+
await putScheduledJobRun({ ...run, ...updates })
9285
}
9386

87+
// Takes an id, not the job: a snapshot captured before the run would be
88+
// minutes stale by the time this writes, and putting it back would revert any
89+
// edit made while the run was going.
9490
const updateJobLastRunAt = async (jobId: string) => {
95-
const current = (await scheduledJobStorage.getValue()) ?? []
96-
await scheduledJobStorage.setValue(
97-
current.map((j) =>
98-
j.id === jobId ? { ...j, lastRunAt: new Date().toISOString() } : j,
99-
),
100-
)
91+
const jobs = await listScheduledJobsOrNull()
92+
if (jobs === null) return
93+
94+
const updated = applyLastRunAt(jobs, jobId, new Date().toISOString())
95+
if (updated) await putScheduledJob(updated)
10196
}
10297

10398
const executeScheduledJob = async (jobId: string): Promise<void> => {
104-
const job = (await scheduledJobStorage.getValue()).find(
105-
(each) => each.id === jobId,
106-
)
99+
const jobs = await listScheduledJobsOrNull()
100+
if (jobs === null) {
101+
throw new Error('Cannot reach the BrowserOS server to load the job')
102+
}
107103

104+
const job = jobs.find((each) => each.id === jobId)
108105
if (!job) {
109106
throw new Error(`Job not found: ${jobId}`)
110107
}
@@ -120,7 +117,7 @@ export const scheduledJobRuns = async () => {
120117
providerId: job.providerId,
121118
})
122119

123-
await updateJobRun(jobRun.id, {
120+
await updateJobRun(jobRun, {
124121
status: 'completed',
125122
completedAt: new Date().toISOString(),
126123
result: response.text,
@@ -135,7 +132,7 @@ export const scheduledJobRuns = async () => {
135132
: e instanceof Error
136133
? e.message
137134
: String(e)
138-
await updateJobRun(jobRun.id, {
135+
await updateJobRun(jobRun, {
139136
status: 'failed',
140137
completedAt: new Date().toISOString(),
141138
result: errorMessage,
@@ -155,10 +152,11 @@ export const scheduledJobRuns = async () => {
155152
runningMissedJobs = true
156153

157154
try {
158-
const jobs = (await scheduledJobStorage.getValue()).filter(
159-
(j) => j.enabled,
160-
)
161-
const runs = (await scheduledJobRunStorage.getValue()) ?? []
155+
const loadedJobs = await listScheduledJobsOrNull()
156+
const runs = await listScheduledJobRunsOrNull()
157+
if (loadedJobs === null || runs === null) return
158+
159+
const jobs = loadedJobs.filter((j) => j.enabled)
162160
const now = Date.now()
163161
const cutoff = now - TWENTY_FOUR_HOURS_MS
164162

packages/browseros-agent/apps/app/lib/llm-providers/providerTemplates.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,19 @@ const DEFAULT_BASE_URLS: Record<ProviderType, string> = {
199199
* Get default base URL for a provider type
200200
* @public
201201
*/
202+
/**
203+
* Whether a stored type string is one this build understands.
204+
*
205+
* Keyed off DEFAULT_BASE_URLS because it is a `Record<ProviderType, string>`,
206+
* so the compiler keeps it exhaustive as the union changes. Used to filter
207+
* rows written by a newer build after a downgrade: icons, templates and base
208+
* URLs are all keyed by this union, so an unknown type would read as
209+
* undefined through every one of them.
210+
*/
211+
export function isProviderType(value: string): value is ProviderType {
212+
return Object.hasOwn(DEFAULT_BASE_URLS, value)
213+
}
214+
202215
export const getDefaultBaseUrlForProviders = (type: ProviderType): string => {
203216
return DEFAULT_BASE_URLS[type] || ''
204217
}

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ import { getAgentServerUrl } from '@/lib/browseros/helpers'
44
import {
55
createDefaultBrowserOSProvider,
66
defaultProviderIdStorage,
7-
providersStorage,
87
} from '@/lib/llm-providers/storage'
98
import type { LlmProviderConfig } from '@/lib/llm-providers/types'
109
import { mcpServerStorage } from '@/lib/mcp/mcpServerStorage'
1110
import { buildChatRequestBody } from '@/lib/messaging/server/buildChatRequestBody'
1211
import type { ChatMode } from '@/modules/chat/chat-types'
12+
import { listProvidersOrNull } from '@/modules/llm-providers/llm-providers.api'
1313
import {
1414
findChatProviderById,
1515
resolveChatProvider,
@@ -73,23 +73,42 @@ interface StreamParseState {
7373
receivedFinish: boolean
7474
}
7575

76-
const getDefaultProvider = async (): Promise<LlmProviderConfig | null> => {
77-
const providers = await providersStorage.getValue()
78-
if (!providers?.length) return null
79-
80-
const defaultProviderId = await defaultProviderIdStorage.getValue()
81-
return resolveChatProvider(providers, defaultProviderId)
82-
}
83-
8476
const resolveProvider = async (
8577
providerId?: string,
8678
): Promise<LlmProviderConfig> => {
79+
// One read for both branches: the list is now a request rather than a local
80+
// storage lookup, and the explicit-provider path used to fetch it twice.
81+
const loaded = await listProvidersOrNull()
82+
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) {
93+
throw new Error(
94+
'Cannot reach the BrowserOS server to load the selected provider',
95+
)
96+
}
97+
98+
const providers = loaded
99+
87100
if (providerId) {
88-
const providers = await providersStorage.getValue()
89-
const match = findChatProviderById(providers ?? [], providerId)
101+
const match = findChatProviderById(providers, providerId)
90102
if (match) return match
91103
}
92-
return (await getDefaultProvider()) ?? createDefaultBrowserOSProvider()
104+
105+
if (providers.length > 0) {
106+
const defaultProviderId = await defaultProviderIdStorage.getValue()
107+
const provider = resolveChatProvider(providers, defaultProviderId)
108+
if (provider) return provider
109+
}
110+
111+
return createDefaultBrowserOSProvider()
93112
}
94113

95114
export async function getChatServerResponse(

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ mock.module('@/lib/llm-providers/storage', () => ({
4646
},
4747
}))
4848

49+
// The provider list is a request now, not a storage read. Mocked here so the
50+
// fetch stub below still sees only the chat call it is asserting on.
51+
mock.module('@/modules/llm-providers/llm-providers.api', () => ({
52+
listProvidersOrNull: async () =>
53+
storageValues.has('unreachable')
54+
? null
55+
: ((storageValues.get('providers') as LlmProviderConfig[]) ?? []),
56+
}))
57+
4958
mock.module('@/lib/browseros/helpers', () => ({
5059
getAgentServerUrl: async () => 'http://127.0.0.1:9105',
5160
getMcpServerUrl: async () => 'http://127.0.0.1:9106/mcp',
@@ -160,3 +169,73 @@ const providers: LlmProviderConfig[] = [
160169
updatedAt: timestamp,
161170
},
162171
]
172+
173+
describe('provider resolution when the server is unreachable', () => {
174+
// The list being unreachable says nothing about whether the chosen provider
175+
// exists, so running anyway would spend the wrong credentials on the wrong
176+
// model. The job runner turns this into a failed run the user can see.
177+
it('fails a scheduled job that named a provider', async () => {
178+
storageValues.set('unreachable', true)
179+
const { getChatServerResponse } = await import('./getChatServerResponse')
180+
181+
await expect(
182+
getChatServerResponse({
183+
message: 'Run my schedule',
184+
providerId: 'anthropic-sonnet',
185+
}),
186+
).rejects.toThrow('Cannot reach the BrowserOS server')
187+
188+
expect(fetchBodies).toHaveLength(0)
189+
})
190+
191+
it('fails a refine that named a provider', async () => {
192+
storageValues.set('unreachable', true)
193+
const { refinePrompt } = await import('./refine-prompt')
194+
195+
await expect(
196+
refinePrompt({
197+
prompt: 'Check mail',
198+
name: 'Morning brief',
199+
providerId: 'anthropic-sonnet',
200+
}),
201+
).rejects.toThrow('Cannot reach the BrowserOS server')
202+
})
203+
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 () => {
208+
storageValues.set('unreachable', true)
209+
const { getChatServerResponse } = await import('./getChatServerResponse')
210+
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+
224+
await getChatServerResponse({ message: 'Run my schedule' })
225+
226+
expect(fetchBodies[0]).toMatchObject({ provider: 'browseros' })
227+
})
228+
229+
// A provider that was genuinely deleted still falls back, as before. Only
230+
// the unreachable case is treated as unsafe.
231+
it('falls back when the named provider no longer exists', async () => {
232+
const { getChatServerResponse } = await import('./getChatServerResponse')
233+
234+
await getChatServerResponse({
235+
message: 'Run my schedule',
236+
providerId: 'deleted-provider',
237+
})
238+
239+
expect(fetchBodies[0]).toMatchObject({ provider: 'anthropic' })
240+
})
241+
})

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import { getAgentServerUrl } from '@/lib/browseros/helpers'
22
import {
33
createDefaultBrowserOSProvider,
44
defaultProviderIdStorage,
5-
providersStorage,
65
} from '@/lib/llm-providers/storage'
76
import type { LlmProviderConfig } from '@/lib/llm-providers/types'
7+
import { listProvidersOrNull } from '@/modules/llm-providers/llm-providers.api'
88
import {
99
findChatProviderById,
1010
resolveChatProvider,
@@ -13,8 +13,18 @@ import {
1313
const resolveProvider = async (
1414
providerId?: string,
1515
): Promise<LlmProviderConfig> => {
16-
const providers = await providersStorage.getValue()
17-
if (providers?.length) {
16+
const loaded = await listProvidersOrNull()
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) {
21+
throw new Error(
22+
'Cannot reach the BrowserOS server to load the selected provider',
23+
)
24+
}
25+
26+
const providers = loaded
27+
if (providers.length) {
1828
const explicitProvider = findChatProviderById(providers, providerId)
1929
if (explicitProvider) return explicitProvider
2030

0 commit comments

Comments
 (0)