From f0f8517699102716e72c722a0a917a4b5a738e5c Mon Sep 17 00:00:00 2001 From: Yufei Date: Thu, 27 Aug 2026 12:48:16 -0700 Subject: [PATCH] Keep the user's provider order across the installed-only provider split MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What was wrong listSystemProviderInfosForHost concatenated the always-listed providers before the installed-only ones, so the registry's user-ordered listing never survived across the visibility boundary. A provider dragged past that boundary in Settings → Providers persisted its providerOrder write but snapped back on refetch. ## What changed - listSystemProviderInfosForHost now filters the registry's listing by which half survived (capability filter, installed-only health probe) instead of concatenating the halves, so picker order holds across the boundary. - The test harness registry reads providerOrder/defaultProviderId from the test database the same way the real server does, and a regression test pins an installed-only provider above always-listed ones. ## How you verified - New test fails before the fix, passes after. - pnpm exec turbo run typecheck lint --filter=@bb/server - apps/server suite: 2050 passed. - Live dev stack: dragged Claude Code below acp-omp in Settings → Providers; the order survived a page reload. --- .../src/services/system/execution-options.ts | 20 +++++++++- apps/server/test/helpers/test-app.ts | 14 ++++++- .../test/system/provider-routing.test.ts | 37 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/apps/server/src/services/system/execution-options.ts b/apps/server/src/services/system/execution-options.ts index 8fcda4ca20..871e5a0dc1 100644 --- a/apps/server/src/services/system/execution-options.ts +++ b/apps/server/src/services/system/execution-options.ts @@ -219,9 +219,25 @@ async function listSystemProviderInfosForHost( hostId: string, capability?: ProviderCapabilityFilter, ): Promise { - return listConfiguredSystemProviderInfos(deps, capability).concat( - await listInstalledPluginProviderInfos(deps, hostId, capability), + const configured = listConfiguredSystemProviderInfos(deps, capability); + const installed = await listInstalledPluginProviderInfos( + deps, + hostId, + capability, ); + // Both halves follow the registry's user order, but concatenating them + // would pin every always-listed provider above every installed-only one, + // so a drag that moves a provider across that boundary never sticks. + // Keep the registry's order instead: list whichever half survived the + // capability filter and the installed-only health probe. + const listedIds = new Set([ + ...configured.map((provider) => provider.id), + ...installed.map((provider) => provider.id), + ]); + return deps.providerRegistry + .list() + .filter((registration) => listedIds.has(registration.info.id)) + .map((registration) => registration.info); } function resolveSystemProviderInfosPlan( diff --git a/apps/server/test/helpers/test-app.ts b/apps/server/test/helpers/test-app.ts index 504e451757..bb14fdc909 100644 --- a/apps/server/test/helpers/test-app.ts +++ b/apps/server/test/helpers/test-app.ts @@ -3,7 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { serve } from "@hono/node-server"; import type { AddressInfo } from "node:net"; -import { createConnection, type DbConnection } from "@bb/db"; +import { createConnection, getAppSettings, type DbConnection } from "@bb/db"; import { defaultFeatureFlags, type HostType } from "@bb/domain"; import { initDb } from "../../src/db.js"; import { createApp } from "../../src/server.js"; @@ -166,7 +166,17 @@ export async function createTestAppHarness( const watchInterests = new WatchInterestCoordinator({ db, hub }); const sharedPorts = new HostSharedPortCoordinator({ db, hub }); const workspaceReadCaches = new WorkspaceReadCaches({ hub }); - const providerRegistry = createProviderRegistryService({}); + // Same preference wiring as the real server: picker order and the default + // provider are user settings read per registry call. + const providerRegistry = createProviderRegistryService({ + readUserProviderPreferences: () => { + const settings = getAppSettings(db); + return { + providerOrder: settings.providerOrder, + defaultProviderId: settings.defaultProviderId, + }; + }, + }); const pluginHostArtifacts = new PluginHostArtifactRegistry(); const providerNativeRoots = createProviderNativeRootsCache( nativeRootsClock === undefined ? {} : { now: nativeRootsClock }, diff --git a/apps/server/test/system/provider-routing.test.ts b/apps/server/test/system/provider-routing.test.ts index a7fd5534d8..fde168cafb 100644 --- a/apps/server/test/system/provider-routing.test.ts +++ b/apps/server/test/system/provider-routing.test.ts @@ -1,4 +1,5 @@ import { updateHost } from "@bb/db"; +import { defaultAppSettings } from "@bb/domain"; import { describe, expect, it } from "vitest"; import { z } from "zod"; import type { HostDaemonOnlineRpcRequestMessage } from "@bb/host-daemon-contract"; @@ -265,4 +266,40 @@ describe("GET /api/v1/system/providers", () => { } }); }); + + it("keeps the user's providerOrder across the installed-only visibility split", async () => { + await withTestHarness({}, async (harness) => { + const primary = seedHostSession(harness.deps, { + id: "host-provider-order-primary", + }); + seedPrimaryHost(harness.deps, primary.host.id); + // Only acp-opencode reports installed; every other installed-only + // agent (acp-omp, acp-grok) stays hidden from the listing. + registerHostRpcResponder(harness, { + hostId: primary.host.id, + sessionId: primary.session.id, + handle: (request) => + providerHostResponse(request, "acp-opencode", "model"), + }); + + const put = await harness.app.request("/api/v1/settings/general", { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + ...defaultAppSettings, + providerOrder: ["acp-opencode", "claude-code", "codex"], + defaultProviderId: null, + }), + }); + expect(put.status).toBe(200); + + const ids = await providerIds( + await harness.app.request("/api/v1/system/providers"), + ); + // The installed-only provider the user pinned first must lead the + // listing; the always-listed providers follow in pinned order, not + // ahead of it. + expect(ids.slice(0, 3)).toEqual(["acp-opencode", "claude-code", "codex"]); + }); + }); });