feat: unify the provider tables and resolve the chat provider on the server (#2540)

* feat(server): merge acp agents and llm providers into one table

Both are providers for the chat, and everything above the database already
said so: the chat target is one union with a kind on the client, and the
wire target has always been a discriminated union. Only storage disagreed,
and it charged for the mismatch. Selecting an acp agent left the default
provider pointing at the previously selected llm one, because the write was
conditional on kind. A scheduled job could reference an llm provider and
nothing else, so picking Claude Code in chat was possible while scheduling
against it was not.

Migration 0010 creates the unified table and copies both sources in; 0011
repoints scheduled jobs and drops the old tables. Two migrations rather than
one, so the copy proves itself before anything is dropped, and because
drizzle cannot diff a simultaneous add and remove without a rename prompt.

Drizzle put its drops ahead of its own foreign key pragma, which fired the
ON DELETE SET NULL on scheduled jobs and silently unlinked every job from
its provider. Reordered so the drops happen after the rebuild, inside the
block where enforcement is off.

The default provider now lives on a column, and a partial unique index
admits exactly one row, of either kind. Not keyed by profile: sqlite treats
nulls as distinct in a unique index, so pairing it with the unset profile
column would let every row be default at once.

Also brings the packaged-build schema fallback current. It had drifted four
migrations behind, so a build without migration files would have created a
database with none of this epic's tables. The test's copy of the migration
history is now derived from the journal, since a hand written duplicate is
what let the drift go unnoticed.

* feat: resolve the chat provider on the server

The provider block is gone from the chat body. A request names an id, or
names nothing and gets whichever provider is selected, and the server fills
in the model, endpoint and credentials from the row. Sixteen fields of
provider configuration collapse into one, and the api key, the aws secret
and the session token stop crossing the wire on every message.

Every one of those fields is still accepted. The extension updates
independently of the browser binary, so a shipped build can be running a
client that sends the whole configuration inline; the server stops
requiring them, not accepting them, and a row it does not recognise leaves
whatever the client sent in place.

The selection moved to the server with the tables it points at, so choosing
a coding agent now records it. It could not before: the default lived in
extension storage and only ever named an llm provider, so picking an agent
left it pointing at the provider chosen before it.

The scheduled runner drops its provider lookup entirely, and the guard that
came with it. That guard existed because an unreachable list and an empty
one looked alike, so a job could run on the built-in provider with the
wrong credentials and still be recorded as a success. There is nothing to
tell apart now: the job names an id and the server resolves it.

Refine prompt still resolves on the client. It posts to its own endpoint
with its own schema, and giving it the same treatment is separate work.

* fix(server): gate chat on trust when the server supplies the credentials

A browseros chat request is deliberately allowed without the app-origin
check, on the reasoning that it carries its own credentials and so can only
spend what the caller already had. Resolving the provider from storage broke
that reasoning: naming an id, or naming nothing and taking the selected
provider, would have let any local caller spend the user's key against an
external service.

The check now applies exactly when the configuration came from a stored row.
A request that brought its own is as unrestricted as it was before, so the
capability that reasoning was about is untouched.

* fix(server): gate chat on every path where the server holds the credential

The previous gate keyed on whether a stored provider row was read, on the
reasoning that a request naming no known row must have brought its own key.
That is false for four provider types. The oauth three take a token from
this machine's oauth store and browseros takes the gateway credential, so
naming one with an unknown id skipped the check and had the server hand
over a credential the caller never held.

The predicate lives beside resolveLLMConfig, since it has to mirror those
branches exactly and would drift if the route kept its own copy.

This particular hole predates the change: the exemption for browseros
requests and the credential injection behind it were both already there. It
is fixed here because the gate added alongside it claims the ungated path is
safe, and that claim has to hold.

The chat integration test now sends the origin header the extension always
sends. It was relying on the exemption this closes, and the routes the
background alarm runner already calls carry that header today.

* test(server): stop a module mock dropping the exports it does not name

CI failed with `Export named 'SERVER_CREDENTIALED_PROVIDERS' not found`
against a file that plainly exports it. A module factory is a total
replacement, so everything it omits disappears for every file importing that
module afterwards, and bun's registry is process wide. Adding an export to a
module someone mocks partially is enough to break a different file entirely.

The factory now spreads the real module and overrides only the function under
test. This does not reproduce locally: file ordering is stable on APFS and
not on ext4, which the test runner's own notes call out as the reason this
class of failure kills CI while local runs pass.

Author: DaniAkash

packages/browseros-agent/apps/app/lib/messaging/server/buildChatRequestBody.test.ts

@@ -25,7 +25,46 @@ describe('buildChatRequestBody', () => {
       type: 'browseros',
       providerId: 'provider-1',
     })
-    expect(body.providerId).toBe('provider-1')
+  })
+
+  // The provider is named and nothing more. Its model, endpoint and
+  // credentials are resolved from the id server side, so an api key and an aws
+  // secret no longer cross the wire on every message.
+  it('sends no provider configuration or credentials', () => {
+    const body = buildChatRequestBody({
+      conversationId: '6ff46e3b-e45a-40a4-9157-ca520e800f43',
+      provider: {
+        ...provider,
+        id: 'bedrock-1',
+        type: 'bedrock',
+        apiKey: 'sk-secret',
+        accessKeyId: 'AKIA',
+        secretAccessKey: 'aws-secret',
+        sessionToken: 'token',
+        baseUrl: 'https://example.com',
+      },
+    })
+
+    for (const field of [
+      'apiKey',
+      'accessKeyId',
+      'secretAccessKey',
+      'sessionToken',
+      'baseUrl',
+      'model',
+      'provider',
+      'providerId',
+      'providerType',
+      'providerName',
+      'temperature',
+      'contextWindowSize',
+      'region',
+      'resourceName',
+    ]) {
+      expect(field in body).toBe(false)
+    }
+    expect(JSON.stringify(body)).not.toContain('aws-secret')
+    expect(JSON.stringify(body)).not.toContain('sk-secret')
   })
 
   it('preserves browser context and chat metadata', () => {

packages/browseros-agent/apps/app/lib/messaging/server/buildChatRequestBody.ts

@@ -40,7 +40,16 @@ export interface ChatRequestBrowserContext {
 
 export interface ChatRequestBodyParams {
   conversationId: string
-  provider: LlmProviderConfig
+  /**
+   * The provider config, when the caller already holds it. Only the id is sent;
+   * the rest is used to describe what the chosen model can do, which comes from
+   * a catalogue the extension bundles.
+   *
+   * Callers that hold nothing but an id, such as the scheduled runner, pass
+   * `providerId` instead and let the server resolve the rest.
+   */
+  provider?: LlmProviderConfig
+  providerId?: string
   message?: string
   mode?: ChatMode
   browserContext?: ChatRequestBrowserContext
@@ -61,6 +70,7 @@ export interface ChatRequestBodyParams {
 export const buildChatRequestBody = ({
   conversationId,
   provider,
+  providerId,
   message = '',
   mode,
   browserContext,
@@ -74,31 +84,26 @@ export const buildChatRequestBody = ({
   selectedTextSource,
   isScheduledTask,
 }: ChatRequestBodyParams) => ({
-  target: { type: 'browseros' as const, providerId: provider.id },
+  // The provider is named, not described. The server holds the list and which
+  // one is selected, so it resolves the model, endpoint and credentials from
+  // the id. Those used to travel on every message, which meant the api key and
+  // the aws secret crossed the wire each time the user pressed send.
+  target: {
+    type: 'browseros' as const,
+    // Absent when the caller has neither, which tells the server to use the
+    // selected provider.
+    providerId: provider?.id ?? providerId,
+  },
   message,
-  provider: provider.type,
-  providerId: provider.id,
-  providerType: provider.type,
-  providerName: provider.name,
-  apiKey: provider.apiKey,
-  baseUrl: provider.baseUrl,
   conversationId,
-  model: provider.modelId ?? 'default',
   mode,
-  contextWindowSize: provider.contextWindow,
-  temperature: provider.temperature,
-  resourceName: provider.resourceName,
-  accessKeyId: provider.accessKeyId,
-  secretAccessKey: provider.secretAccessKey,
-  region: provider.region,
-  sessionToken: provider.sessionToken,
-  reasoningEffort: provider.reasoningEffort,
-  reasoningSummary: provider.reasoningSummary,
   browserContext,
   userSystemPrompt,
   userWorkingDir,
-  supportsImages: supportsImages ?? provider.supportsImages,
-  supportsReasoning: resolvesSupportsReasoning(provider),
+  // Sent because the caller can override what the provider says, and because
+  // the reasoning answer comes from a model catalogue the extension bundles.
+  supportsImages: supportsImages ?? provider?.supportsImages,
+  supportsReasoning: provider ? resolvesSupportsReasoning(provider) : undefined,
   previousConversation,
   historyMode,
   declinedApps: declinedApps?.length ? declinedApps : undefined,

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

@@ -1,19 +1,9 @@
 import { chatErrorMessage } from '@browseros/shared/schemas/chat-error'
 import { createParser, type EventSourceMessage } from 'eventsource-parser'
 import { getAgentServerUrl } from '@/lib/browseros/helpers'
-import {
-  createDefaultBrowserOSProvider,
-  defaultProviderIdStorage,
-} from '@/lib/llm-providers/storage'
-import type { LlmProviderConfig } from '@/lib/llm-providers/types'
 import { mcpServerStorage } from '@/lib/mcp/mcpServerStorage'
 import { buildChatRequestBody } from '@/lib/messaging/server/buildChatRequestBody'
 import type { ChatMode } from '@/modules/chat/chat-types'
-import { listProvidersOrNull } from '@/modules/llm-providers/llm-providers.api'
-import {
-  findChatProviderById,
-  resolveChatProvider,
-} from '../llm-providers/provider-runtime'
 import { personalizationStorage } from '../personalization/personalizationStorage'
 import { scheduleSystemPrompt } from './scheduleSystemPrompt'
 import type { ToolCallExecution } from './scheduleTypes'
@@ -73,49 +63,15 @@ interface StreamParseState {
   receivedFinish: boolean
 }
 
-const resolveProvider = async (
-  providerId?: string,
-): Promise<LlmProviderConfig> => {
-  // One read for both branches: the list is now a request rather than a local
-  // storage lookup, and the explicit-provider path used to fetch it twice.
-  const loaded = await listProvidersOrNull()
-
-  // Never resolve a provider from a list that failed to load. A job that named
-  // one must not quietly run on a different one, and a job that named none
-  // still has a choice behind it: the configured default, whose id lives in
-  // extension storage but whose credentials and model live in that list. Either
-  // way, substituting the built-in would spend the wrong credentials on the
-  // wrong model and still record the run as completed.
-  //
-  // An empty list is a different answer and keeps the fallback: the server
-  // answered, and it really has no providers.
-  if (loaded === null) {
-    throw new Error(
-      'Cannot reach the BrowserOS server to load the selected provider',
-    )
-  }
-
-  const providers = loaded
-
-  if (providerId) {
-    const match = findChatProviderById(providers, providerId)
-    if (match) return match
-  }
-
-  if (providers.length > 0) {
-    const defaultProviderId = await defaultProviderIdStorage.getValue()
-    const provider = resolveChatProvider(providers, defaultProviderId)
-    if (provider) return provider
-  }
-
-  return createDefaultBrowserOSProvider()
-}
-
 export async function getChatServerResponse(
   request: ChatServerRequest,
 ): Promise<ChatServerResponse> {
   const agentServerUrl = await getAgentServerUrl()
-  const provider = await resolveProvider(request.providerId)
+  // No provider lookup here any more. The server holds the list and the
+  // selection, so a job names an id or names nothing and the server resolves
+  // it. That also removes the guard this path needed when it did the lookup
+  // itself: an unreachable list could not be told apart from an empty one, so
+  // a job risked running on the built-in provider with the wrong credentials.
   const conversationId = request.conversationId ?? crypto.randomUUID()
   const personalization = await personalizationStorage.getValue()
 
@@ -138,9 +94,9 @@ export async function getChatServerResponse(
     body: JSON.stringify({
       messages: [{ role: 'user', content: request.message }],
       ...buildChatRequestBody({
+        providerId: request.providerId,
         message: request.message,
         conversationId,
-        provider,
         mode: request.mode ?? 'agent',
         browserContext:
           request.activeTab ||
@@ -157,7 +113,6 @@ export async function getChatServerResponse(
               }
             : undefined,
         userSystemPrompt: `${personalization}\n${scheduleSystemPrompt}`,
-        supportsImages: provider.supportsImages,
         isScheduledTask: true,
       }),
     }),

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

@@ -78,6 +78,22 @@ mock.module('../personalization/personalizationStorage', () => ({
   },
 }))
 
+// Only the refine path still resolves a provider on this side; the scheduled
+// path names an id and lets the server do it.
+const providers: LlmProviderConfig[] = [
+  {
+    id: 'anthropic-sonnet',
+    type: 'anthropic',
+    name: 'Anthropic Sonnet',
+    modelId: 'claude-sonnet-4-6',
+    supportsImages: true,
+    contextWindow: 200000,
+    temperature: 0.2,
+    createdAt: 0,
+    updatedAt: 0,
+  },
+]
+
 beforeEach(() => {
   storageValues.clear()
   fetchBodies.length = 0
@@ -106,7 +122,12 @@ afterEach(() => {
 })
 
 describe('scheduled provider resolution', () => {
-  it('uses an explicit scheduled provider', async () => {
+  // The runner names the provider and stops there. Its model, endpoint and
+  // credentials are resolved from the id on the server, which is what let the
+  // client-side lookup and its unreachable-versus-empty guard go: the guard
+  // existed because a failed lookup and an empty list looked the same, and a
+  // job risked running on the built-in provider with the wrong credentials.
+  it('names the provider and sends no configuration', async () => {
     const { getChatServerResponse } = await import('./getChatServerResponse')
 
     await getChatServerResponse({
@@ -115,10 +136,24 @@ describe('scheduled provider resolution', () => {
     })
 
     expect(fetchBodies[0]).toMatchObject({
-      provider: 'anthropic',
-      providerName: 'Anthropic Sonnet',
-      model: 'claude-sonnet-4-6',
+      target: { type: 'browseros', providerId: 'anthropic-sonnet' },
+      isScheduledTask: true,
     })
+    for (const field of ['apiKey', 'model', 'provider', 'baseUrl']) {
+      expect(field in fetchBodies[0]).toBe(false)
+    }
+  })
+
+  // A job created without picking a provider names none, and the server uses
+  // whichever is selected.
+  it('leaves the provider unnamed when the job has none', async () => {
+    const { getChatServerResponse } = await import('./getChatServerResponse')
+
+    await getChatServerResponse({ message: 'Run my schedule' })
+
+    expect(
+      (fetchBodies[0] as { target: { providerId?: string } }).target.providerId,
+    ).toBeUndefined()
   })
 
   it('uses an explicit refine provider', async () => {
@@ -141,101 +176,3 @@ describe('scheduled provider resolution', () => {
     })
   })
 })
-
-const timestamp = 1000
-
-const providers: LlmProviderConfig[] = [
-  {
-    id: 'browseros',
-    type: 'browseros',
-    name: 'BrowserOS',
-    modelId: 'browseros-auto',
-    supportsImages: true,
-    contextWindow: 200000,
-    temperature: 0.2,
-    createdAt: timestamp,
-    updatedAt: timestamp,
-  },
-  {
-    id: 'anthropic-sonnet',
-    type: 'anthropic',
-    name: 'Anthropic Sonnet',
-    modelId: 'claude-sonnet-4-6',
-    apiKey: 'sk-ant',
-    supportsImages: true,
-    contextWindow: 200000,
-    temperature: 0.2,
-    createdAt: timestamp,
-    updatedAt: timestamp,
-  },
-]
-
-describe('provider resolution when the server is unreachable', () => {
-  // The list being unreachable says nothing about whether the chosen provider
-  // exists, so running anyway would spend the wrong credentials on the wrong
-  // model. The job runner turns this into a failed run the user can see.
-  it('fails a scheduled job that named a provider', async () => {
-    storageValues.set('unreachable', true)
-    const { getChatServerResponse } = await import('./getChatServerResponse')
-
-    await expect(
-      getChatServerResponse({
-        message: 'Run my schedule',
-        providerId: 'anthropic-sonnet',
-      }),
-    ).rejects.toThrow('Cannot reach the BrowserOS server')
-
-    expect(fetchBodies).toHaveLength(0)
-  })
-
-  it('fails a refine that named a provider', async () => {
-    storageValues.set('unreachable', true)
-    const { refinePrompt } = await import('./refine-prompt')
-
-    await expect(
-      refinePrompt({
-        prompt: 'Check mail',
-        name: 'Morning brief',
-        providerId: 'anthropic-sonnet',
-      }),
-    ).rejects.toThrow('Cannot reach the BrowserOS server')
-  })
-
-  // A job that named nothing still has a choice behind it: the configured
-  // default. Its id is in extension storage but its model and credentials are
-  // in the list that failed to load, so the built-in is not a safe stand-in.
-  it('fails a scheduled job that relies on the configured default', async () => {
-    storageValues.set('unreachable', true)
-    const { getChatServerResponse } = await import('./getChatServerResponse')
-
-    await expect(
-      getChatServerResponse({ message: 'Run my schedule' }),
-    ).rejects.toThrow('Cannot reach the BrowserOS server')
-
-    expect(fetchBodies).toHaveLength(0)
-  })
-
-  // An empty list is a different answer from an unreachable one: the server
-  // replied and really has no providers, so the built-in is correct.
-  it('still falls back to the built-in provider when the server has none', async () => {
-    storageValues.set('providers', [])
-    const { getChatServerResponse } = await import('./getChatServerResponse')
-
-    await getChatServerResponse({ message: 'Run my schedule' })
-
-    expect(fetchBodies[0]).toMatchObject({ provider: 'browseros' })
-  })
-
-  // A provider that was genuinely deleted still falls back, as before. Only
-  // the unreachable case is treated as unsafe.
-  it('falls back when the named provider no longer exists', async () => {
-    const { getChatServerResponse } = await import('./getChatServerResponse')
-
-    await getChatServerResponse({
-      message: 'Run my schedule',
-      providerId: 'deleted-provider',
-    })
-
-    expect(fetchBodies[0]).toMatchObject({ provider: 'anthropic' })
-  })
-})

packages/browseros-agent/apps/app/modules/chat/chat-session-request.test.ts

@@ -21,9 +21,11 @@ describe('chat request preparation', () => {
     expect(request.api).toBe('http://127.0.0.1:5151/chat')
     expect(request.body).toMatchObject({
       target: { type: 'browseros', providerId: 'browseros' },
-      provider: 'browseros',
       message: 'Summarize this page',
     })
+    // The provider is named, not described: its configuration is resolved
+    // from the id on the server.
+    expect('provider' in request.body).toBe(false)
   })
 
   it('sends ACP agents to the same endpoint without provider fields', () => {