diff --git a/src/agentMode/ui/ModelEnableList.tsx b/src/agentMode/ui/ModelEnableList.tsx index 84fe9d5e..25227c7b 100644 --- a/src/agentMode/ui/ModelEnableList.tsx +++ b/src/agentMode/ui/ModelEnableList.tsx @@ -27,10 +27,16 @@ export interface ModelEnableGroup { /** Group heading — a provider display name (no glyphs/avatars). */ label: string; /** - * Origin badge (e.g. "BYOK", "Copilot Plus", "Agent Provided"). Set only when - * the list spans multiple origins, so it actually disambiguates. + * Origin badge (e.g. "BYOK", "Agent Provided"). Set only when the list spans + * multiple origins, so it actually disambiguates. Copilot Plus carries no + * badge — its label already reads "Copilot Plus". */ badge?: string; + /** + * Visually emphasize the group header (accent color). Set for Copilot Plus, + * which the caller also floats to the top of the list. + */ + highlight?: boolean; rows: ModelEnableRow[]; } @@ -123,7 +129,14 @@ export const ModelEnableList: React.FC = ({ isOpen(group.key) && "tw-rotate-90" )} /> - {group.label} + + {group.label} + {group.badge && ( {group.badge} diff --git a/src/main.ts b/src/main.ts index a483d8e5..6cab9caa 100644 --- a/src/main.ts +++ b/src/main.ts @@ -45,7 +45,11 @@ import { ChatManager } from "@/core/ChatManager"; import { MessageRepository } from "@/core/MessageRepository"; import { logError, logInfo, logWarn } from "@/logger"; import { logFileManager } from "@/logFileManager"; -import { createModelManagement, type ModelManagementApi } from "@/modelManagement"; +import { + createModelManagement, + syncCopilotPlusProvider, + type ModelManagementApi, +} from "@/modelManagement"; import { KeychainService } from "@/services/keychainService"; import { persistSettings, @@ -148,6 +152,22 @@ export default class CopilotPlugin extends Plugin { this.modelManagement = createModelManagement({ app: this.app, }); + // Register/unregister the Copilot Plus provider (and its models) to match + // Plus state, so Plus models surface in the chat + opencode pickers. The + // license key (raw/encrypted) is decrypted inside the sync for the relay's + // Bearer token. Idempotent, so the redundant initial call below + per-change + // calls are safe. Serialized through `plusSyncChain` so a fast + // sign-out→sign-in (each its own settings change) settles in issue order, + // not in whichever overlapping reconcile happens to finish last. + let plusSyncChain: Promise = Promise.resolve(); + const syncPlus = (isPlusUser: boolean | undefined, licenseKey: string): void => { + plusSyncChain = plusSyncChain.then(() => + syncCopilotPlusProvider(this.modelManagement, !!isPlusUser, licenseKey) + ); + }; + // Initial reconcile: an already-signed-in user's `isPlusUser` is restored + // from disk without firing the subscription, so register on load. + syncPlus(getSettings().isPlusUser, getSettings().plusLicenseKey); this.settingsUnsubscriber = subscribeToSettingsChange((prev, next) => { void (async () => { try { @@ -166,6 +186,13 @@ export default class CopilotPlugin extends Plugin { if (prev && prev.agentMode?.enabled !== next.agentMode?.enabled) { this.refreshRibbonIcon(); } + // Sign-in / sign-out (isPlusUser flip) or key rotation while signed in. + if ( + prev?.isPlusUser !== next.isPlusUser || + (next.isPlusUser && prev?.plusLicenseKey !== next.plusLicenseKey) + ) { + syncPlus(next.isPlusUser, next.plusLicenseKey); + } })(); }); this.addSettingTab(new CopilotSettingTab(this.app, this)); diff --git a/src/modelManagement/index.ts b/src/modelManagement/index.ts index 5ddc2d2c..4b4a7ff7 100644 --- a/src/modelManagement/index.ts +++ b/src/modelManagement/index.ts @@ -73,6 +73,7 @@ export type { export { CopilotPlusSetupApi } from "./setup/CopilotPlusSetupApi"; export type { PlusSetupResult, RegisterPlusProviderInput } from "./setup/CopilotPlusSetupApi"; +export { COPILOT_PLUS_MODELS, syncCopilotPlusProvider } from "./setup/copilotPlusSync"; // --------------------------------------------------------------------------- // Top-level factory + coordinator diff --git a/src/modelManagement/setup/CopilotPlusSetupApi.test.ts b/src/modelManagement/setup/CopilotPlusSetupApi.test.ts new file mode 100644 index 00000000..466cee7d --- /dev/null +++ b/src/modelManagement/setup/CopilotPlusSetupApi.test.ts @@ -0,0 +1,352 @@ +/** + * Tests for `CopilotPlusSetupApi` — the singleton Copilot Plus + * provider/model enrollment + sign-out cascade. + * + * The injected deps are mocked as plain in-memory fakes (no real settings + * store), mirroring `AgentSetupApi.test.ts`. They hold just enough state to + * exercise create / idempotent-update / diff-reconcile / auto-enroll / + * embedding-skip / sign-out-cascade. + */ + +import { CopilotPlusSetupApi } from "./CopilotPlusSetupApi"; + +import type { ModelManagementCoordinator } from "@/modelManagement/createModelManagement"; +import type { ModelInfo } from "@/modelManagement/types/catalog"; +import type { BackendType, ConfiguredModel, Provider } from "@/modelManagement/types/persisted"; +import type { BackendConfigRegistry } from "@/modelManagement/backends/BackendConfigRegistry"; +import type { ConfiguredModelRegistry } from "@/modelManagement/models/ConfiguredModelRegistry"; +import type { ProviderRegistry } from "@/modelManagement/providers/ProviderRegistry"; + +jest.mock("@/logger", () => ({ + logInfo: jest.fn(), + logWarn: jest.fn(), + logError: jest.fn(), +})); + +// --------------------------------------------------------------------------- +// In-memory fakes +// --------------------------------------------------------------------------- + +let idCounter = 0; +function nextId(prefix: string): string { + idCounter += 1; + return `${prefix}-${idCounter}`; +} + +class FakeProviderRegistry { + rows = new Map(); + setApiKey = jest.fn(async (providerId: string, apiKey: string) => { + const row = this.rows.get(providerId); + if (row) { + row.apiKeyKeychainId = `keychain-${providerId}`; + void apiKey; + } + }); + + add = jest.fn(async (input: Omit) => { + const providerId = nextId("provider"); + this.rows.set(providerId, { + ...input, + providerId, + addedAt: Date.now(), + apiKeyKeychainId: null, + }); + return providerId; + }); + + update = jest.fn(async (providerId: string, patch: Partial) => { + const existing = this.rows.get(providerId); + if (!existing) throw new Error(`unknown providerId ${providerId}`); + // Mirror the real registry: providerType/origin/keychain id immutable. + const safe = { ...patch }; + delete (safe as Record).providerType; + delete (safe as Record).origin; + delete (safe as Record).apiKeyKeychainId; + this.rows.set(providerId, { ...existing, ...safe }); + }); + + remove = jest.fn(async (providerId: string) => { + this.rows.delete(providerId); + }); + + listByOrigin = jest.fn((kind: Provider["origin"]["kind"]) => + Array.from(this.rows.values()).filter((p) => p.origin.kind === kind) + ); + + get(providerId: string): Provider | undefined { + return this.rows.get(providerId); + } +} + +class FakeConfiguredModelRegistry { + rows: ConfiguredModel[] = []; + + add = jest.fn(async (input: { providerId: string; info: ModelInfo }) => { + if (this.rows.some((m) => m.providerId === input.providerId && m.info.id === input.info.id)) { + throw new Error(`duplicate (${input.providerId}, ${input.info.id})`); + } + const configuredModelId = nextId("model"); + this.rows.push({ ...input, configuredModelId, configuredAt: Date.now() }); + return configuredModelId; + }); + + update = jest.fn(async (configuredModelId: string, patch: { info?: Partial }) => { + const row = this.rows.find((m) => m.configuredModelId === configuredModelId); + if (!row) throw new Error(`unknown ${configuredModelId}`); + if (patch.info) row.info = { ...row.info, ...patch.info }; + }); + + remove = jest.fn(async (configuredModelId: string) => { + this.rows = this.rows.filter((m) => m.configuredModelId !== configuredModelId); + }); + + removeByProvider = jest.fn(async (providerId: string) => { + this.rows = this.rows.filter((m) => m.providerId !== providerId); + }); + + listByProvider = jest.fn((providerId: string) => + this.rows.filter((m) => m.providerId === providerId) + ); + + getByWireId = jest.fn((providerId: string, wireModelId: string) => + this.rows.find((m) => m.providerId === providerId && m.info.id === wireModelId) + ); +} + +class FakeBackendConfigRegistry { + enabled = new Map(); + + enableModel = jest.fn(async (backend: BackendType, configuredModelId: string) => { + const ids = this.enabled.get(backend) ?? []; + if (!ids.includes(configuredModelId)) ids.push(configuredModelId); + this.enabled.set(backend, ids); + }); + + removeRefs = jest.fn(async (configuredModelIds: readonly string[]) => { + const drop = new Set(configuredModelIds); + for (const [backend, ids] of this.enabled) { + this.enabled.set( + backend, + ids.filter((id) => !drop.has(id)) + ); + } + }); + + enabledFor(backend: BackendType): string[] { + return this.enabled.get(backend) ?? []; + } +} + +/** Coordinator fake mirroring the real cascades (refs → rows). */ +class FakeCoordinator { + constructor( + private readonly providers: FakeProviderRegistry, + private readonly backends: FakeBackendConfigRegistry, + private readonly models: FakeConfiguredModelRegistry + ) {} + + removeConfiguredModel = jest.fn(async (configuredModelId: string) => { + await this.backends.removeRefs([configuredModelId]); + await this.models.remove(configuredModelId); + }); + + removeProvider = jest.fn(async (providerId: string) => { + const ids = this.models.listByProvider(providerId).map((m) => m.configuredModelId); + await this.backends.removeRefs(ids); + await this.models.removeByProvider(providerId); + await this.providers.remove(providerId); + }); +} + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +const FLASH: ModelInfo = { + id: "copilot-plus-flash", + displayName: "Copilot Plus Flash", + toolCall: true, +}; +const EMBEDDING: ModelInfo = { + id: "copilot-plus-small", + displayName: "Copilot Plus Small", + isEmbedding: true, +}; + +interface Harness { + api: CopilotPlusSetupApi; + providers: FakeProviderRegistry; + models: FakeConfiguredModelRegistry; + backends: FakeBackendConfigRegistry; + coordinator: FakeCoordinator; +} + +function makeHarness(): Harness { + const providers = new FakeProviderRegistry(); + const models = new FakeConfiguredModelRegistry(); + const backends = new FakeBackendConfigRegistry(); + const coordinator = new FakeCoordinator(providers, backends, models); + const api = new CopilotPlusSetupApi( + providers as unknown as ProviderRegistry, + models as unknown as ConfiguredModelRegistry, + backends as unknown as BackendConfigRegistry, + coordinator as unknown as ModelManagementCoordinator + ); + return { api, providers, models, backends, coordinator }; +} + +function register(h: Harness, models: ModelInfo[], apiKey: string | undefined = "lic-key") { + return h.api.registerPlusProvider({ + providerType: "openai-compatible", + displayName: "Copilot Plus", + baseUrl: "https://models.brevilabs.com/v1", + apiKey, + models, + }); +} + +beforeEach(() => { + idCounter = 0; +}); + +// --------------------------------------------------------------------------- +// registerPlusProvider +// --------------------------------------------------------------------------- + +describe("CopilotPlusSetupApi.registerPlusProvider", () => { + it("creates one copilot-plus provider, stores the key, and auto-enrolls the chat model into chat + opencode", async () => { + const h = makeHarness(); + const result = await register(h, [FLASH]); + + expect(h.providers.rows.size).toBe(1); + const provider = h.providers.get(result.providerId)!; + expect(provider.origin).toEqual({ kind: "copilot-plus" }); + expect(provider.providerType).toBe("openai-compatible"); + expect(provider.displayName).toBe("Copilot Plus"); + + expect(h.providers.setApiKey).toHaveBeenCalledWith(result.providerId, "lic-key"); + + expect(result.configuredModelIds).toHaveLength(1); + const flashId = result.configuredModelIds[0]; + expect(h.backends.enabledFor("chat")).toEqual([flashId]); + expect(h.backends.enabledFor("opencode")).toEqual([flashId]); + expect(h.backends.enabledFor("claude")).toEqual([]); + expect(h.backends.enabledFor("codex")).toEqual([]); + }); + + it("creates an embedding model but never enrolls it into a completion backend", async () => { + const h = makeHarness(); + const result = await register(h, [FLASH, EMBEDDING]); + + expect(result.configuredModelIds).toHaveLength(2); + const flashId = h.models.getByWireId( + result.providerId, + "copilot-plus-flash" + )!.configuredModelId; + const embeddingId = h.models.getByWireId( + result.providerId, + "copilot-plus-small" + )!.configuredModelId; + + expect(h.backends.enabledFor("chat")).toEqual([flashId]); + expect(h.backends.enabledFor("opencode")).toEqual([flashId]); + expect(h.backends.enabledFor("chat")).not.toContain(embeddingId); + }); + + it("does not call setApiKey when no key is supplied", async () => { + const h = makeHarness(); + const result = await h.api.registerPlusProvider({ + providerType: "openai-compatible", + displayName: "Copilot Plus", + baseUrl: "https://models.brevilabs.com/v1", + models: [FLASH], + }); + expect(h.providers.setApiKey).not.toHaveBeenCalled(); + expect(h.providers.get(result.providerId)!.apiKeyKeychainId).toBeNull(); + }); + + it("is idempotent: re-running updates in place, rotates the key, no duplicate provider/model", async () => { + const h = makeHarness(); + const first = await register(h, [FLASH]); + + h.models.add.mockClear(); + h.backends.enableModel.mockClear(); + h.coordinator.removeConfiguredModel.mockClear(); + + const second = await h.api.registerPlusProvider({ + providerType: "openai-compatible", + displayName: "Copilot Plus (renamed)", + baseUrl: "https://models.brevilabs.com/v1", + apiKey: "rotated-key", + models: [FLASH], + }); + + expect(second.providerId).toBe(first.providerId); + expect(h.providers.rows.size).toBe(1); + expect(h.providers.add).toHaveBeenCalledTimes(1); + expect(h.providers.update).toHaveBeenCalledTimes(1); + expect(h.providers.get(first.providerId)!.displayName).toBe("Copilot Plus (renamed)"); + expect(h.providers.setApiKey).toHaveBeenLastCalledWith(first.providerId, "rotated-key"); + + expect(second.configuredModelIds).toEqual(first.configuredModelIds); + // Unchanged model list → pure no-op reconcile. + expect(h.models.add).not.toHaveBeenCalled(); + expect(h.backends.enableModel).not.toHaveBeenCalled(); + expect(h.coordinator.removeConfiguredModel).not.toHaveBeenCalled(); + }); + + it("cascade-removes a model Plus no longer offers", async () => { + const h = makeHarness(); + const first = await register(h, [FLASH, EMBEDDING]); + const embeddingId = h.models.getByWireId( + first.providerId, + "copilot-plus-small" + )!.configuredModelId; + + await register(h, [FLASH]); // embedding vanished + + expect(h.coordinator.removeConfiguredModel).toHaveBeenCalledWith(embeddingId); + expect(h.models.getByWireId(first.providerId, "copilot-plus-small")).toBeUndefined(); + // Surviving chat model still enrolled. + const flashId = h.models.getByWireId(first.providerId, "copilot-plus-flash")!.configuredModelId; + expect(h.backends.enabledFor("opencode")).toContain(flashId); + }); + + it("refreshes a model's display string in place on re-register, preserving its id", async () => { + const h = makeHarness(); + const first = await register(h, [FLASH]); + const idBefore = first.configuredModelIds[0]; + + await register(h, [{ ...FLASH, displayName: "Copilot Plus Flash 2" }]); + + const row = h.models.getByWireId(first.providerId, "copilot-plus-flash")!; + expect(row.configuredModelId).toBe(idBefore); + expect(row.info.displayName).toBe("Copilot Plus Flash 2"); + }); +}); + +// --------------------------------------------------------------------------- +// unregisterPlusProvider +// --------------------------------------------------------------------------- + +describe("CopilotPlusSetupApi.unregisterPlusProvider", () => { + it("cascade-removes the provider, its models, and backend refs", async () => { + const h = makeHarness(); + const reg = await register(h, [FLASH]); + const flashId = reg.configuredModelIds[0]; + + await h.api.unregisterPlusProvider(); + + expect(h.coordinator.removeProvider).toHaveBeenCalledWith(reg.providerId); + expect(h.providers.rows.size).toBe(0); + expect(h.models.rows).toHaveLength(0); + expect(h.backends.enabledFor("chat")).not.toContain(flashId); + expect(h.backends.enabledFor("opencode")).not.toContain(flashId); + }); + + it("is a no-op when no Plus provider exists", async () => { + const h = makeHarness(); + await h.api.unregisterPlusProvider(); + expect(h.coordinator.removeProvider).not.toHaveBeenCalled(); + }); +}); diff --git a/src/modelManagement/setup/CopilotPlusSetupApi.ts b/src/modelManagement/setup/CopilotPlusSetupApi.ts index 8c7d77d6..4870c309 100644 --- a/src/modelManagement/setup/CopilotPlusSetupApi.ts +++ b/src/modelManagement/setup/CopilotPlusSetupApi.ts @@ -18,11 +18,12 @@ import type { ModelManagementCoordinator } from "@/modelManagement/createModelManagement"; import type { ProviderType } from "@/modelManagement/types/catalog"; -import type { BackendType } from "@/modelManagement/types/persisted"; +import type { BackendType, Provider } from "@/modelManagement/types/persisted"; import type { ModelInfo } from "@/modelManagement/types/catalog"; import type { BackendConfigRegistry } from "@/modelManagement/backends/BackendConfigRegistry"; import type { ConfiguredModelRegistry } from "@/modelManagement/models/ConfiguredModelRegistry"; import type { ProviderRegistry } from "@/modelManagement/providers/ProviderRegistry"; +import { BYOK_DEFAULT_AUTO_ENROLL } from "@/modelManagement/setup/ByokSetupApi"; export interface RegisterPlusProviderInput { /** Which adapter family backs the Plus relay (Plus may add more @@ -47,6 +48,11 @@ export interface PlusSetupResult { } export class CopilotPlusSetupApi { + readonly #providers: ProviderRegistry; + readonly #models: ConfiguredModelRegistry; + readonly #backends: BackendConfigRegistry; + readonly #coordinator: ModelManagementCoordinator; + constructor( providerRegistry: ProviderRegistry, configuredModelRegistry: ConfiguredModelRegistry, @@ -56,7 +62,12 @@ export class CopilotPlusSetupApi { * `coordinator.removeConfiguredModel` / * `coordinator.removeProvider`. */ coordinator: ModelManagementCoordinator - ) {} + ) { + this.#providers = providerRegistry; + this.#models = configuredModelRegistry; + this.#backends = backendConfigRegistry; + this.#coordinator = coordinator; + } /** * Idempotent. First call creates a `Provider` row with @@ -69,11 +80,45 @@ export class CopilotPlusSetupApi { * `autoEnrollIn ?? BYOK_DEFAULT_AUTO_ENROLL`. Existing models that * are no longer in `input.models` are removed (cascading their * backend refs through the coordinator). + * + * Writes happen in order — provider row, then key (only if non-null), + * then models — so a failed later step leaves earlier writes for + * `coordinator.removeProvider` (sign-out) to clean up. */ - registerPlusProvider(input: RegisterPlusProviderInput): Promise { - throw new Error( - "[modelManagement] CopilotPlusSetupApi.registerPlusProvider not implemented yet" + async registerPlusProvider(input: RegisterPlusProviderInput): Promise { + const existing = this.#findPlusProvider(); + + let providerId: string; + if (existing) { + providerId = existing.providerId; + // Reuse the row so re-syncing never spawns a second Plus provider; + // providerType/origin/keychain id are immutable through `update`. + await this.#providers.update(providerId, { + displayName: input.displayName, + baseUrl: input.baseUrl, + }); + } else { + providerId = await this.#providers.add({ + providerType: input.providerType, + displayName: input.displayName, + baseUrl: input.baseUrl, + origin: { kind: "copilot-plus" }, + }); + } + + // The license key IS the relay token; store it so the chat factory and + // `buildOpencodeConfig` can read it back via `getApiKey` at call time. + if (input.apiKey != null) { + await this.#providers.setApiKey(providerId, input.apiKey); + } + + const configuredModelIds = await this.#reconcileModels( + providerId, + input.models, + input.autoEnrollIn ?? BYOK_DEFAULT_AUTO_ENROLL ); + + return { providerId, configuredModelIds }; } /** @@ -81,9 +126,77 @@ export class CopilotPlusSetupApi { * dropping its ConfiguredModels and clearing backend refs. No-op * when no Plus provider exists. */ - unregisterPlusProvider(): Promise { - throw new Error( - "[modelManagement] CopilotPlusSetupApi.unregisterPlusProvider not implemented yet" - ); + async unregisterPlusProvider(): Promise { + const existing = this.#findPlusProvider(); + if (!existing) return; + await this.#coordinator.removeProvider(existing.providerId); + } + + /** + * The single Plus provider (origin filter). Throws if more than one exists — + * Plus is a singleton origin and a silent pick would corrupt state. + */ + #findPlusProvider(): Provider | undefined { + const matches = this.#providers.listByOrigin("copilot-plus"); + if (matches.length > 1) { + throw new Error( + `[modelManagement] CopilotPlusSetupApi: ${matches.length} copilot-plus providers ` + + `found; the singleton invariant is violated` + ); + } + return matches[0]; + } + + /** + * Diff-reconcile the Plus provider's ConfiguredModel set against `models`: + * add new wire ids (auto-enrolling each non-embedding model), refresh drifted + * display strings in place (no configuredModelId churn), and cascade-remove + * vanished ones. Returns the resulting ids in input order. Mirrors + * `AgentSetupApi.#reconcileModels`; only real deltas write, so re-syncing an + * unchanged list never resets user curation. + */ + async #reconcileModels( + providerId: string, + models: readonly ModelInfo[], + autoEnrollIn: readonly BackendType[] + ): Promise { + const existing = this.#models.listByProvider(providerId); + const existingByWireId = new Map(existing.map((m) => [m.info.id, m])); + const desiredWireIds = new Set(models.map((info) => info.id)); + + for (const info of models) { + const current = existingByWireId.get(info.id); + if (!current) { + const configuredModelId = await this.#models.add({ providerId, info }); + // Embedding models aren't chat models — enrolling them into chat/agent + // backends would surface them in completion pickers where they fail. + if (!info.isEmbedding) { + for (const backend of autoEnrollIn) { + await this.#backends.enableModel(backend, configuredModelId); + } + } + continue; + } + if ( + current.info.displayName !== info.displayName || + current.info.description !== info.description + ) { + await this.#models.update(current.configuredModelId, { + info: { displayName: info.displayName, description: info.description }, + }); + } + } + + for (const model of existing) { + if (desiredWireIds.has(model.info.id)) continue; + await this.#coordinator.removeConfiguredModel(model.configuredModelId); + } + + const ids: string[] = []; + for (const info of models) { + const found = this.#models.getByWireId(providerId, info.id); + if (found) ids.push(found.configuredModelId); + } + return ids; } } diff --git a/src/modelManagement/setup/copilotPlusSync.test.ts b/src/modelManagement/setup/copilotPlusSync.test.ts new file mode 100644 index 00000000..35a4d70f --- /dev/null +++ b/src/modelManagement/setup/copilotPlusSync.test.ts @@ -0,0 +1,99 @@ +/** + * Tests for `syncCopilotPlusProvider` — the sign-in/sign-out bridge that + * reconciles the singleton Copilot Plus provider. + * + * The register/unregister decision must key on Plus sign-in state (`isPlusUser` + * + a raw stored key), NOT on whether that key decrypts. A decrypt failure + * (safeStorage unavailable, vault synced to another machine) returns "" from + * `getDecryptedKey`; treating that as sign-out would tear down the persisted + * provider + user curation. These tests pin that down. + */ + +import { syncCopilotPlusProvider } from "./copilotPlusSync"; + +import type { ModelManagementApi } from "@/modelManagement/createModelManagement"; +import type { RegisterPlusProviderInput } from "@/modelManagement/setup/CopilotPlusSetupApi"; + +jest.mock("@/logger", () => ({ + logInfo: jest.fn(), + logWarn: jest.fn(), + logError: jest.fn(), +})); + +const getDecryptedKey = jest.fn, [string]>(); +jest.mock("@/encryptionService", () => ({ + getDecryptedKey: (k: string) => getDecryptedKey(k), +})); + +function makeApi() { + const registerPlusProvider = jest.fn(async (_input: RegisterPlusProviderInput) => ({ + providerId: "plus-1", + configuredModelIds: [] as string[], + })); + const unregisterPlusProvider = jest.fn(async () => {}); + const api = { + setup: { copilotPlus: { registerPlusProvider, unregisterPlusProvider } }, + } as unknown as ModelManagementApi; + return { api, registerPlusProvider, unregisterPlusProvider }; +} + +beforeEach(() => { + getDecryptedKey.mockReset(); +}); + +describe("syncCopilotPlusProvider", () => { + it("registers with the decrypted token when signed in with a decryptable key", async () => { + getDecryptedKey.mockResolvedValue("decrypted-token"); + const { api, registerPlusProvider, unregisterPlusProvider } = makeApi(); + + await syncCopilotPlusProvider(api, true, "enc_desk_raw"); + + expect(getDecryptedKey).toHaveBeenCalledWith("enc_desk_raw"); + expect(unregisterPlusProvider).not.toHaveBeenCalled(); + expect(registerPlusProvider).toHaveBeenCalledTimes(1); + expect(registerPlusProvider.mock.calls[0][0]).toMatchObject({ apiKey: "decrypted-token" }); + }); + + it("still registers (never tears down) when the stored key fails to decrypt, leaving the token untouched", async () => { + // Decrypt failure: getDecryptedKey returns "" but the raw key is present + // and the user is still a Plus user. + getDecryptedKey.mockResolvedValue(""); + const { api, registerPlusProvider, unregisterPlusProvider } = makeApi(); + + await syncCopilotPlusProvider(api, true, "enc_desk_raw"); + + expect(unregisterPlusProvider).not.toHaveBeenCalled(); + expect(registerPlusProvider).toHaveBeenCalledTimes(1); + // `undefined` makes registerPlusProvider leave the existing keychain token + // in place rather than overwriting it with "". + expect(registerPlusProvider.mock.calls[0][0].apiKey).toBeUndefined(); + }); + + it("unregisters when not a Plus user", async () => { + const { api, registerPlusProvider, unregisterPlusProvider } = makeApi(); + + await syncCopilotPlusProvider(api, false, "enc_desk_raw"); + + expect(registerPlusProvider).not.toHaveBeenCalled(); + expect(unregisterPlusProvider).toHaveBeenCalledTimes(1); + // No need to decrypt on the teardown path. + expect(getDecryptedKey).not.toHaveBeenCalled(); + }); + + it("unregisters when signed in but no key is stored", async () => { + const { api, registerPlusProvider, unregisterPlusProvider } = makeApi(); + + await syncCopilotPlusProvider(api, true, ""); + + expect(registerPlusProvider).not.toHaveBeenCalled(); + expect(unregisterPlusProvider).toHaveBeenCalledTimes(1); + }); + + it("swallows errors (best-effort background reconcile)", async () => { + getDecryptedKey.mockResolvedValue("decrypted-token"); + const { api, registerPlusProvider } = makeApi(); + registerPlusProvider.mockRejectedValueOnce(new Error("boom")); + + await expect(syncCopilotPlusProvider(api, true, "enc_desk_raw")).resolves.toBeUndefined(); + }); +}); diff --git a/src/modelManagement/setup/copilotPlusSync.ts b/src/modelManagement/setup/copilotPlusSync.ts new file mode 100644 index 00000000..be120111 --- /dev/null +++ b/src/modelManagement/setup/copilotPlusSync.ts @@ -0,0 +1,71 @@ +/** + * Reconciles the Copilot Plus provider with the user's current Plus state. + * + * Plus has no model-list endpoint, so the model set is a hardcoded snapshot + * (`COPILOT_PLUS_MODELS`). `syncCopilotPlusProvider` is the single bridge the + * plugin host calls on Plus sign-in / sign-out (and once on load): it + * registers the Plus provider when signed in (with a key) and unregisters it + * otherwise. Both `register`/`unregister` are idempotent, so calling this on + * every relevant settings change is safe. + */ + +import { BREVILABS_MODELS_BASE_URL, ChatModels } from "@/constants"; +import { getDecryptedKey } from "@/encryptionService"; +import { logError } from "@/logger"; +import type { ModelManagementApi } from "@/modelManagement/createModelManagement"; +import type { ModelInfo } from "@/modelManagement/types/catalog"; + +/** + * The Copilot Plus models the brevilabs relay exposes. Hardcoded — Plus offers + * a single curated chat model today and there's no relay catalog to fetch. Wire + * ids must match what the relay accepts; opencode routes them as + * `copilot-plus/` (see `mapProviderToOpencodeId`). + */ +export const COPILOT_PLUS_MODELS: readonly ModelInfo[] = Object.freeze([ + { + id: ChatModels.COPILOT_PLUS_FLASH, + displayName: "Copilot Plus Flash", + toolCall: true, + modalities: { input: ["text", "image"], output: ["text"] }, + }, +]); + +/** + * Register or unregister the Plus provider to match Plus state. Best-effort: + * a failure is logged, not thrown, since this runs as background reconciliation + * off a settings change. + * + * `licenseKey` is the RAW stored key (still encrypted on disk) — the same value + * the rest of the plugin gates on (`brevilabsClient`, `plusUtils`). The + * register/unregister decision keys on sign-in state (`isPlusUser` + a stored + * key), NOT on whether that key happens to decrypt: a decrypt failure (Electron + * `safeStorage` unavailable, a vault synced to another machine) must not tear + * down the persisted provider + the user's curation. Decryption is only for the + * relay Bearer token, and a failed decrypt leaves the previously-stored token + * untouched rather than overwriting it with "". + */ +export async function syncCopilotPlusProvider( + api: ModelManagementApi, + isPlusUser: boolean, + licenseKey: string | undefined +): Promise { + try { + if (isPlusUser && licenseKey) { + const token = await getDecryptedKey(licenseKey); + await api.setup.copilotPlus.registerPlusProvider({ + providerType: "openai-compatible", + displayName: "Copilot Plus", + baseUrl: BREVILABS_MODELS_BASE_URL, + // Only refresh the stored relay token when decryption succeeded; "" + // would clobber a previously-good keychain entry, and `undefined` makes + // `registerPlusProvider` leave the existing token in place. + apiKey: token || undefined, + models: COPILOT_PLUS_MODELS, + }); + } else { + await api.setup.copilotPlus.unregisterPlusProvider(); + } + } catch (err) { + logError("[modelManagement] Copilot Plus provider sync failed", err); + } +} diff --git a/src/settings/v2/components/configuredModelGrouping.test.ts b/src/settings/v2/components/configuredModelGrouping.test.ts index 41b87443..2363c677 100644 --- a/src/settings/v2/components/configuredModelGrouping.test.ts +++ b/src/settings/v2/components/configuredModelGrouping.test.ts @@ -318,7 +318,7 @@ describe("buildModelEnableGroups", () => { expect(groups[0].label).toBe("Codex"); }); - it("badges each group with its origin when the list mixes origins (opencode)", () => { + it("badges non-Plus origins when the list mixes origins (opencode)", () => { const plusProvider: Provider = { providerId: "plus-1", providerType: "anthropic", @@ -349,10 +349,47 @@ describe("buildModelEnableGroups", () => { }; const groups = buildModelEnableGroups(partition, true, ""); expect(groups.find((g) => g.key === "byok:byok-1")?.badge).toBe("BYOK"); - expect(groups.find((g) => g.key === "byok:plus-1")?.badge).toBe("Copilot Plus"); expect(groups.find((g) => g.label === "opencode")?.badge).toBe("Agent Provided"); }); + it("floats Copilot Plus to the top, highlights it, and gives it no redundant badge", () => { + const plusProvider: Provider = { + providerId: "plus-1", + providerType: "anthropic", + displayName: "Copilot Plus", + origin: { kind: "copilot-plus" }, + addedAt: 0, + }; + const partition = { + byokPlusCandidates: [ + { + configuredModel: model("m-byok", "byok-1", "claude-sonnet-4-5"), + provider: byok, + enabled: true, + }, + { + configuredModel: model("m-plus", "plus-1", "gpt-5"), + provider: plusProvider, + enabled: false, + }, + ], + agentOriginCandidates: [ + { + configuredModel: model("m-oc", "oc-agent", "opencode/big-pickle"), + provider: ocAgent, + enabled: false, + }, + ], + }; + const groups = buildModelEnableGroups(partition, true, ""); + // Copilot Plus is first regardless of candidate order. + expect(groups[0].key).toBe("byok:plus-1"); + expect(groups[0].highlight).toBe(true); + expect(groups[0].badge).toBeUndefined(); + // Non-Plus groups are not highlighted. + expect(groups.find((g) => g.key === "byok:byok-1")?.highlight).toBeUndefined(); + }); + it("omits badges when the list has a single origin (claude/codex)", () => { const codexAgent = agentProvider("codex-agent", "codex", "Codex"); const partition = { diff --git a/src/settings/v2/components/configuredModelGrouping.ts b/src/settings/v2/components/configuredModelGrouping.ts index 7e5581f2..8b288902 100644 --- a/src/settings/v2/components/configuredModelGrouping.ts +++ b/src/settings/v2/components/configuredModelGrouping.ts @@ -179,10 +179,18 @@ export function buildModelEnableGroups( out.push({ group: { key: `agent:${label}`, label, rows }, kind: "agent" }); } - // Badge each group only when the list actually mixes origins. + // Copilot Plus is highlighted and floated to the top; its provider name + // already reads "Copilot Plus", so it carries no disambiguating badge. Every + // other origin gets a badge only when the list actually mixes origins. const mixed = new Set(out.map((o) => o.kind)).size > 1; - if (mixed) { - for (const o of out) o.group.badge = originBadgeLabel(o.kind); + for (const o of out) { + if (o.kind === "copilot-plus") { + o.group.highlight = true; + } else if (mixed) { + o.group.badge = originBadgeLabel(o.kind); + } } + // Stable sort (V8 sort is stable): Copilot Plus first, others keep their order. + out.sort((a, b) => Number(b.kind === "copilot-plus") - Number(a.kind === "copilot-plus")); return out.map((o) => o.group); }