obsidian-plugin/logancyang_obsidian-copilot
1
0
Fork 0
mirror of https://github.com/logancyang/obsidian-copilot.git synced 2026-07-22 07:50:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(agent-mode): surface missing provider credentials (#2529)

Browse source
This commit is contained in:
Zero Liu 2026-05-29 23:39:18 -07:00 committed by GitHub
parent 9ff5ba05de
commit 38f6e42bcc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
37 changed files with 1341 additions and 259 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CLAUDE.md
View file

@ -1,3 +0,0 @@
# CLAUDE.md
@AGENTS.md

1
CLAUDE.md Symbolic link
View file

@ -0,0 +1 @@
AGENTS.md

9
src/agentMode/backends/claude/descriptor.ts
View file

@ -13,12 +13,13 @@ import {
import type { AgentSession } from "@/agentMode/session/AgentSession";
import { MethodUnsupportedError } from "@/agentMode/session/errors";
import { resolveClaudeBinary } from "./claudeBinaryResolver";
import { agentOriginEnabledWireIds } from "@/agentMode/backends/shared/agentEnabledModels";
import { agentOriginEnabledModelEntries } from "@/agentMode/backends/shared/agentEnabledModels";
import { ClaudeSdkBackendProcess } from "@/agentMode/sdk/ClaudeSdkBackendProcess";
import { getCachedSdkCatalog, synthesizeEffortConfigOption } from "@/agentMode/sdk/effortOption";
import { buildAgentSystemPrompt } from "@/agentMode/backends/shared/agentSystemPrompt";
import type {
BackendConfigOption,
EnabledModelEntry,
ModeMapping,
ModelSelection,
ModelWireCodec,
@ -132,9 +133,11 @@ export const ClaudeBackendDescriptor: BackendDescriptor = {
wire: claudeWire,
showModelDescriptions: true,
getEnabledBaseModelIds(settings: CopilotSettings): ReadonlySet<string> {
getEnabledModelEntries(settings: CopilotSettings): EnabledModelEntry[] {
// All Claude Code models are agent-origin.
return agentOriginEnabledWireIds(settings, "claude", (wireId) => claudeWire.decode(wireId));
return [
...agentOriginEnabledModelEntries(settings, "claude", (wireId) => claudeWire.decode(wireId)),
];
},
getInstallState(settings: CopilotSettings): InstallState {

17
src/agentMode/backends/codex/descriptor.ts
View file

@ -10,12 +10,17 @@ import { CodexInstallModal } from "./CodexInstallModal";
import CodexLogo from "./logo.svg";
import { CodexSettingsPanel } from "./CodexSettingsPanel";
import type { AgentSession } from "@/agentMode/session/AgentSession";
import { agentOriginEnabledWireIds } from "@/agentMode/backends/shared/agentEnabledModels";
import { agentOriginEnabledModelEntries } from "@/agentMode/backends/shared/agentEnabledModels";
import {
binaryPathInstallState,
simpleBinaryBackendProcess,
} from "@/agentMode/backends/shared/simpleBinaryBackend";
import type { ModeMapping, ModelSelection, ModelWireCodec } from "@/agentMode/session/types";
import type {
EnabledModelEntry,
ModeMapping,
ModelSelection,
ModelWireCodec,
} from "@/agentMode/session/types";
import type { BackendDescriptor, BackendProcess, InstallState } from "@/agentMode/session/types";
export const CODEX_BINARY_NAME = "codex-acp";
@ -61,7 +66,7 @@ const codexWire: ModelWireCodec = {
* from the local `codex` CLI login. Auth is CLI-owned (no Copilot-side keys),
* so the candidate models come entirely from the CLI's live `availableModels`
* (active session or preloader cache); curation is the model-management
* `backends.codex.enabledModels` set surfaced via `getEnabledBaseModelIds`.
* `backends.codex.enabledModels` set surfaced via `getEnabledModelEntries`.
*
* Effort is surfaced via opencode-style model-id parsing — codex-acp
* advertises one model per (base × effort) combination, and we collapse
@ -79,9 +84,11 @@ export const CodexBackendDescriptor: BackendDescriptor = {
wire: codexWire,
showModelDescriptions: true,
getEnabledBaseModelIds(settings: CopilotSettings): ReadonlySet<string> {
getEnabledModelEntries(settings: CopilotSettings): EnabledModelEntry[] {
// All Codex models are agent-origin.
return agentOriginEnabledWireIds(settings, "codex", (wireId) => codexWire.decode(wireId));
return [
...agentOriginEnabledModelEntries(settings, "codex", (wireId) => codexWire.decode(wireId)),
];
},
/**

13
src/agentMode/backends/opencode/descriptor.ts
View file

@ -12,12 +12,17 @@ import {
OPENCODE_PROVIDER_MAP,
} from "./OpencodeBackend";
import { computeInstallState, OpencodeBinaryManager } from "./OpencodeBinaryManager";
import { opencodeEnabledWireIds } from "./opencodeModelResolve";
import { opencodeEnabledModelEntries } from "./opencodeModelResolve";
import { OpencodeSettingsPanel } from "./OpencodeSettingsPanel";
import { mapNodeArch, mapNodePlatform } from "./platformResolver";
import type { AgentSession } from "@/agentMode/session/AgentSession";
import { simpleBinaryBackendProcess } from "@/agentMode/backends/shared/simpleBinaryBackend";
import type { ModeMapping, ModelSelection, ModelWireCodec } from "@/agentMode/session/types";
import type {
EnabledModelEntry,
ModeMapping,
ModelSelection,
ModelWireCodec,
} from "@/agentMode/session/types";
import type { BackendDescriptor, BackendProcess, InstallState } from "@/agentMode/session/types";
/** Config option id OpenCode uses to switch the active agent at runtime. */
@ -98,8 +103,8 @@ export const OpencodeBackendDescriptor: BackendDescriptor = {
restartOnSystemPromptChange: true,
wire: opencodeWire,
getEnabledBaseModelIds(settings: CopilotSettings): ReadonlySet<string> {
return opencodeEnabledWireIds(settings);
getEnabledModelEntries(settings: CopilotSettings): EnabledModelEntry[] {
return [...opencodeEnabledModelEntries(settings)];
},
getInstallState(settings: CopilotSettings): InstallState {

108
src/agentMode/backends/opencode/opencodeModelResolve.test.ts
View file

@ -1,6 +1,6 @@
import type { CopilotSettings } from "@/settings/model";
import type { ConfiguredModel, Provider, ProviderOrigin, ProviderType } from "@/modelManagement";
import { mapProviderToOpencodeId, opencodeEnabledWireIds } from "./opencodeModelResolve";
import { mapProviderToOpencodeId, opencodeEnabledModelEntries } from "./opencodeModelResolve";
/** Build a minimal `Provider` row for a given origin + type. */
function makeProvider(
@ -29,7 +29,7 @@ function makeModel(configuredModelId: string, providerId: string, wireId: string
/**
* Assemble a `CopilotSettings`-shaped object with only the slices
* `opencodeEnabledWireIds` reads. Cast through `unknown` since the resolver
* `opencodeEnabledModelEntries` reads. Cast through `unknown` since the resolver
* touches just `backends` / `configuredModels` / `providers`.
*/
function makeSettings(args: {
@ -85,48 +85,71 @@ describe("mapProviderToOpencodeId", () => {
});
});
describe("opencodeEnabledWireIds", () => {
it("returns the shared frozen empty set when no models are enabled", () => {
const first = opencodeEnabledWireIds(makeSettings({ enabledModels: [] }));
const second = opencodeEnabledWireIds(makeSettings({ enabledModels: [] }));
expect(first.size).toBe(0);
// Referential stability: the same frozen constant on every empty call.
expect(first).toBe(second);
describe("opencodeEnabledModelEntries", () => {
const byokProvider = (overrides: Partial<Provider> = {}): Provider => ({
...makeProvider("p1", { kind: "byok", catalogProviderId: "openrouter" }, "openai-compatible"),
requiresApiKey: true,
apiKeyKeychainId: "kc-1",
...overrides,
});
it("returns the shared frozen empty set when the opencode backend is absent", () => {
const result = opencodeEnabledWireIds(makeSettings({}));
expect(result.size).toBe(0);
});
it("builds `<provider>/<model>` wire ids for BYOK models", () => {
it("flags a required-key provider with no key as missing_key", () => {
const settings = makeSettings({
enabledModels: ["cm1"],
providers: { p1: makeProvider("p1", { kind: "byok", catalogProviderId: "anthropic" }) },
configuredModels: [makeModel("cm1", "p1", "claude-sonnet-4-6")],
providers: { p1: byokProvider({ apiKeyKeychainId: null }) },
configuredModels: [makeModel("cm1", "p1", "qwen/qwen3-max")],
});
const result = opencodeEnabledWireIds(settings);
expect([...result]).toEqual(["anthropic/claude-sonnet-4-6"]);
const [entry] = opencodeEnabledModelEntries(settings);
expect(entry.baseModelId).toBe("openrouter/qwen/qwen3-max");
expect(entry.credentialState).toBe("missing_key");
});
it("builds `<provider>/<model>` wire ids for copilot-plus models", () => {
it("reports a keyed, never-failed provider as ok with its display name", () => {
const settings = makeSettings({
enabledModels: ["cm1"],
providers: { p1: makeProvider("p1", { kind: "copilot-plus" }) },
configuredModels: [makeModel("cm1", "p1", "copilot-plus-flash")],
providers: { p1: byokProvider() },
configuredModels: [
{
configuredModelId: "cm1",
providerId: "p1",
info: { id: "x", displayName: "Big X" },
configuredAt: 0,
},
],
});
const result = opencodeEnabledWireIds(settings);
expect([...result]).toEqual(["copilot-plus/copilot-plus-flash"]);
const [entry] = opencodeEnabledModelEntries(settings);
expect(entry.credentialState).toBe("ok");
expect(entry.name).toBe("Big X");
});
it("uses the verbatim info.id for agent-origin models (already full wire form)", () => {
it("treats agent-origin (native) models as ok regardless of key", () => {
const settings = makeSettings({
enabledModels: ["cm1"],
providers: { p1: makeProvider("p1", { kind: "agent", agentType: "opencode" }) },
configuredModels: [makeModel("cm1", "p1", "opencode/big-pickle")],
});
const result = opencodeEnabledWireIds(settings);
expect([...result]).toEqual(["opencode/big-pickle"]);
const [entry] = opencodeEnabledModelEntries(settings);
expect(entry.baseModelId).toBe("opencode/big-pickle");
expect(entry.credentialState).toBe("ok");
});
it("returns the shared frozen empty array when nothing is enabled", () => {
const first = opencodeEnabledModelEntries(makeSettings({ enabledModels: [] }));
const second = opencodeEnabledModelEntries(makeSettings({ enabledModels: [] }));
expect(first).toHaveLength(0);
// Referential stability: the same frozen constant on every empty call.
expect(first).toBe(second);
});
it("builds the `<provider>/<model>` wire base id for copilot-plus models", () => {
const settings = makeSettings({
enabledModels: ["cm1"],
providers: { p1: makeProvider("p1", { kind: "copilot-plus" }) },
configuredModels: [makeModel("cm1", "p1", "copilot-plus-flash")],
});
expect(opencodeEnabledModelEntries(settings)[0].baseModelId).toBe(
"copilot-plus/copilot-plus-flash"
);
});
it("skips models whose provider row is missing", () => {
@ -135,7 +158,7 @@ describe("opencodeEnabledWireIds", () => {
providers: {},
configuredModels: [makeModel("cm1", "p1", "claude-sonnet-4-6")],
});
expect(opencodeEnabledWireIds(settings).size).toBe(0);
expect(opencodeEnabledModelEntries(settings)).toHaveLength(0);
});
it("skips models whose configured-model row is missing", () => {
@ -144,7 +167,7 @@ describe("opencodeEnabledWireIds", () => {
providers: { p1: makeProvider("p1", { kind: "byok", catalogProviderId: "anthropic" }) },
configuredModels: [],
});
expect(opencodeEnabledWireIds(settings).size).toBe(0);
expect(opencodeEnabledModelEntries(settings)).toHaveLength(0);
});
it("skips models on unroutable providers (BYOK without catalog id)", () => {
@ -153,31 +176,6 @@ describe("opencodeEnabledWireIds", () => {
providers: { p1: makeProvider("p1", { kind: "byok" }, "azure") },
configuredModels: [makeModel("cm1", "p1", "some-azure-model")],
});
expect(opencodeEnabledWireIds(settings).size).toBe(0);
});
it("builds `<providerId>/<model>` wire ids for OpenAI-compatible BYOK models", () => {
const settings = makeSettings({
enabledModels: ["cm1"],
providers: { p1: makeProvider("p1", { kind: "byok" }, "openai-compatible") },
configuredModels: [makeModel("cm1", "p1", "llama3.2")],
});
expect([...opencodeEnabledWireIds(settings)]).toEqual(["p1/llama3.2"]);
});
it("mixes BYOK and agent-origin models with the correct wire shapes", () => {
const settings = makeSettings({
enabledModels: ["cm-byok", "cm-agent"],
providers: {
byok: makeProvider("byok", { kind: "byok", catalogProviderId: "openai" }),
agent: makeProvider("agent", { kind: "agent", agentType: "opencode" }),
},
configuredModels: [
makeModel("cm-byok", "byok", "gpt-5"),
makeModel("cm-agent", "agent", "opencode/big-pickle"),
],
});
const result = opencodeEnabledWireIds(settings);
expect([...result].sort()).toEqual(["openai/gpt-5", "opencode/big-pickle"].sort());
expect(opencodeEnabledModelEntries(settings)).toHaveLength(0);
});
});

68
src/agentMode/backends/opencode/opencodeModelResolve.ts
View file

@ -1,5 +1,7 @@
import type { CopilotSettings } from "@/settings/model";
import type { ConfiguredModel, Provider } from "@/modelManagement";
import { providerRequiresApiKey } from "@/modelManagement";
import type { EnabledModelCredentialState, EnabledModelEntry } from "@/agentMode/session/types";
export interface OpencodeProviderMapping {
/** The opencode provider id — leading segment of `<provider>/<model>`. */
@ -16,7 +18,7 @@ export interface OpencodeProviderMapping {
export const COPILOT_PLUS_OPENCODE_PROVIDER_ID = "copilot-plus";
/** See AGENTS.md → "Referential stability". */
const EMPTY_WIRE_IDS: ReadonlySet<string> = Object.freeze(new Set<string>());
const EMPTY_ENABLED_ENTRIES: readonly EnabledModelEntry[] = Object.freeze([]);
/**
* Map a Copilot `Provider` onto its opencode provider id, or `null` when
@ -52,25 +54,48 @@ export function mapProviderToOpencodeId(provider: Provider): OpencodeProviderMap
}
/**
* The opencode wire ids for the backend's enabled models, joining
* `backends.opencode.enabledModels` to the configured-model + provider state.
* BYOK / Plus models become `${opencodeProviderId}/${info.id}`; agent-origin
* models use `info.id` verbatim (already the full wire form). Unroutable or
* missing entries are skipped.
*
* Shared by `buildOpencodeConfig` (injection) and the descriptor's picker
* filter so the injected / enabled / shown sets agree.
* The opencode wire base id for one routable configured model
* (`<providerId>/<model>` for non-native, `info.id` verbatim for agent-hosted
* native). Returns `null` when the provider isn't opencode-routable.
*/
export function opencodeEnabledWireIds(settings: CopilotSettings): ReadonlySet<string> {
function opencodeWireBaseId(provider: Provider, configuredModel: ConfiguredModel): string | null {
const mapping = mapProviderToOpencodeId(provider);
if (!mapping) return null;
return mapping.native ? configuredModel.info.id : `${mapping.id}/${configuredModel.info.id}`;
}
/**
* Credential health for an enabled opencode model, derived purely from the
* persisted provider row (sync — no keychain read). Native (agent-hosted)
* providers carry their own auth, so they're always `ok`. Otherwise a
* required-key provider with no key reads `missing_key`.
*/
function credentialStateFor(provider: Provider, native: boolean): EnabledModelCredentialState {
if (native) return "ok";
if (providerRequiresApiKey(provider) && !provider.apiKeyKeychainId) return "missing_key";
return "ok";
}
/**
* Enabled opencode models enriched for the chat picker: wire base id, display
* name/description, and per-model credential health. Lets the picker iterate
* the enabled set (not the reported∩enabled intersection) so a model opencode
* dropped for a missing/expired key still appears, flagged. Joins
* `backends.opencode.enabledModels` to the configured-model + provider state
* via `opencodeWireBaseId`; unroutable / missing entries are skipped.
*/
export function opencodeEnabledModelEntries(
settings: CopilotSettings
): readonly EnabledModelEntry[] {
const enabledIds = settings.backends.opencode?.enabledModels ?? [];
if (enabledIds.length === 0) return EMPTY_WIRE_IDS;
if (enabledIds.length === 0) return EMPTY_ENABLED_ENTRIES;
const modelsById = new Map<string, ConfiguredModel>();
for (const model of settings.configuredModels) {
modelsById.set(model.configuredModelId, model);
}
const wireIds = new Set<string>();
const out: EnabledModelEntry[] = [];
for (const configuredModelId of enabledIds) {
const configuredModel = modelsById.get(configuredModelId);
if (!configuredModel) continue;
@ -78,15 +103,14 @@ export function opencodeEnabledWireIds(settings: CopilotSettings): ReadonlySet<s
if (!provider) continue;
const mapping = mapProviderToOpencodeId(provider);
if (!mapping) continue;
if (mapping.native) {
// Agent-origin: `info.id` is already the full opencode wire form.
wireIds.add(configuredModel.info.id);
} else {
wireIds.add(`${mapping.id}/${configuredModel.info.id}`);
}
const baseModelId = opencodeWireBaseId(provider, configuredModel);
if (!baseModelId) continue;
out.push({
baseModelId,
name: configuredModel.info.displayName || configuredModel.info.id,
description: configuredModel.info.description,
credentialState: credentialStateFor(provider, mapping.native),
});
}
if (wireIds.size === 0) return EMPTY_WIRE_IDS;
return wireIds;
return out.length === 0 ? EMPTY_ENABLED_ENTRIES : out;
}

36
src/agentMode/backends/shared/agentEnabledModels.test.ts
View file

@ -1,4 +1,4 @@
import { agentOriginEnabledWireIds } from "./agentEnabledModels";
import { agentOriginEnabledModelEntries } from "./agentEnabledModels";
import type { CopilotSettings } from "@/settings/model";
import type { ConfiguredModel } from "@/modelManagement";
@ -37,35 +37,41 @@ function settingsWith(
} as unknown as CopilotSettings;
}
describe("agentOriginEnabledWireIds", () => {
it("returns the shared frozen empty set when nothing is enabled", () => {
const a = agentOriginEnabledWireIds(settingsWith("claude", [], []), "claude", bareDecode);
const b = agentOriginEnabledWireIds(settingsWith("codex", [], []), "codex", suffixDecode);
expect(a.size).toBe(0);
describe("agentOriginEnabledModelEntries", () => {
it("returns the shared frozen empty array when nothing is enabled", () => {
const a = agentOriginEnabledModelEntries(settingsWith("claude", [], []), "claude", bareDecode);
const b = agentOriginEnabledModelEntries(settingsWith("codex", [], []), "codex", suffixDecode);
expect(a).toHaveLength(0);
// Frozen empty constant — same reference across calls (referential stability).
expect(a).toBe(b);
});
it("claude: maps enabled configured-model ids to their bare info.id baseModelId", () => {
it("claude: maps enabled configured-model ids to their bare info.id baseModelId, all ok", () => {
const settings = settingsWith(
"claude",
["cm1", "cm2"],
[model("cm1", "claude-sonnet-4-5"), model("cm2", "claude-opus-4-1")]
);
const set = agentOriginEnabledWireIds(settings, "claude", bareDecode);
expect([...set].sort()).toEqual(["claude-opus-4-1", "claude-sonnet-4-5"]);
const entries = agentOriginEnabledModelEntries(settings, "claude", bareDecode);
expect(entries.map((e) => e.baseModelId).sort()).toEqual([
"claude-opus-4-1",
"claude-sonnet-4-5",
]);
// Agent-native: CLI-owned auth, never a credential flag.
expect(entries.every((e) => e.credentialState === "ok")).toBe(true);
expect(entries[0].name).toBe("claude-sonnet-4-5");
});
it("codex: strips the effort suffix to the base model id", () => {
const settings = settingsWith("codex", ["cm1"], [model("cm1", "gpt-5/high")]);
const set = agentOriginEnabledWireIds(settings, "codex", suffixDecode);
expect([...set]).toEqual(["gpt-5"]);
const entries = agentOriginEnabledModelEntries(settings, "codex", suffixDecode);
expect(entries.map((e) => e.baseModelId)).toEqual(["gpt-5"]);
});
it("skips enabled ids with no matching configured-model row", () => {
const settings = settingsWith("claude", ["cm1", "ghost"], [model("cm1", "claude-sonnet-4-5")]);
const set = agentOriginEnabledWireIds(settings, "claude", bareDecode);
expect([...set]).toEqual(["claude-sonnet-4-5"]);
const entries = agentOriginEnabledModelEntries(settings, "claude", bareDecode);
expect(entries.map((e) => e.baseModelId)).toEqual(["claude-sonnet-4-5"]);
});
it("only reads the requested agentType's enabledModels", () => {
@ -76,7 +82,7 @@ describe("agentOriginEnabledWireIds", () => {
},
configuredModels: [model("cm1", "claude-sonnet-4-5"), model("cm2", "gpt-5")],
} as unknown as CopilotSettings;
const claude = agentOriginEnabledWireIds(settings, "claude", bareDecode);
expect([...claude]).toEqual(["claude-sonnet-4-5"]);
const claude = agentOriginEnabledModelEntries(settings, "claude", bareDecode);
expect(claude.map((e) => e.baseModelId)).toEqual(["claude-sonnet-4-5"]);
});
});

35
src/agentMode/backends/shared/agentEnabledModels.ts
View file

@ -1,43 +1,48 @@
import type { CopilotSettings } from "@/settings/model";
import type { ConfiguredModel } from "@/modelManagement";
import type { EnabledModelEntry } from "@/agentMode/session/types";
/** See AGENTS.md → "Referential stability". */
const EMPTY_WIRE_IDS: ReadonlySet<string> = Object.freeze(new Set<string>());
const EMPTY_ENABLED_ENTRIES: readonly EnabledModelEntry[] = Object.freeze([]);
/** A descriptor's own `wire.decode`, accepted as a parameter so this backend-layer helper needn't import the session-domain codec type. */
export type WireDecode = (wireId: string) => { selection: { baseModelId: string } };
/**
* The wire baseModelIds for a claude / codex backend's enabled models —
* each enabled `ConfiguredModel.info.id` decoded via the descriptor's
* `wire.decode`. Used by the descriptor's `getEnabledBaseModelIds` so the
* enabled and picker-shown sets agree.
* The enabled-model entries for a claude / codex backend — each enabled
* `ConfiguredModel.info.id` decoded via the descriptor's `wire.decode` to the
* baseModelId the picker compares against `ModelEntry.baseModelId`, enriched
* with the model's display name/description. These backends are all
* agent-origin (auth is CLI-owned, no Copilot-side keys), so every entry is
* `credentialState: "ok"`.
*
* Only for all-agent-origin backends; opencode mixes in BYOK/Plus models and
* uses `opencodeEnabledWireIds` instead.
* uses `opencodeEnabledModelEntries` instead.
*/
export function agentOriginEnabledWireIds(
export function agentOriginEnabledModelEntries(
settings: CopilotSettings,
agentType: "claude" | "codex",
wireDecode: WireDecode
): ReadonlySet<string> {
): readonly EnabledModelEntry[] {
const enabledIds = settings.backends[agentType]?.enabledModels ?? [];
if (enabledIds.length === 0) return EMPTY_WIRE_IDS;
if (enabledIds.length === 0) return EMPTY_ENABLED_ENTRIES;
const modelsById = new Map<string, ConfiguredModel>();
for (const model of settings.configuredModels) {
modelsById.set(model.configuredModelId, model);
}
const wireIds = new Set<string>();
const entries: EnabledModelEntry[] = [];
for (const configuredModelId of enabledIds) {
const configuredModel = modelsById.get(configuredModelId);
if (!configuredModel) continue;
// `info.id` is the agent-reported wire id; decode it to the baseModelId
// the picker compares against `ModelEntry.baseModelId`.
wireIds.add(wireDecode(configuredModel.info.id).selection.baseModelId);
entries.push({
baseModelId: wireDecode(configuredModel.info.id).selection.baseModelId,
name: configuredModel.info.displayName || configuredModel.info.id,
description: configuredModel.info.description,
credentialState: "ok",
});
}
if (wireIds.size === 0) return EMPTY_WIRE_IDS;
return wireIds;
return entries.length === 0 ? EMPTY_ENABLED_ENTRIES : entries;
}

6
src/agentMode/index.ts
View file

@ -151,6 +151,12 @@ export function createAgentSessionManager(app: App, plugin: CopilotPlugin): Agen
// new spawn picks them up. Without this, a key entered after the
// subprocess started never reaches it — opencode keeps making un-
// authenticated requests and surfaces them as silent zero-token turns.
//
// A single BYOK save fires several emits in quick succession (provider row →
// API key → enabled models). Coalescing them into one re-probe lives a layer
// down: a running backend folds rapid restarts via the manager's restart
// queue, and a warm preload probe folds them via `preloader.refresh` — both
// re-read the *final* config once the burst settles, so no debounce here.
const restartProviderAffected = (reason: string): void => {
for (const descriptor of listBackendDescriptors()) {
if (!descriptor.restartOnProviderConfigChange) continue;

44
src/agentMode/session/AgentModelPreloader.test.ts
View file

@ -196,6 +196,50 @@ describe("AgentModelPreloader.takeWarm", () => {
expect(preloader.takeWarm("claude-sdk")).toBeNull();
});
it("refresh re-probes a warm backend against current settings", async () => {
const { descriptor } = buildDescriptor(() => makeMockProc());
const create = descriptor.createBackendProcess as jest.Mock;
const preloader = new AgentModelPreloader(buildApp(), buildPlugin(), () => descriptor);
await preloader.preload("claude-sdk");
expect(create).toHaveBeenCalledTimes(1);
// A config change after the warm probe settled: refresh drops the warm
// entry and runs a fresh probe.
await preloader.refresh("claude-sdk");
expect(create).toHaveBeenCalledTimes(2);
});
it("refresh returns null when nothing is warm or in flight", async () => {
const { descriptor } = buildDescriptor(() => makeMockProc());
const preloader = new AgentModelPreloader(buildApp(), buildPlugin(), () => descriptor);
// Never preloaded → a config change must not spin a probe up from nothing.
expect(preloader.refresh("claude-sdk")).toBeNull();
});
it("coalesces a burst of refreshes into a single trailing re-probe", async () => {
// Models a BYOK save: several config writes call refresh in one synchronous
// burst. The 2nd/3rd land while the 1st's probe is still in flight (runProbe
// awaits proc.start before reading the catalog), so they fold into exactly
// one trailing re-probe against the settled settings — not one per write.
const { descriptor } = buildDescriptor(() => makeMockProc());
const create = descriptor.createBackendProcess as jest.Mock;
const preloader = new AgentModelPreloader(buildApp(), buildPlugin(), () => descriptor);
await preloader.preload("claude-sdk"); // probe #1 → something warm
expect(create).toHaveBeenCalledTimes(1);
const chain = preloader.refresh("claude-sdk"); // drops warm, starts probe #2
expect(chain).not.toBeNull();
void preloader.refresh("claude-sdk"); // in-flight → trailing-rerun flag
void preloader.refresh("claude-sdk"); // in-flight → already flagged
await chain;
// probe #2 (in-flight) + exactly one trailing probe #3 = 3 total.
expect(create).toHaveBeenCalledTimes(3);
});
it("clearCached shuts down and drops a still-warm proc", async () => {
const { descriptor, procHandle } = buildDescriptor(() => makeMockProc());
const preloader = new AgentModelPreloader(buildApp(), buildPlugin(), () => descriptor);

60
src/agentMode/session/AgentModelPreloader.ts
View file

@ -51,6 +51,10 @@ export class AgentModelPreloader {
// The active session's `attachModelCacheSync` writes here.
private readonly cache = new Map<BackendId, BackendState>();
private readonly inflight = new Map<BackendId, Promise<void>>();
// Backends whose in-flight probe baked stale spawn config and must re-probe
// once it settles. Set by `refresh`, drained by the probe chain. Coalesces
// the burst of config writes one BYOK save produces into a single re-probe.
private readonly pendingRefresh = new Set<BackendId>();
private readonly listeners = new Set<() => void>();
// Per-warm-entry exit-listener teardowns. Wired when the warm entry is
// recorded so we can clear it if the probe subprocess dies before the
@ -127,13 +131,66 @@ export class AgentModelPreloader {
if (this.disposed) return Promise.resolve();
const existing = this.inflight.get(backendId);
if (existing) return existing;
const promise = this.runProbe(backendId).finally(() => {
return this.startProbeChain(backendId);
}
/**
* Re-probe `backendId` against current settings after a spawn-config change
* (a new API key, an enabled-models edit, …). Unlike {@link preload} — whose
* dedupe is right for "ensure a warm proc exists" — a config change may land
* *after* an in-flight probe baked its spawn config, so that probe would
* cache a stale catalog and the picker would flag the freshly-enabled model
* "not offered by agent" until a reload.
*
* A single BYOK save lands several writes (provider row → key → enabled
* models) in a burst, each calling here. The first drops the warm entry and
* starts a fresh probe; the rest just flag a trailing re-run, so exactly one
* more probe runs once the in-flight one finishes — observing the settled
* settings. This coalesces the burst into a single final re-probe without a
* debounce timer.
*
* Returns the probe-chain promise (for preload-status wiring), or `null` when
* nothing is warm or in flight — a config change for a never-probed backend
* must not spin one up.
*/
refresh(backendId: BackendId): Promise<void> | null {
if (this.disposed) return null;
const existing = this.inflight.get(backendId);
if (existing) {
this.pendingRefresh.add(backendId);
return existing;
}
if (this.getCachedBackendState(backendId) === null) return null;
this.clearCached(backendId);
return this.startProbeChain(backendId);
}
/**
* Track a probe as one in-flight promise so concurrent callers dedupe against
* the whole chain, including any trailing re-runs requested via
* {@link refresh}.
*/
private startProbeChain(backendId: BackendId): Promise<void> {
const promise = this.runProbeChain(backendId).finally(() => {
this.inflight.delete(backendId);
this.pendingRefresh.delete(backendId);
});
this.inflight.set(backendId, promise);
return promise;
}
private async runProbeChain(backendId: BackendId): Promise<void> {
let round = 0;
do {
this.pendingRefresh.delete(backendId);
// Later rounds replace a warm entry the prior probe set; drop it first so
// the abandoned subprocess is shut down rather than leaked.
if (round > 0) this.clearCached(backendId);
round += 1;
await this.runProbe(backendId);
} while (this.pendingRefresh.has(backendId) && !this.disposed);
}
subscribe(listener: () => void): () => void {
this.listeners.add(listener);
return () => this.listeners.delete(listener);
@ -143,6 +200,7 @@ export class AgentModelPreloader {
this.disposed = true;
this.cache.clear();
this.inflight.clear();
this.pendingRefresh.clear();
this.listeners.clear();
for (const [backendId, warm] of this.warm) {
this.warmExitUnsubs.get(backendId)?.();

119
src/agentMode/session/AgentSessionManager.test.ts
View file

@ -149,6 +149,7 @@ function buildManager(): AgentSessionManager {
const modelPreloader = {
getCachedBackendState: jest.fn(() => null),
preload: jest.fn(async () => undefined),
refresh: jest.fn(() => null),
subscribe: jest.fn(() => () => {}),
shutdown: jest.fn(),
setCached: jest.fn(),
@ -219,6 +220,7 @@ describe("AgentSessionManager.createSession", () => {
const modelPreloader = {
getCachedBackendState: jest.fn((id: string) => cache.get(id) ?? null),
preload: jest.fn(async () => undefined),
refresh: jest.fn(() => null),
subscribe: jest.fn(() => () => {}),
shutdown: jest.fn(),
setCached: jest.fn((id: string, state: unknown) => {
@ -523,6 +525,123 @@ describe("AgentSessionManager.restartBackend", () => {
expect(mockBackendShutdown).not.toHaveBeenCalled();
});
it("refreshes a warm preload probe when the manager owns no process yet", async () => {
// Repro for the BYOK key-add bug: opencode was only preloaded (warm proc
// held by the preloader, never adopted into a session), so the provider
// restart no-oped and the stale probe's catalog made the picker flag the
// freshly-keyed model "not offered by agent" until a full reload.
const refresh = jest.fn(() => Promise.resolve());
const modelPreloader = {
getCachedBackendState: jest.fn(() => ({ model: null, mode: null })),
preload: jest.fn(async () => undefined),
refresh,
subscribe: jest.fn(() => () => {}),
shutdown: jest.fn(),
setCached: jest.fn(),
clearCached: jest.fn(),
takeWarm: jest.fn(() => null),
};
const descriptor = {
...buildDescriptor(),
getInstallState: jest.fn(() => ({ kind: "ready" })),
} as unknown as BackendDescriptor;
const mgr = new AgentSessionManager(
buildApp(),
buildPlugin() as unknown as ConstructorParameters<typeof AgentSessionManager>[1],
{
permissionPrompter: jest.fn(),
resolveDescriptor: (id) => (id === descriptor.id ? descriptor : undefined),
modelPreloader: modelPreloader as unknown as ConstructorParameters<
typeof AgentSessionManager
>[2]["modelPreloader"],
}
);
// The preloader owns the clear+re-probe (and its coalescing); the manager
// just delegates to `refresh` and registers the returned probe.
await expect(mgr.restartBackend("opencode", "byok key added")).resolves.toBe(true);
expect(refresh).toHaveBeenCalledWith("opencode");
// No session was ever created, so no proc to shut down.
expect(mockBackendShutdown).not.toHaveBeenCalled();
});
it("does not refresh a warm probe when nothing was preloaded", async () => {
// Backend is installed, but the preloader has nothing warm/in-flight, so
// `refresh` returns null and the manager must not spin a probe up.
const refresh = jest.fn(() => null);
const modelPreloader = {
getCachedBackendState: jest.fn(() => null),
preload: jest.fn(async () => undefined),
refresh,
subscribe: jest.fn(() => () => {}),
shutdown: jest.fn(),
setCached: jest.fn(),
clearCached: jest.fn(),
takeWarm: jest.fn(() => null),
};
const descriptor = {
...buildDescriptor(),
getInstallState: jest.fn(() => ({ kind: "ready" })),
} as unknown as BackendDescriptor;
const mgr = new AgentSessionManager(
buildApp(),
buildPlugin() as unknown as ConstructorParameters<typeof AgentSessionManager>[1],
{
permissionPrompter: jest.fn(),
resolveDescriptor: (id) => (id === descriptor.id ? descriptor : undefined),
modelPreloader: modelPreloader as unknown as ConstructorParameters<
typeof AgentSessionManager
>[2]["modelPreloader"],
}
);
await expect(mgr.restartBackend("opencode", "byok key added")).resolves.toBe(false);
expect(refresh).toHaveBeenCalledWith("opencode");
});
it("re-probes after restart when no replacement session is created", async () => {
// Proc exists but no active session is on this backend (e.g. the active
// tab is on a different agent). The torn-down proc leaves nothing to
// repopulate the cache, so the manager must re-probe — otherwise the
// picker flags freshly-enabled models "not offered by agent" until reload.
const preload = jest.fn(async () => undefined);
const modelPreloader = {
getCachedBackendState: jest.fn(() => null),
preload,
refresh: jest.fn(() => null),
subscribe: jest.fn(() => () => {}),
shutdown: jest.fn(),
setCached: jest.fn(),
clearCached: jest.fn(),
takeWarm: jest.fn(() => null),
};
const descriptor = {
...buildDescriptor(),
getInstallState: jest.fn(() => ({ kind: "ready" })),
} as unknown as BackendDescriptor;
const mgr = new AgentSessionManager(
buildApp(),
buildPlugin() as unknown as ConstructorParameters<typeof AgentSessionManager>[1],
{
permissionPrompter: jest.fn(),
resolveDescriptor: (id) => (id === descriptor.id ? descriptor : undefined),
modelPreloader: modelPreloader as unknown as ConstructorParameters<
typeof AgentSessionManager
>[2]["modelPreloader"],
}
);
// Create then close a session so the proc stays up with no active session.
const session = await mgr.createSession();
await mgr.closeSession(session.internalId);
preload.mockClear();
await expect(mgr.restartBackend("opencode", "byok save")).resolves.toBe(true);
expect(mockBackendShutdown).toHaveBeenCalled();
expect(preload).toHaveBeenCalledWith("opencode");
});
it("restarts an idle backend and replaces the active affected session", async () => {
const mgr = buildManager();
const first = await mgr.createSession();

45
src/agentMode/session/AgentSessionManager.ts
View file

@ -401,8 +401,12 @@ export class AgentSessionManager {
* skill discovery and deny rules, is rebuilt from current settings. If a
* session on that backend is busy, the restart is deferred until it is idle.
*
* Returns `true` when a running backend was restarted or a restart was
* scheduled; `false` when no backend process exists yet.
* When the manager owns no process yet, a warm probe held by the preloader
* may still carry the pre-change spawn config; we refresh it instead (see
* `refreshWarmProbe`) so the first chat-open doesn't adopt a stale proc.
*
* Returns `true` when a running backend was restarted, a restart was
* scheduled, or a warm probe was refreshed; `false` when nothing exists yet.
*/
async restartBackend(backendId: BackendId, reason: string): Promise<boolean> {
if (this.disposed) return false;
@ -411,7 +415,7 @@ export class AgentSessionManager {
await inflight.catch(() => undefined);
}
const backend = this.backends.get(backendId);
if (!backend) return false;
if (!backend) return this.refreshWarmProbe(backendId, reason);
if (this.hasBusySession(backendId)) {
const prev = this.pendingBackendRestarts.get(backendId);
this.pendingBackendRestarts.set(backendId, prev ? `${prev}; ${reason}` : reason);
@ -422,6 +426,35 @@ export class AgentSessionManager {
return true;
}
/**
* Refresh a preloader warm probe when the manager owns no process yet.
* Spawn-time config (provider keys, enabled models, native skills, system
* prompt) is baked into the warm probe when it spawns, so a config change
* made before the first chat-open would otherwise never reach it — the
* first `createSession` adopts the stale warm proc and the picker flags
* freshly-enabled models as "not offered by agent" until a full reload.
*
* Delegates the re-probe to `preloader.refresh`, which coalesces the burst of
* writes a single BYOK save produces (provider row → key → enabled models)
* into one re-probe against the settled settings. No-op (returns `false`)
* when nothing is warm/in-flight for this backend, or it isn't installed.
*/
private refreshWarmProbe(backendId: BackendId, reason: string): boolean {
if (this.disposed) return false;
if (!this.isBackendInstalled(backendId)) return false;
const probe = this.preloader.refresh(backendId);
if (!probe) return false;
logInfo(`[AgentMode] refreshing warm ${backendId} probe: ${reason}`);
this.registerPreload(backendId, probe);
return true;
}
/** Whether `backendId`'s descriptor reports its binary/runtime as installed. */
private isBackendInstalled(backendId: BackendId): boolean {
const descriptor = this.opts.resolveDescriptor(backendId);
return descriptor?.getInstallState(getSettings())?.kind === "ready";
}
/** Cached unified backend state for `backendId`, populated by the model preloader. */
getCachedBackendState(backendId: BackendId): BackendState | null {
return this.preloader.getCachedBackendState(backendId);
@ -1161,6 +1194,12 @@ export class AgentSessionManager {
new Notice(`${this.resolveDescriptor(backendId).displayName} refreshed.`);
if (shouldCreateReplacement && !this.disposed) {
await this.createSession(backendId);
} else if (!this.disposed && this.isBackendInstalled(backendId)) {
// No live session will repopulate the cache we just cleared (the
// active tab is on a different backend, or there are no tabs). Re-probe
// so the picker reflects the new spawn config instead of flagging
// freshly-enabled models as "not offered by agent" until a reload.
this.registerPreload(backendId, this.preloader.preload(backendId));
}
this.notify();
} finally {

19
src/agentMode/session/descriptor.ts
View file

@ -7,6 +7,7 @@ import type {
BackendConfigOption,
BackendId,
BackendProcess,
EnabledModelEntry,
ModelSelection,
ModelWireCodec,
ModeMapping,
@ -228,13 +229,19 @@ export interface BackendDescriptor {
getProbeSessionId?(settings: CopilotSettings): string | undefined;
/**
* Optional: the backend's enabled set as wire baseModelIds, which the chat
* picker matches against the reported catalog. The signature is limited to
* `CopilotSettings` so `session/` stays free of `@/modelManagement` — the
* backend implements the join. `null` opts out: the picker then keeps only
* the active session's selection.
* Optional: the backend's enabled models with display metadata and per-model
* credential health — the single accessor the chat picker drives its section
* from. The picker iterates this set (showing every enabled model, flagging
* those the agent can't serve) rather than a reported∩enabled intersection,
* so a model the agent dropped for a missing/expired key — or one it no
* longer reports at all — is never silently hidden. The signature is limited
* to `CopilotSettings` so `session/` stays free of `@/modelManagement` — the
* backend implements the join. Agent-native backends (claude, codex) report
* `credentialState: "ok"` for every entry; key-bearing BYOK backends
* (opencode) compute real per-model health. `null` opts out: the picker then
* keeps only the active session's selection.
*/
getEnabledBaseModelIds?(settings: CopilotSettings): ReadonlySet<string> | null;
getEnabledModelEntries?(settings: CopilotSettings): EnabledModelEntry[] | null;
/**
* Optional: persist the probe sessionId returned by a successful

25
src/agentMode/session/types.ts
View file

@ -102,6 +102,31 @@ export interface ModelEntry {
effortOptions: EffortOption[];
}
/**
* Credential health of an enabled model, derived from its provider row.
* Drives the agent picker's non-selectable flags so an enabled model is
* never silently hidden:
* - `ok` — usable (or agent-hosted / native).
* - `missing_key` — a required-key provider with no key → "Add API key".
*/
export type EnabledModelCredentialState = "ok" | "missing_key";
/**
* One entry in a backend's enabled set, enriched for the picker. Lets the
* picker iterate the *enabled* models (rather than the reported∩enabled
* intersection) so a model the agent dropped for a missing/expired key still
* appears, flagged. `baseModelId` is the backend's wire base id (matched
* against the reported catalog's `ModelEntry.baseModelId`). The signature
* stays limited to `CopilotSettings` on the descriptor so `session/` needn't
* import `@/modelManagement` — the backend computes `credentialState`.
*/
export interface EnabledModelEntry {
baseModelId: string;
name: string;
description?: string;
credentialState: EnabledModelCredentialState;
}
/**
* Normalized model selection — the single shape used by both runtime
* state (`BackendState.model.current`) and persisted preferences

117
src/agentMode/ui/agentModelPickerHelpers.test.ts
View file

@ -1,12 +1,15 @@
import {
appendBackendSection,
buildEffortSibling,
buildModelOnChange,
buildPickerEntries,
resolveActiveDisplayState,
} from "./agentModelPickerHelpers";
import type { ModelSelectorEntry } from "@/components/ui/ModelSelector";
import type {
BackendDescriptor,
BackendState,
EnabledModelEntry,
ModelEntry,
ModelState,
} from "@/agentMode/session/types";
@ -228,14 +231,20 @@ describe("buildPickerEntries", () => {
expect(entries[0].name).toBe("gpt-5");
});
it("filters by getEnabledBaseModelIds when the descriptor implements it (new path)", () => {
it("filters to the enabled set via getEnabledModelEntries", () => {
const enabled = makeModelEntry("anthropic/claude-sonnet-4-6");
const disabled = makeModelEntry("anthropic/claude-haiku");
// Opencode-style descriptor exposing the new enabled-set hook. Only the
// first model is enabled; the second must be dropped from the picker.
// Only the first model is enabled; the second must be dropped from the
// picker even though the agent reports it.
const opencode = {
...makeDescriptor("opencode"),
getEnabledBaseModelIds: () => new Set(["anthropic/claude-sonnet-4-6"]),
getEnabledModelEntries: () => [
{
baseModelId: "anthropic/claude-sonnet-4-6",
name: "Sonnet",
credentialState: "ok" as const,
},
],
} as unknown as BackendDescriptor;
const manager = makeManager({
cachedStateById: {
@ -258,13 +267,15 @@ describe("buildPickerEntries", () => {
expect(entries.map((e) => e.name)).toEqual(["anthropic/claude-sonnet-4-6"]);
});
it("drops every model when the descriptor has no getEnabledBaseModelIds (no legacy branch) except the kept one", () => {
// With the legacy override branch removed, a descriptor that doesn't
// implement getEnabledBaseModelIds yields an empty enabled set, so only
// keepBaseModelId survives. `dropped` is neither enabled nor kept.
it("drops every model not in the enabled set except the kept one", () => {
// An empty enabled set curates nothing in, so only keepBaseModelId
// survives. `dropped` is neither enabled nor kept.
const kept = makeModelEntry("kept-model");
const dropped = makeModelEntry("dropped-model");
const opencode = makeDescriptor("opencode");
const opencode = {
...makeDescriptor("opencode"),
getEnabledModelEntries: () => [],
} as unknown as BackendDescriptor;
const manager = makeManager({
cachedStateById: {
opencode: { model: makeModelState("kept-model", [kept, dropped]), mode: null },
@ -293,7 +304,9 @@ describe("buildPickerEntries", () => {
};
const codex = {
...makeDescriptor("codex"),
getEnabledBaseModelIds: () => new Set(["gpt-5"]),
getEnabledModelEntries: () => [
{ baseModelId: "gpt-5", name: "GPT-5", credentialState: "ok" as const },
],
} as unknown as BackendDescriptor;
const manager = makeManager({
cachedStateById: {
@ -341,7 +354,7 @@ describe("buildPickerEntries", () => {
expect(entries[0]._subtitle).toBe("Opus 4.7 with 1M context");
});
it("keeps the sticky active model even when getEnabledBaseModelIds excludes it", () => {
it("keeps the sticky active model even when the enabled set excludes it", () => {
// The active (sticky) model is no longer in the enabled set, but
// keepBaseModelId must preserve it so curation never strands the
// running selection.
@ -349,7 +362,7 @@ describe("buildPickerEntries", () => {
const opencode = {
...makeDescriptor("opencode"),
// Empty enabled set — nothing curated in.
getEnabledBaseModelIds: () => new Set<string>(),
getEnabledModelEntries: () => [],
} as unknown as BackendDescriptor;
const manager = makeManager({
cachedStateById: {
@ -370,6 +383,86 @@ describe("buildPickerEntries", () => {
});
});
// ---- appendBackendSection (enabled-driven credential flags) ----
describe("appendBackendSection — getEnabledModelEntries path", () => {
function opencodeWithEntries(enabled: EnabledModelEntry[]): BackendDescriptor {
return {
...makeDescriptor("opencode"),
getEnabledModelEntries: () => enabled,
};
}
it("flags each enabled model by credential state and 'not offered by agent'", () => {
const enabled: EnabledModelEntry[] = [
{ baseModelId: "openrouter/a", name: "A", credentialState: "missing_key" },
{ baseModelId: "openrouter/c", name: "C", credentialState: "ok" },
{ baseModelId: "openrouter/d", name: "D", credentialState: "ok" },
];
const entries: ModelSelectorEntry[] = [];
appendBackendSection(entries, opencodeWithEntries(enabled), {
// Only `c` is reported by the agent; `d` is keyed+ok but unreported.
backendModels: [makeModelEntry("openrouter/c", "Reported C")],
keepBaseModelId: null,
settings: emptySettings,
});
const byId = Object.fromEntries(entries.map((e) => [e.name, e]));
expect(byId["openrouter/a"]._disabledReason).toBe("Add API key");
expect(byId["openrouter/c"]._disabledReason).toBeUndefined();
expect(byId["openrouter/d"]._disabledReason).toBe("Not offered by agent");
// Reported metadata enriches the row name when present.
expect(byId["openrouter/c"].displayName).toBe("Reported C");
});
it("flags a stale, unreported agent-native model as 'not offered by agent'", () => {
// claude/codex entries are always credentialState "ok"; an enabled id the
// agent no longer reports renders flagged rather than silently hidden.
const claude = {
...makeDescriptor("claude"),
getEnabledModelEntries: () => [
{ baseModelId: "opus-4-5", name: "Opus 4.5", credentialState: "ok" as const },
{ baseModelId: "retired-model", name: "Retired", credentialState: "ok" as const },
],
} as unknown as BackendDescriptor;
const entries: ModelSelectorEntry[] = [];
appendBackendSection(entries, claude, {
backendModels: [makeModelEntry("opus-4-5", "Opus 4.5")],
keepBaseModelId: null,
settings: emptySettings,
});
const byId = Object.fromEntries(entries.map((e) => [e.name, e]));
expect(byId["opus-4-5"]._disabledReason).toBeUndefined();
expect(byId["retired-model"]._disabledReason).toBe("Not offered by agent");
});
it("appends a reported keepBaseModelId not already in the enabled set", () => {
const entries: ModelSelectorEntry[] = [];
appendBackendSection(
entries,
opencodeWithEntries([{ baseModelId: "openrouter/a", name: "A", credentialState: "ok" }]),
{
backendModels: [makeModelEntry("openrouter/a"), makeModelEntry("sticky")],
keepBaseModelId: "sticky",
settings: emptySettings,
}
);
const names = entries.map((e) => e.name);
expect(names).toContain("sticky");
expect(entries.find((e) => e.name === "sticky")?._disabledReason).toBeUndefined();
});
it("defers to the loading placeholder during preload (no reported catalog yet)", () => {
const entries: ModelSelectorEntry[] = [];
appendBackendSection(
entries,
opencodeWithEntries([{ baseModelId: "openrouter/a", name: "A", credentialState: "ok" }]),
{ backendModels: null, keepBaseModelId: null, settings: emptySettings }
);
// No flags before the catalog loads — buildPickerEntries shows "Loading…".
expect(entries).toHaveLength(0);
});
});
// ---- buildEffortSibling ----
describe("buildEffortSibling", () => {

93
src/agentMode/ui/agentModelPickerHelpers.ts
View file

@ -12,6 +12,8 @@ import type {
BackendDescriptor,
BackendId,
BackendState,
EnabledModelCredentialState,
EnabledModelEntry,
ModelEntry,
ModelState,
} from "@/agentMode/session/types";
@ -53,10 +55,22 @@ export function handlePickerSwitchError(err: unknown, action: "model" | "effort"
export const AGENT_PROVIDER = "agent";
/**
* Append one backend's section to the picker: its reported catalog filtered to
* the backend's enabled set (`getEnabledBaseModelIds`). The active session's
* selection is always kept via `keepBaseModelId` so curation never strands it.
* A descriptor returning `null` is treated as an empty enabled set.
* Append one backend's section to the picker.
*
* One policy for every backend, via `getEnabledModelEntries`: iterate the
* backend's *enabled* models, enriched by the reported catalog when present.
* Every enabled model appears — one the agent dropped for a missing key, or
* no longer reports at all, is shown non-selectable with a flag
* (`"Add API key"`, `"Not offered by agent"`, …) rather than silently hidden.
* Agent-native backends (claude, codex) report `credentialState: "ok"`, so
* their only flag is `"Not offered by agent"` for a stale enabled id.
*
* The active session's selection (and an inactive backend's persisted default)
* is always kept via `keepBaseModelId`. A descriptor that implements no
* `getEnabledModelEntries` opts out entirely; `buildPickerEntries` still re-adds
* the active selection if curation stranded it. During preload (no reported
* catalog yet) we defer to the "Loading…" placeholder `buildPickerEntries`
* adds, so we don't flag "Not offered by agent" before the catalog loads.
*/
export function appendBackendSection(
entries: ModelSelectorEntry[],
@ -66,19 +80,76 @@ export function appendBackendSection(
backendModels: ReadonlyArray<ModelEntry> | null;
/** baseModelId of the active session — never filtered out. */
keepBaseModelId: string | null;
/** Current settings — read by `getEnabledBaseModelIds`. */
/** Current settings — read by `getEnabledModelEntries`. */
settings: CopilotSettings;
}
): void {
const enabledEntries = descriptor.getEnabledModelEntries?.(ctx.settings) ?? null;
if (!enabledEntries) return;
// Preload still pending → no reported catalog yet; let buildPickerEntries
// render the Loading placeholder instead of flagging prematurely.
if (!ctx.backendModels) return;
const enabledSet = descriptor.getEnabledBaseModelIds?.(ctx.settings) ?? null;
const filtered = ctx.backendModels.filter(
(entry) =>
enabledSet?.has(entry.baseModelId) === true || entry.baseModelId === ctx.keepBaseModelId
appendFromEnabledEntries(
entries,
descriptor,
enabledEntries,
ctx.backendModels,
ctx.keepBaseModelId
);
for (const m of filtered) {
entries.push(synthesizeAgentEntry(m.baseModelId, m.name, descriptor, m.description));
}
/**
* Enabled-set-driven section build. Each enabled model becomes a row; the
* reported catalog enriches its name/description when present. A credential
* issue (missing key) or "agent didn't offer it" sets
* `_disabledReason`, which `ModelSelector` renders as a right-side label on a
* non-selectable row. `keepBaseModelId` (the active/default selection) is
* appended as a selectable row when it's reported but not already in the
* enabled set, so curation never strands the running selection.
*/
function appendFromEnabledEntries(
entries: ModelSelectorEntry[],
descriptor: BackendDescriptor,
enabledEntries: ReadonlyArray<EnabledModelEntry>,
backendModels: ReadonlyArray<ModelEntry>,
keepBaseModelId: string | null
): void {
const reportedById = new Map(backendModels.map((m) => [m.baseModelId, m]));
const emitted = new Set<string>();
for (const enabled of enabledEntries) {
const reported = reportedById.get(enabled.baseModelId);
const name = reported?.name || enabled.name || enabled.baseModelId;
const subtitle = reported?.description ?? enabled.description;
const entry = synthesizeAgentEntry(enabled.baseModelId, name, descriptor, subtitle);
const reason = credentialDisabledReason(enabled.credentialState, !!reported);
if (reason) entry._disabledReason = reason;
entries.push(entry);
emitted.add(enabled.baseModelId);
}
if (keepBaseModelId && !emitted.has(keepBaseModelId)) {
const reported = reportedById.get(keepBaseModelId);
if (reported) {
entries.push(
synthesizeAgentEntry(reported.baseModelId, reported.name, descriptor, reported.description)
);
}
}
}
/**
* Map an enabled model's credential state (+ whether the agent reported it) to
* a `_disabledReason`, or `undefined` when the row is selectable. Credential
* problems take precedence over "not offered" — they're the root cause of the
* agent dropping the model.
*/
function credentialDisabledReason(
state: EnabledModelCredentialState,
reported: boolean
): string | undefined {
if (state === "missing_key") return "Add API key";
if (!reported) return "Not offered by agent";
return undefined;
}
export function synthesizeAgentEntry(

1
src/modelManagement/index.ts
View file

@ -33,6 +33,7 @@ export type { CatalogDownloadDeps, CatalogRefreshResult } from "./catalog/Catalo
export { ProviderRegistry } from "./providers/ProviderRegistry";
export { isSelfHostedProvider, isSelfHostedUrl } from "./providers/isSelfHostedProvider";
export { providerRequiresApiKey } from "./providers/providerRequiresApiKey";
export { ConfiguredModelRegistry } from "./models/ConfiguredModelRegistry";
export { BackendConfigRegistry } from "./backends/BackendConfigRegistry";
export { ChatModelFactory } from "./chatModel/ChatModelFactory";

39
src/modelManagement/providers/adapters/openaiCompatibleAdapter.test.ts
View file

@ -98,4 +98,43 @@ describe("openaiCompatibleAdapter.verifyCredentials", () => {
});
expect(mockVerify).toHaveBeenCalledWith("https://example.test/v1/models", expect.any(Object));
});
it("verifies OpenRouter against the auth-gated /key path (public /models would 200)", async () => {
await openaiCompatibleAdapter.verifyCredentials({
provider: provider({
baseUrl: "https://openrouter.ai/api/v1",
origin: { kind: "byok", catalogProviderId: "openrouter" },
}),
apiKey: "sk-or",
extras: {},
});
expect(mockVerify).toHaveBeenCalledWith("https://openrouter.ai/api/v1/key", {
Authorization: "Bearer sk-or",
});
});
it("uses /models for non-OpenRouter catalog providers", async () => {
await openaiCompatibleAdapter.verifyCredentials({
provider: provider({ origin: { kind: "byok", catalogProviderId: "groq" } }),
apiKey: "k",
extras: {},
});
expect(mockVerify).toHaveBeenCalledWith("https://api.openai.com/v1/models", expect.any(Object));
});
it("maps a 401 from OpenRouter's /key to invalid_api_key", async () => {
// verifyViaListModels already maps 401/403 → invalid_api_key regardless of
// the path, so the /key probe inherits the right classification.
mockVerify.mockResolvedValue({ ok: false, code: "invalid_api_key", checkedAt: 1 });
const result = await openaiCompatibleAdapter.verifyCredentials({
provider: provider({
baseUrl: "https://openrouter.ai/api/v1",
origin: { kind: "byok", catalogProviderId: "openrouter" },
}),
apiKey: "sk-wrong",
extras: {},
});
expect(result.code).toBe("invalid_api_key");
expect(mockVerify).toHaveBeenCalledWith("https://openrouter.ai/api/v1/key", expect.any(Object));
});
});

15
src/modelManagement/providers/adapters/openaiCompatibleAdapter.ts
View file

@ -13,7 +13,9 @@
*
* Verification hits the standard `GET /models` endpoint — implemented
* by every public OpenAI-compatible provider and by both local
* runners (Ollama / LMStudio).
* runners (Ollama / LMStudio) — except for providers whose `/models`
* is public (OpenRouter), which verify against an auth-gated path via
* `openaiCompatibleVerifyPath`.
*/
import { z } from "zod";
@ -22,6 +24,7 @@ import type { BaseChatModel } from "@langchain/core/language_models/chat_models"
import type { VerificationResult } from "@/modelManagement/types/runtime";
import type { AdapterBuildContext, AdapterVerifyContext, ProviderAdapter } from "./ProviderAdapter";
import { verifyPathForCatalogProviderId } from "./openaiCompatibleVerifyPath";
import { verifyViaListModels } from "./verifyViaListModels";
const extrasSchema = z
@ -69,6 +72,14 @@ export const openaiCompatibleAdapter: ProviderAdapter<Extras> = {
headers["OpenAI-Organization"] = ctx.extras.openAIOrgId;
}
return verifyViaListModels(`${base}/models`, headers);
// Most providers gate `/models` behind the key; the few whose `/models`
// is public (OpenRouter) verify against an auth-gated path instead, keyed
// by catalog id. `verifyViaListModels` already maps 401/403 →
// `invalid_api_key`, so the `/key` probe needs no special handling.
const catalogProviderId =
ctx.provider.origin.kind === "byok" ? ctx.provider.origin.catalogProviderId : undefined;
const verifyPath = verifyPathForCatalogProviderId(catalogProviderId);
return verifyViaListModels(`${base}/${verifyPath}`, headers);
},
};

34
src/modelManagement/providers/adapters/openaiCompatibleVerifyPath.ts Normal file
View file

@ -0,0 +1,34 @@
/**
* Per-provider verification path for OpenAI-compatible endpoints.
*
* Most hosted OpenAI-compatible providers (OpenAI, Groq, Mistral, DeepSeek,
* xAI) gate `GET /models` behind the API key, so verify-via-`/models` proves
* the key is valid. OpenRouter is the notable exception: its `/models` is
* public and returns 200 for any (or no) `Authorization`, so a blank/wrong key
* would read as "Verified". OpenRouter exposes `GET /api/v1/key` which 401s on
* a bad key and 200s on a valid one, so we probe `/key` for it instead.
*
* Data-driven by `catalogProviderId` so adding another public-`/models`
* provider is a one-line entry, never an `if (openrouter)` branch.
*/
/** The path suffix appended to the provider's base URL when verifying. */
export type VerifyPath = "models" | "key";
const DEFAULT_VERIFY_PATH: VerifyPath = "models";
/** Catalog provider ids whose `/models` is public → verify against an
* auth-gated endpoint instead. */
const VERIFY_PATH_BY_CATALOG_ID: Record<string, VerifyPath> = {
openrouter: "key",
};
/**
* The verification path for a given catalog provider id. Returns `"models"`
* for everything not in the override map (including custom endpoints with no
* catalog id).
*/
export function verifyPathForCatalogProviderId(catalogProviderId: string | undefined): VerifyPath {
if (!catalogProviderId) return DEFAULT_VERIFY_PATH;
return VERIFY_PATH_BY_CATALOG_ID[catalogProviderId] ?? DEFAULT_VERIFY_PATH;
}

35
src/modelManagement/providers/providerRequiresApiKey.test.ts Normal file
View file

@ -0,0 +1,35 @@
import type { Provider } from "@/modelManagement/types/persisted";
import { providerRequiresApiKey } from "./providerRequiresApiKey";
function provider(overrides: Partial<Provider> = {}): Provider {
return {
providerId: "p1",
providerType: "openai-compatible",
displayName: "P",
origin: { kind: "byok" },
addedAt: 0,
apiKeyKeychainId: null,
...overrides,
};
}
describe("providerRequiresApiKey", () => {
it("returns the explicit flag — the only runtime criteria", () => {
// The flag wins regardless of identity: a hosted catalog provider marked
// keyless reads keyless; a self-hosted row marked key-requiring reads so.
expect(
providerRequiresApiKey(
provider({ requiresApiKey: false, origin: { kind: "byok", catalogProviderId: "openai" } })
)
).toBe(false);
expect(
providerRequiresApiKey(provider({ requiresApiKey: true, baseUrl: "http://localhost:11434" }))
).toBe(true);
});
it("defaults a flagless row to key-requiring (defensive backstop)", () => {
// Post-migration every persisted row carries the flag; a stray undefined
// must never read as keyless or its models would be silently dropped.
expect(providerRequiresApiKey(provider({ requiresApiKey: undefined }))).toBe(true);
});
});

18
src/modelManagement/providers/providerRequiresApiKey.ts Normal file
View file

@ -0,0 +1,18 @@
import type { Provider } from "@/modelManagement/types/persisted";
/**
* The single runtime read point for "does this provider need an API key".
*
* Reads the explicit, persisted `Provider.requiresApiKey` flag — written at
* every creation path (BYOK setup, agent setup, Plus sign-in) and backfilled
* onto legacy rows by the settings-v5 migration. Self-hosting is NOT the
* criteria — a self-hosted endpoint can sit behind an auth proxy, and a hosted
* provider can be keyless — so the answer is always the stored flag.
*
* The `?? true` is a defensive backstop only: post-migration every persisted
* row carries the flag, but a stray flagless row defaults to key-requiring so a
* key-gated provider is never silently treated as keyless.
*/
export function providerRequiresApiKey(provider: Provider): boolean {
return provider.requiresApiKey ?? true;
}

3
src/modelManagement/setup/AgentSetupApi.ts
View file

@ -119,6 +119,9 @@ export class AgentSetupApi {
baseUrl: input.baseUrl,
origin: { kind: "agent", agentType: input.agentType },
extras: input.extras,
// Agent-owned providers route through the agent's own auth (native /
// CLI-managed); the user never supplies a BYOK key for them.
requiresApiKey: false,
});
}

8
src/modelManagement/setup/ByokSetupApi.ts
View file

@ -59,6 +59,11 @@ export interface SetupProviderInput {
/** Per-providerType payload (Azure deployment, Bedrock region,
* OpenAI org id, …). */
extras?: Record<string, unknown>;
/** Whether this provider needs an API key. Flows through from the
* `ProviderDefinition` (`state.source.requiresApiKey`); persisted on the
* `Provider` row so the requires-key question never re-infers from the
* endpoint. Defaults to `true` when omitted (a hosted provider). */
requiresApiKey?: boolean;
/** Full `ModelInfo` snapshots for every model the user selected.
* The caller is responsible for catalog enrichment so the API can
* stay catalog-agnostic. */
@ -103,6 +108,9 @@ export class ByokSetupApi {
providerType: input.providerType,
displayName: input.displayName,
baseUrl: input.baseUrl,
// Persist the explicit requires-key flag (default hosted = needs a key)
// so the runtime never re-infers it from the endpoint.
requiresApiKey: input.requiresApiKey ?? true,
origin: {
kind: "byok",
...(input.catalogProviderId ? { catalogProviderId: input.catalogProviderId } : {}),

2
src/modelManagement/setup/CopilotPlusSetupApi.ts
View file

@ -103,6 +103,8 @@ export class CopilotPlusSetupApi {
displayName: input.displayName,
baseUrl: input.baseUrl,
origin: { kind: "copilot-plus" },
// Provisioned by Plus sign-in, not a user-entered BYOK key.
requiresApiKey: false,
});
}

9
src/modelManagement/types/persisted.ts
View file

@ -81,6 +81,15 @@ export interface Provider {
* API key (Ollama, LMStudio, some agent-owned providers).
*/
apiKeyKeychainId?: string | null;
/**
* Whether this provider needs an API key to function. Authoritative and
* explicit — never inferred from self-hosting at runtime (self-hosted ≠
* keyless; see `providerRequiresApiKey`). Written at every creation path
* (BYOK setup, agent setup, Plus sign-in). Optional only for rows that
* predate the flag; the settings-v5 migration backfills those by identity, so
* at runtime `providerRequiresApiKey` reads the flag directly.
*/
requiresApiKey?: boolean;
/**
* Opaque per-`providerType` payload.
* azure: { azureDeploymentName, azureApiVersion, azureInstanceName }

128
src/modelManagement/ui/dialogs/ConfigureProviderDialog.test.tsx
View file

@ -214,10 +214,13 @@ describe("ConfigureProviderForm (new mode)", () => {
it("calls setupProvider with the catalog id + enriched model metadata", async () => {
mockGetProvider.mockReturnValue(anthropicCatalogMetadata);
mockVerifyCredentials.mockResolvedValue({ ok: true, checkedAt: 1 });
const onClose = jest.fn();
render(
<ConfigureProviderForm state={{ mode: "new", source: anthropicSource }} onClose={onClose} />
);
// A required-key provider can't save without a key, so supply one.
fireEvent.change(screen.getByTestId("api-key"), { target: { value: "sk-ant" } });
manualAddId("claude-sonnet");
fireEvent.click(screen.getByRole("button", { name: "Save" }));
await waitFor(() => expect(mockSetupProvider).toHaveBeenCalledTimes(1));
@ -345,7 +348,8 @@ describe("ConfigureProviderForm (edit mode)", () => {
render(
<ConfigureProviderForm state={{ mode: "edit", providerId: "p1" }} onClose={jest.fn()} />
);
fireEvent.click(screen.getByRole("button", { name: "Test" }));
// The body mounts once the gate resolves the saved key.
fireEvent.click(await screen.findByRole("button", { name: "Test" }));
await waitFor(() => expect(mockVerifyCredentials).toHaveBeenCalled());
expect(mockGetApiKey).toHaveBeenCalledWith("p1");
expect(mockSetApiKey).not.toHaveBeenCalled();
@ -356,7 +360,7 @@ describe("ConfigureProviderForm (edit mode)", () => {
render(
<ConfigureProviderForm state={{ mode: "edit", providerId: "p1" }} onClose={jest.fn()} />
);
const baseUrlInput = screen.getByPlaceholderText("https://api.anthropic.com");
const baseUrlInput = await screen.findByPlaceholderText("https://api.anthropic.com");
fireEvent.change(baseUrlInput, { target: { value: "https://proxy.example.com" } });
fireEvent.click(screen.getByRole("button", { name: "Test" }));
await waitFor(() => expect(mockVerifyCredentials).toHaveBeenCalled());
@ -462,14 +466,32 @@ describe("ConfigureProviderForm (edit mode)", () => {
await waitFor(() => expect(onClose).toHaveBeenCalled());
});
it("Clear button wipes the saved API key and closes the dialog", async () => {
it("Clear empties the field, keeps the dialog open, and persists nothing immediately", async () => {
mockGetApiKey.mockResolvedValue("saved-secret");
const onClose = jest.fn();
render(<ConfigureProviderForm state={{ mode: "edit", providerId: "p1" }} onClose={onClose} />);
const clear = await screen.findByTestId("api-key-clear");
fireEvent.click(clear);
await waitFor(() => expect(mockClearApiKey).toHaveBeenCalledWith("p1"));
await waitFor(() => expect(onClose).toHaveBeenCalled());
// Field is wiped, but the keychain isn't touched and the dialog stays open —
// the removal is staged for Save, like every other field.
await waitFor(() => expect(screen.getByTestId<HTMLInputElement>("api-key").value).toBe(""));
expect(mockClearApiKey).not.toHaveBeenCalled();
expect(onClose).not.toHaveBeenCalled();
// Nothing left to clear → the button is gone.
expect(screen.queryByTestId("api-key-clear")).toBeNull();
});
it("disables Save after clearing a required-key provider's key", async () => {
mockGetApiKey.mockResolvedValue("saved-secret");
render(
<ConfigureProviderForm state={{ mode: "edit", providerId: "p1" }} onClose={jest.fn()} />
);
// p1 (anthropic, byok + catalogProviderId) requires a key and seeds two
// selected models, so the empty field is the only thing blocking Save.
fireEvent.click(await screen.findByTestId("api-key-clear"));
await waitFor(() =>
expect(screen.getByRole("button", { name: "Save" }).hasAttribute("disabled")).toBe(true)
);
});
});
@ -526,3 +548,99 @@ describe("ConfigureProviderForm (custom-openai source)", () => {
expect(screen.getByPlaceholderText("e.g. gpt-5.5")).toBeTruthy();
});
});
describe("ConfigureProviderForm (credential verification + save gating)", () => {
it("A1: a required-key provider with an empty field fails Test without probing", async () => {
render(
<ConfigureProviderForm state={{ mode: "new", source: anthropicSource }} onClose={jest.fn()} />
);
fireEvent.click(screen.getByRole("button", { name: "Test" }));
// No network probe — the guard short-circuits so a public /models 200
// can't read as "Verified".
expect(mockVerifyCredentials).not.toHaveBeenCalled();
expect(await screen.findByText("Enter an API key to verify this provider.")).toBeTruthy();
});
it("B2: Save is disabled for a required-key provider with no key", () => {
render(
<ConfigureProviderForm state={{ mode: "new", source: anthropicSource }} onClose={jest.fn()} />
);
manualAddId("claude-sonnet");
expect(screen.getByRole("button", { name: "Save" }).hasAttribute("disabled")).toBe(true);
});
it("B2: an untested invalid key auto-verifies on Save, aborts, and blocks further Save", async () => {
mockVerifyCredentials.mockResolvedValue({
ok: false,
code: "invalid_api_key",
message: "Authentication failed",
checkedAt: 1,
});
render(
<ConfigureProviderForm state={{ mode: "new", source: anthropicSource }} onClose={jest.fn()} />
);
fireEvent.change(screen.getByTestId("api-key"), { target: { value: "sk-bad" } });
manualAddId("claude-sonnet");
// Untested key → Save is enabled, but Save auto-verifies first.
expect(screen.getByRole("button", { name: "Save" }).hasAttribute("disabled")).toBe(false);
fireEvent.click(screen.getByRole("button", { name: "Save" }));
await waitFor(() => expect(mockVerifyCredentials).toHaveBeenCalled());
expect(mockSetupProvider).not.toHaveBeenCalled();
// The conclusive failure now disables Save.
await waitFor(() =>
expect(screen.getByRole("button", { name: "Save" }).hasAttribute("disabled")).toBe(true)
);
});
it("B2: an inconclusive verification (network) does not block Save", async () => {
mockVerifyCredentials.mockResolvedValue({
ok: false,
code: "network",
message: "connection refused",
checkedAt: 1,
});
const onClose = jest.fn();
render(
<ConfigureProviderForm state={{ mode: "new", source: anthropicSource }} onClose={onClose} />
);
fireEvent.change(screen.getByTestId("api-key"), { target: { value: "sk-maybe" } });
manualAddId("claude-sonnet");
fireEvent.click(screen.getByRole("button", { name: "Save" }));
// Offline users aren't stranded — the provider still saves.
await waitFor(() => expect(mockSetupProvider).toHaveBeenCalledTimes(1));
await waitFor(() => expect(onClose).toHaveBeenCalled());
});
it("B1/B2: edit mode pre-fills the saved key and saves without re-typing or re-writing it", async () => {
mockGetApiKey.mockResolvedValue("saved-secret");
mockBulkSet.mockResolvedValue(["cm1", "cm2"]);
const onClose = jest.fn();
render(<ConfigureProviderForm state={{ mode: "edit", providerId: "p1" }} onClose={onClose} />);
// The field is seeded with the stored key (visible, masked by PasswordInput).
const input = await screen.findByTestId<HTMLInputElement>("api-key");
expect(input.value).toBe("saved-secret");
fireEvent.click(screen.getByRole("button", { name: "Save" }));
await waitFor(() => expect(onClose).toHaveBeenCalled());
// Unchanged key → no keychain churn and no re-verification.
expect(mockSetApiKey).not.toHaveBeenCalled();
expect(mockVerifyCredentials).not.toHaveBeenCalled();
});
it("B2: a keyless provider (requiresApiKey:false) Tests and Saves with an empty field", async () => {
mockVerifyCredentials.mockResolvedValue({ ok: true, checkedAt: 1 });
const onClose = jest.fn();
render(
<ConfigureProviderForm state={{ mode: "new", source: ollamaSource }} onClose={onClose} />
);
// Test with an empty field probes the endpoint (no required-key guard).
fireEvent.click(screen.getByRole("button", { name: "Test" }));
await waitFor(() => expect(mockVerifyCredentials).toHaveBeenCalled());
manualAddId("llama3.2");
fireEvent.click(screen.getByRole("button", { name: "Save" }));
await waitFor(() => expect(mockSetupProvider).toHaveBeenCalledTimes(1));
expect(mockSetupProvider).toHaveBeenCalledWith(
expect.objectContaining({ requiresApiKey: false })
);
await waitFor(() => expect(onClose).toHaveBeenCalled());
});
});

262
src/modelManagement/ui/dialogs/ConfigureProviderDialog.tsx
View file

@ -32,6 +32,7 @@ import { useApp } from "@/context";
import { logError } from "@/logger";
import type { ModelManagementApi } from "@/modelManagement/createModelManagement";
import { BYOK_DEFAULT_AUTO_ENROLL } from "@/modelManagement/setup/ByokSetupApi";
import { providerRequiresApiKey } from "@/modelManagement/providers/providerRequiresApiKey";
import { byokProvidersAtom, configuredModelsAtom } from "@/modelManagement/state/atoms";
import type { ModelInfo, ProviderType } from "@/modelManagement/types/catalog";
import type { ConfiguredModel, Provider } from "@/modelManagement/types/persisted";
@ -72,12 +73,16 @@ interface ConfigureProviderFormProps {
}
/**
* Gate: resolve the persisted provider row and only mount the stateful body
* once it has hydrated. Without this, the body's `useState` initializers
* could read an empty `provider` during the atom-load race and lock in blank
* values the user would then unwittingly Save.
* Gate: resolve the persisted provider row AND its saved API key, and only
* mount the stateful body once both have hydrated. Without this, the body's
* `useState` initializers could read an empty `provider` / blank key during
* the atom-load race and lock in values the user would then unwittingly Save.
* Resolving the key here (rather than probing inside the body) lets edit mode
* seed the key field with the real value — so a genuinely keyless provider is
* visibly empty — without an empty→filled flash.
*/
export const ConfigureProviderForm: React.FC<ConfigureProviderFormProps> = ({ state, onClose }) => {
const api = useModelManagement();
const byokProviders = useAtomValue(byokProvidersAtom, { store: settingsStore });
const configuredModels = useAtomValue(configuredModelsAtom, { store: settingsStore });
@ -97,7 +102,37 @@ export const ConfigureProviderForm: React.FC<ConfigureProviderFormProps> = ({ st
[state, configuredModels]
);
if (state.mode === "edit" && !provider) {
// Edit mode: read the stored key once so the body can seed its field with
// it. New mode resolves immediately with no key. A probe failure resolves
// to `null` (treated as keyless) rather than wedging the spinner.
//
// Two primitive states (not one object) so a re-run that resolves the same
// value bails out via React's `Object.is` short-circuit — the
// `useModelManagement()` hook returns a fresh object each render, so an
// object state here would re-render forever. Keyed on the primitive
// `providerId` so new mode never schedules a probe.
const keyProviderId = state.mode === "edit" ? state.providerId : null;
const [initialApiKey, setInitialApiKey] = useState<string | null>(null);
const [keyResolved, setKeyResolved] = useState(keyProviderId === null);
useEffect(() => {
if (keyProviderId === null) return;
let cancelled = false;
void api.providerRegistry
.getApiKey(keyProviderId)
.then((key) => {
if (cancelled) return;
setInitialApiKey(key);
setKeyResolved(true);
})
.catch(() => {
if (!cancelled) setKeyResolved(true);
});
return () => {
cancelled = true;
};
}, [keyProviderId, api]);
if (state.mode === "edit" && (!provider || !keyResolved)) {
return (
<div className="tw-flex tw-h-full tw-items-center tw-justify-center">
<Loader2 className="tw-size-5 tw-animate-spin tw-text-muted" />
@ -111,6 +146,7 @@ export const ConfigureProviderForm: React.FC<ConfigureProviderFormProps> = ({ st
onClose={onClose}
provider={provider}
existingModels={existingModels}
initialApiKey={initialApiKey}
/>
);
};
@ -121,6 +157,9 @@ interface ConfigureProviderBodyProps {
/** Guaranteed defined in edit mode (the gate waits for it); undefined in new mode. */
provider: Provider | undefined;
existingModels: readonly ConfiguredModel[];
/** Edit mode: the stored API key, resolved by the gate. `null` for a
* keyless provider (or a probe failure). Always `null` in new mode. */
initialApiKey: string | null;
}
const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
@ -128,6 +167,7 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
onClose,
provider,
existingModels,
initialApiKey,
}) => {
const api = useModelManagement();
const app = useApp();
@ -153,7 +193,9 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
const [displayName, setDisplayName] = useState(() =>
state.mode === "new" ? state.source.displayName : (provider?.displayName ?? "")
);
const [apiKey, setApiKey] = useState("");
// Edit mode seeds with the resolved stored key (plaintext — it's what
// opencode injects; the field masks it via PasswordInput's reveal toggle).
const [apiKey, setApiKey] = useState(() => initialApiKey ?? "");
const [baseUrl, setBaseUrl] = useState(() =>
state.mode === "edit" ? (provider?.baseUrl ?? "") : ""
);
@ -165,11 +207,11 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
const [saving, setSaving] = useState(false);
const [modelQuery, setModelQuery] = useState("");
const hasSavedKey = useSavedKeyProbe(
state.mode,
state.mode === "edit" ? state.providerId : undefined,
api
);
// Whether this provider needs a key. New mode reads the picked definition;
// edit mode reads the explicit persisted flag — never inferred from the
// endpoint. Drives the Test guard, the save gate, and the inline hint.
const requiresApiKey =
state.mode === "new" ? state.source.requiresApiKey : providerRequiresApiKey(provider!);
const defaultBaseUrl =
state.mode === "new"
@ -188,41 +230,61 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
catalogMetadata,
apiKey,
extras,
requiresApiKey: state.mode === "new" ? state.source.requiresApiKey : false,
requiresApiKey,
providerHydrated: state.mode === "edit" ? !!provider : true,
api,
});
// Single verification path shared by Test and save-time auto-verify. In new
// mode the synthetic provider carries `catalogProviderId` so the adapter's
// per-provider verify path (e.g. OpenRouter's auth-gated `/key`) resolves —
// without it OpenRouter's public `/models` would 200 on a blank key.
const runVerification = async (): Promise<VerificationResult> => {
if (state.mode === "new") {
const synthetic: Provider = {
providerId: "test",
providerType: providerType!,
displayName,
baseUrl: effectiveBaseUrl || undefined,
requiresApiKey: state.source.requiresApiKey,
extras,
origin: {
kind: "byok",
...(state.source.catalogProviderId
? { catalogProviderId: state.source.catalogProviderId }
: {}),
},
addedAt: Date.now(),
};
return api.adapters.verifyCredentials(providerType!, {
provider: synthetic,
apiKey: apiKey || null,
extras: extras ?? {},
});
}
return api.adapters.verifyCredentials(providerType!, {
provider: { ...provider!, displayName, baseUrl: effectiveBaseUrl || undefined, extras },
apiKey: apiKey || null,
extras: extras ?? {},
});
};
const handleTest = async (): Promise<void> => {
if (!providerType) return;
// A1: a required-key provider with an empty field cannot read as
// "Verified" — a public `/models` 200 would lie. Fail fast, no probe.
if (requiresApiKey && apiKey.trim().length === 0) {
setVerification({
ok: false,
code: "missing_api_key",
message: "Enter an API key to verify this provider.",
checkedAt: Date.now(),
});
return;
}
setTesting(true);
try {
let result: VerificationResult;
if (state.mode === "new") {
const synthetic: Provider = {
providerId: "test",
providerType,
displayName,
baseUrl: effectiveBaseUrl || undefined,
extras,
origin: { kind: "byok" },
addedAt: Date.now(),
};
result = await api.adapters.verifyCredentials(providerType, {
provider: synthetic,
apiKey: apiKey || null,
extras: extras ?? {},
});
} else if (provider) {
const key = apiKey || (await api.providerRegistry.getApiKey(state.providerId));
result = await api.adapters.verifyCredentials(providerType, {
provider: { ...provider, displayName, baseUrl: effectiveBaseUrl || undefined, extras },
apiKey: key || null,
extras: extras ?? {},
});
} else {
return;
}
const result = await runVerification();
setVerification(result);
if (result.ok) {
// Successful auth — refetch the model list, since the previous
@ -244,12 +306,24 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
if (state.mode !== "new" || !providerType) return;
setSaving(true);
try {
// B2: auto-verify a present key before persisting so an untested-but-
// invalid key is caught even if the user never clicked Test. Abort on a
// conclusive failure; proceed on `ok` or an inconclusive result (offline
// users aren't stranded). A keyless field skips the probe.
if (apiKey.trim().length > 0) {
const result = await runVerification();
if (!result.ok && isConclusiveVerificationFailure(result.code)) {
setVerification(result);
return;
}
}
await api.setup.byok.setupProvider({
catalogProviderId: state.source.catalogProviderId,
providerType,
displayName,
baseUrl: effectiveBaseUrl || undefined,
apiKey: apiKey || undefined,
requiresApiKey: state.source.requiresApiKey,
extras,
models: pool.buildSelectedModelInfos(),
});
@ -266,9 +340,20 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
if (state.mode !== "edit" || !provider) return;
setSaving(true);
try {
// B2: re-verify only a *changed* key (unchanged keys skip the probe and
// the keychain re-write). Abort on a conclusive failure.
const keyChanged = apiKey !== (initialApiKey ?? "");
if (keyChanged && apiKey.trim().length > 0) {
const result = await runVerification();
if (!result.ok && isConclusiveVerificationFailure(result.code)) {
setVerification(result);
return;
}
}
await saveProviderEdit({
providerId: state.providerId,
apiKey,
initialApiKey,
displayName,
effectiveBaseUrl,
extras,
@ -286,15 +371,12 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
}
};
const handleClearKey = async (): Promise<void> => {
if (state.mode !== "edit") return;
try {
await api.providerRegistry.clearApiKey(state.providerId);
onClose();
} catch (err) {
logError("[ConfigureProviderDialog] clearApiKey failed", err);
new Notice("Failed to clear API key. See console for details.");
}
// Stage the clear locally (like every other field); the keychain write is
// deferred to Save via saveProviderEdit. Empties the field so a required-key
// provider lands in the same un-saveable "no key" state as a fresh setup.
const handleClearKey = (): void => {
setApiKey("");
setVerification(null);
};
const handleRemove = (): void => {
@ -321,11 +403,19 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
const headerName =
state.mode === "new" ? state.source.displayName : (provider?.displayName ?? displayName);
// B2: a required-key provider with an empty field can't be saved.
const missingRequiredKey = requiresApiKey && apiKey.trim().length === 0;
// B2: a conclusively-failed verification blocks Save. Inconclusive results
// (network / timeout / rate_limited / http_error) do NOT block — offline
// users can still save. A key edit resets `verification` to `null`, so an
// untested key falls through to the save-time auto-verify in the handlers.
const verificationBlocksSave = isConclusiveVerificationFailure(verification?.code);
// The candidate pool only fills with models the endpoint listed (which
// requires working credentials) or ones the user explicitly typed, so a
// non-empty selection already implies a usable setup. No need to
// re-gate on base URL or test status.
const canSave = pool.selectedWireIds.size > 0;
// non-empty selection already implies a usable setup. On top of that, gate
// on a present + non-conclusively-invalid key.
const canSave = pool.selectedWireIds.size > 0 && !missingRequiredKey && !verificationBlocksSave;
const testFailed = verification?.ok === false;
@ -351,7 +441,9 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
label={
<span className="tw-inline-flex tw-items-center tw-gap-2">
API key
<span className="tw-text-ui-smaller tw-font-normal tw-text-muted">optional</span>
<span className="tw-text-ui-smaller tw-font-normal tw-text-muted">
{requiresApiKey ? "required" : "optional"}
</span>
{verification?.ok === true && (
<Badge className="tw-gap-1 tw-bg-success tw-text-success">
<CheckCircle2 className="tw-size-3" />
@ -370,26 +462,28 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
setVerification(null);
}}
autoDecrypt={false}
placeholder={
state.mode === "edit"
? "Enter a new key to replace the saved one"
: "Paste your API key"
}
placeholder={state.mode === "edit" ? "No API key set" : "Paste your API key"}
/>
<Button variant="secondary" onClick={handleTest} disabled={testing}>
{testing ? <Loader2 className="tw-size-4 tw-animate-spin" /> : "Test"}
</Button>
{state.mode === "edit" && hasSavedKey && (
{state.mode === "edit" && apiKey.length > 0 && (
<Button variant="destructive" onClick={handleClearKey} data-testid="api-key-clear">
Clear
</Button>
)}
</div>
{testFailed && (
{testFailed ? (
<div className="tw-flex tw-items-center tw-gap-1.5 tw-text-xs tw-text-error">
<XCircle className="tw-size-3.5 tw-shrink-0" />
<span>{verification?.message || "Verification failed"}</span>
</div>
) : (
missingRequiredKey && (
<div className="tw-text-xs tw-text-muted">
An API key is required for this provider.
</div>
)
)}
</FormField>
@ -450,38 +544,26 @@ const ConfigureProviderBody: React.FC<ConfigureProviderBodyProps> = ({
);
};
/**
* Probe the keychain once in edit mode so the "Clear" button can show only
* when there's something to clear. Probe failures are non-fatal — the button
* just won't appear.
*/
function useSavedKeyProbe(
mode: ConfigureState["mode"],
providerId: string | undefined,
api: ModelManagementApi
): boolean {
const [hasSavedKey, setHasSavedKey] = useState(false);
useEffect(() => {
if (mode !== "edit" || !providerId) return;
let cancelled = false;
void api.providerRegistry
.getApiKey(providerId)
.then((key) => {
if (!cancelled) setHasSavedKey(!!key);
})
.catch(() => {
// non-fatal
});
return () => {
cancelled = true;
};
}, [mode, providerId, api]);
return hasSavedKey;
/** Verification codes that conclusively mean "don't save" — as opposed to
* inconclusive results (network / timeout / rate_limited / http_error) that
* shouldn't strand an offline user. */
const CONCLUSIVE_VERIFICATION_FAILURE_CODES: readonly string[] = [
"invalid_api_key",
"missing_api_key",
"missing_base_url",
];
function isConclusiveVerificationFailure(code: string | undefined): boolean {
return code !== undefined && CONCLUSIVE_VERIFICATION_FAILURE_CODES.includes(code);
}
interface SaveEditArgs {
providerId: string;
apiKey: string;
/** The key the field was seeded with — used to detect a real change so an
* unchanged key doesn't churn the keychain (which would emit and trigger a
* spurious opencode restart). */
initialApiKey: string | null;
displayName: string;
effectiveBaseUrl: string;
extras: Record<string, unknown>;
@ -504,6 +586,7 @@ interface SaveEditArgs {
async function saveProviderEdit({
providerId,
apiKey,
initialApiKey,
displayName,
effectiveBaseUrl,
extras,
@ -512,7 +595,14 @@ async function saveProviderEdit({
selectedInfos,
api,
}: SaveEditArgs): Promise<void> {
if (apiKey) await api.providerRegistry.setApiKey(providerId, apiKey);
// Touch the keychain only when the key actually changed — re-writing an
// unchanged key would emit and trigger a spurious opencode restart. A key
// cleared to empty drops the keychain entry (only reachable for keyless
// providers, since a required-key provider with an empty field can't Save).
if (apiKey !== (initialApiKey ?? "")) {
if (apiKey.trim().length > 0) await api.providerRegistry.setApiKey(providerId, apiKey);
else await api.providerRegistry.clearApiKey(providerId);
}
await api.providerRegistry.update(providerId, {
displayName,
baseUrl: effectiveBaseUrl || undefined,

18
src/settings/migrations/byokMigration.test.ts
View file

@ -312,6 +312,8 @@ describe("planByokMigration — local providers (custom URL required)", () => {
providerType: "openai-compatible",
baseUrl: "http://192.168.1.5:11434/v1",
autoEnrollIn: ["chat", "opencode"],
// Local runner → keyless.
requiresApiKey: false,
});
const withoutUrl = planByokMigration(
@ -319,6 +321,22 @@ describe("planByokMigration — local providers (custom URL required)", () => {
);
expect(withoutUrl).toEqual([]);
});
it("sets requiresApiKey:true for key-based providers, false for local runners", () => {
const plan = planByokMigration(
settingsWith([
model({ name: "claude-x", provider: ChatModelProviders.ANTHROPIC, apiKey: "sk-ant" }),
model({
name: "llama3.2",
provider: ChatModelProviders.OLLAMA,
baseUrl: "http://localhost:11434/v1",
}),
])
);
expect(byCatalog(plan, "anthropic")?.requiresApiKey).toBe(true);
const ollama = plan.find((p) => p.baseUrl?.includes("11434"));
expect(ollama?.requiresApiKey).toBe(false);
});
});
describe("executeByokMigration", () => {

4
src/settings/migrations/byokMigration.ts
View file

@ -256,6 +256,10 @@ export function planByokMigration(settings: CopilotSettings): SetupProviderInput
displayName: displayNameFor(model.provider),
models: [],
autoEnrollIn: mapping.opencodeRoutable ? ENROLL_CHAT_AND_OPENCODE : ENROLL_CHAT_ONLY,
// Local runners (`requiresBaseUrl`) migrate key-less; every other
// legacy mapping is key-based. Persist it explicitly so the runtime
// never re-infers from the endpoint.
requiresApiKey: !mapping.requiresBaseUrl,
};
if (mapping.catalogProviderId) input.catalogProviderId = mapping.catalogProviderId;
if (baseUrl) input.baseUrl = baseUrl;

24
src/settings/migrations/index.ts
View file

@ -4,9 +4,10 @@
* persisted, and before agent/model-discovery init so OpenCode first sees the
* migrated backends).
*
* The gate is deliberately a plain version comparison — there is a single
* one-time migration today (legacy BYOK → model-management). Promote this to an
* ordered runner only if a second migration ever accrues.
* Ordered runner: each migration is individually version-gated against the
* vault's stored version, so a vault already at an intermediate version only
* runs the migrations newer than it (the legacy BYOK migration never re-runs
* for a v4 vault picking up the v5 backfill).
*/
import { logInfo } from "@/logger";
@ -14,6 +15,7 @@ import type { ModelManagementApi } from "@/modelManagement";
import { getSettings, setSettings } from "@/settings/model";
import { executeByokMigration } from "./byokMigration";
import { planRequiresApiKeyBackfill } from "./requiresApiKeyMigration";
import { CURRENT_SETTINGS_VERSION } from "./version";
export { CURRENT_SETTINGS_VERSION } from "./version";
@ -27,8 +29,20 @@ export async function runSettingsMigrations(api: ModelManagementApi): Promise<vo
if (fromVersion >= CURRENT_SETTINGS_VERSION) return;
logInfo(`[settings-migration] migrating from v${fromVersion} to v${CURRENT_SETTINGS_VERSION}`);
await executeByokMigration(api, getSettings());
// Bump unconditionally after the migration so a per-provider failure can't
// v≤4: legacy BYOK models + keys → the model-management data model.
if (fromVersion < 4) {
await executeByokMigration(api, getSettings());
}
// v5: backfill the explicit `requiresApiKey` flag onto rows that predate it,
// so the runtime read point no longer needs an identity heuristic.
if (fromVersion < 5) {
const backfilled = planRequiresApiKeyBackfill(getSettings().providers);
if (backfilled) setSettings({ providers: backfilled });
}
// Bump unconditionally after the migrations so a per-provider failure can't
// wedge the gate into re-running on every load. Persists via the subscriber.
setSettings({ settingsVersion: CURRENT_SETTINGS_VERSION });
}

71
src/settings/migrations/requiresApiKeyMigration.test.ts Normal file
View file

@ -0,0 +1,71 @@
import type { Provider } from "@/modelManagement";
import { planRequiresApiKeyBackfill } from "./requiresApiKeyMigration";
function provider(overrides: Partial<Provider> = {}): Provider {
return {
providerId: "p1",
providerType: "openai-compatible",
displayName: "P",
origin: { kind: "byok" },
addedAt: 0,
apiKeyKeychainId: null,
...overrides,
};
}
/** Backfilled `requiresApiKey` for a single flagless row, via the planner. */
function backfilledFlag(overrides: Partial<Provider>): boolean | undefined {
const next = planRequiresApiKeyBackfill({ p1: provider(overrides) });
return next?.p1.requiresApiKey;
}
describe("planRequiresApiKeyBackfill", () => {
it("treats catalog-backed BYOK as key-requiring", () => {
expect(backfilledFlag({ origin: { kind: "byok", catalogProviderId: "anthropic" } })).toBe(true);
});
it("treats a self-hosted catalog-less BYOK as keyless", () => {
expect(backfilledFlag({ baseUrl: "http://localhost:11434/v1" })).toBe(false);
expect(backfilledFlag({ baseUrl: "http://192.168.1.9:1234/v1" })).toBe(false);
});
it("requires a key for a catalog-less BYOK pointed at a public host", () => {
expect(backfilledFlag({ baseUrl: "https://proxy.example/v1" })).toBe(true);
});
it("defaults a catalog-less BYOK with no base URL to key-requiring", () => {
expect(backfilledFlag({ baseUrl: undefined })).toBe(true);
});
it("treats agent-owned and Plus providers as keyless (auth managed elsewhere)", () => {
expect(backfilledFlag({ origin: { kind: "agent", agentType: "opencode" } })).toBe(false);
expect(backfilledFlag({ origin: { kind: "copilot-plus" } })).toBe(false);
});
it("never overwrites an already-explicit flag", () => {
// A catalog-backed row the heuristic would call key-requiring stays keyless
// when explicitly flagged so.
const next = planRequiresApiKeyBackfill({
p1: provider({
requiresApiKey: false,
origin: { kind: "byok", catalogProviderId: "openai" },
}),
});
expect(next).toBeNull();
});
it("returns null when there is nothing to backfill (referential stability)", () => {
expect(planRequiresApiKeyBackfill({})).toBeNull();
expect(planRequiresApiKeyBackfill({ p1: provider({ requiresApiKey: true }) })).toBeNull();
});
it("backfills only flagless rows, leaving flagged rows untouched", () => {
const flagged = provider({ providerId: "p2", requiresApiKey: true });
const next = planRequiresApiKeyBackfill({
p1: provider({ origin: { kind: "byok", catalogProviderId: "anthropic" } }),
p2: flagged,
});
expect(next?.p1.requiresApiKey).toBe(true);
expect(next?.p2).toBe(flagged); // unchanged reference
});
});

65
src/settings/migrations/requiresApiKeyMigration.ts Normal file
View file

@ -0,0 +1,65 @@
/**
* One-time migration (settings v5): stamp an explicit `Provider.requiresApiKey`
* on every row persisted before the flag existed.
*
* "Does this provider need an API key" is an authoritative, persisted flag —
* never inferred from self-hosting at runtime (self-hosted ≠ keyless). Every
* creation path now writes it (BYOK setup, agent setup, Plus sign-in), so the
* only flagless rows are legacy ones. This backfill resolves them by identity
* once, here, so the runtime read point (`providerRequiresApiKey`) can be a
* pure flag read with no heuristic fallback.
*
* This is the ONLY place the identity heuristic lives — relocated from the old
* runtime `legacyResolveRequiresApiKey`. A one-time migration is where a
* heuristic is acceptable; the runtime criteria is always the explicit flag.
*/
import { isSelfHostedProvider } from "@/modelManagement";
import type { Provider } from "@/modelManagement";
/**
* Resolve `requiresApiKey` for a flagless row by provider identity. Defaults to
* `true` when unknown so a key-gated provider is never silently treated as
* keyless (which would drop its models downstream).
*/
function resolveByIdentity(provider: Provider): boolean {
switch (provider.origin.kind) {
case "agent":
// Agent-owned providers route through the agent's own auth (native /
// CLI-managed); the user never supplies a BYOK key for them.
return false;
case "copilot-plus":
// Provisioned by Plus sign-in, not a user-entered key.
return false;
case "byok":
// Catalog-backed BYOK providers are hosted clouds — all require a key.
if (provider.origin.catalogProviderId) return true;
// Catalog-less BYOK: a self-hosted runner (Ollama / LM Studio / local
// proxy) commonly runs key-less; anything else (custom hosted proxy)
// needs one.
return !isSelfHostedProvider(provider);
default:
return true;
}
}
/**
* Pure planner: returns a new `providers` map with `requiresApiKey` backfilled
* on every flagless row, or `null` when nothing changed (so the caller can skip
* a redundant write — referential stability, see AGENTS.md).
*/
export function planRequiresApiKeyBackfill(
providers: Record<string, Provider>
): Record<string, Provider> | null {
let changed = false;
const next: Record<string, Provider> = {};
for (const [id, provider] of Object.entries(providers)) {
if (provider.requiresApiKey === undefined) {
next[id] = { ...provider, requiresApiKey: resolveByIdentity(provider) };
changed = true;
} else {
next[id] = provider;
}
}
return changed ? next : null;
}

29
src/settings/migrations/runSettingsMigrations.test.ts
View file

@ -85,6 +85,35 @@ it("bumps the version even when there is nothing to migrate", async () => {
expect(mockSetSettings).toHaveBeenCalledWith({ settingsVersion: CURRENT_SETTINGS_VERSION });
});
it("runs only the v5 backfill for a v4 vault (legacy BYOK migration does not re-run)", async () => {
mockGetSettings.mockReturnValue(
settings({
settingsVersion: 4,
providers: {
p1: {
providerId: "p1",
providerType: "openai-compatible",
displayName: "OpenRouter",
origin: { kind: "byok", catalogProviderId: "openrouter" },
addedAt: 0,
apiKeyKeychainId: null,
},
},
})
);
const { api, setupProvider } = makeApi();
await runSettingsMigrations(api);
expect(setupProvider).not.toHaveBeenCalled();
// Backfill writes the flag, then the version bump lands.
const providerWrite = mockSetSettings.mock.calls.find((call) => "providers" in call[0])?.[0] as
| { providers: Record<string, { requiresApiKey?: boolean }> }
| undefined;
expect(providerWrite?.providers.p1.requiresApiKey).toBe(true);
expect(mockSetSettings).toHaveBeenCalledWith({ settingsVersion: CURRENT_SETTINGS_VERSION });
});
it("skips when already at the current version", async () => {
mockGetSettings.mockReturnValue(settings({ settingsVersion: CURRENT_SETTINGS_VERSION }));
const { api, setupProvider } = makeApi();

13
src/settings/migrations/version.ts
View file

@ -5,8 +5,13 @@
* settings-persistence layer can stamp fresh installs without pulling in the
* model-management barrel that `runSettingsMigrations` depends on.
*
* `= 4` matches the v4 launch. Gate is `(settingsVersion ?? 0) < CURRENT`, so
* pre-versioned installs (real users → `0`) and the orphaned prototype `2`
* both run; migrated vaults (`4`) and freshly-stamped installs skip.
* Gate is `(settingsVersion ?? 0) < CURRENT`, so pre-versioned installs (real
* users → `0`) and the orphaned prototype `2` run every pending migration;
* freshly-stamped installs skip. Each migration is individually version-gated
* in `runSettingsMigrations`, so a vault already at an intermediate version
* only runs the migrations newer than it.
*
* ≤ 4 → legacy BYOK → model-management migration.
* 5 → backfill `Provider.requiresApiKey` on flagless rows.
*/
export const CURRENT_SETTINGS_VERSION = 4;
export const CURRENT_SETTINGS_VERSION = 5;

4
src/settings/v2/components/configuredModelGrouping.ts
View file

@ -24,8 +24,8 @@ export interface CandidatePartition {
*
* For opencode, a BYOK/Plus provider it can't route (`isOpencodeRoutable`
* false — azure / bedrock / self-hosted) is dropped to avoid a dead toggle:
* `opencodeEnabledWireIds` skips unroutable providers at injection and
* picker-filter time, so enabling one would never reach the agent or picker.
* `opencodeEnabledModelEntries` skips unroutable providers, so enabling one
* would never reach the agent or picker.
*/
export function partitionCandidates(
configuredModels: readonly ConfiguredModel[],