mirror of
https://github.com/logancyang/obsidian-copilot.git
synced 2026-07-22 07:50:24 +00:00
Add provider and configured model registry
This commit is contained in:
parent
05f8abf90a
commit
aaf72c19a1
12 changed files with 1033 additions and 103 deletions
|
|
@ -166,6 +166,7 @@ For detailed architecture diagrams and documentation, see [`MESSAGE_ARCHITECTURE
|
|||
- **Avoid hardcoding**: No hardcoded folder names, file patterns, or special-case logic
|
||||
- **Configuration over convention**: If behavior needs to vary, make it configurable, not hardcoded
|
||||
- **Universal patterns**: Solutions should work equally well for any folder structure, naming convention, or content type
|
||||
- **Referential stability — reuse constants for empty values; cache and validate derived views.** Never return a freshly-allocated `[]` / `{}` for an "empty" slice — define a frozen module-level constant (`Object.freeze(...)`) and return it. For derived or filtered views over a settings slice, cache the result on the class/module and validate it by comparing the source-slice reference (`===`) against the live settings on every call; recompute only on miss. Reason: Jotai derived atoms and React memoization both short-circuit on referential equality, so fresh allocations invalidate every downstream cache on every read. Canonical examples: `EMPTY_PROVIDERS` / `EMPTY_CONFIGURED_MODELS` / `EMPTY_BACKENDS` in `src/settings/model.ts`
|
||||
|
||||
### TypeScript
|
||||
|
||||
|
|
|
|||
|
|
@ -526,9 +526,7 @@ export class SkillManager {
|
|||
const patchResult = await this.runInternalMutation(
|
||||
() => runUpdateProperties({ skill: activeSkill, patch: req.patch, fs }),
|
||||
(r) =>
|
||||
r.ok && vaultRoot !== null
|
||||
? buildUpdatePropertiesExpectations(activeSkill, vaultRoot)
|
||||
: []
|
||||
r.ok && vaultRoot !== null ? buildUpdatePropertiesExpectations(activeSkill, vaultRoot) : []
|
||||
);
|
||||
if (!patchResult.ok) {
|
||||
if (newName !== undefined) {
|
||||
|
|
|
|||
|
|
@ -1034,6 +1034,9 @@ export const DEFAULT_SETTINGS: CopilotSettings = {
|
|||
importSkipList: [],
|
||||
},
|
||||
},
|
||||
providers: {},
|
||||
configuredModels: [],
|
||||
backends: {},
|
||||
};
|
||||
|
||||
export const EVENT_NAMES = {
|
||||
|
|
|
|||
198
src/modelManagement/models/ConfiguredModelRegistry.test.ts
Normal file
198
src/modelManagement/models/ConfiguredModelRegistry.test.ts
Normal file
|
|
@ -0,0 +1,198 @@
|
|||
/**
|
||||
* Tests for `ConfiguredModelRegistry`.
|
||||
*
|
||||
* Real settings store via `resetSettings` / `setSettings`. No keychain
|
||||
* or Obsidian APIs are touched.
|
||||
*/
|
||||
|
||||
import { getSettings, resetSettings } from "@/settings/model";
|
||||
|
||||
import type { ModelInfo } from "@/modelManagement/types/catalog";
|
||||
|
||||
import { ConfiguredModelRegistry } from "./ConfiguredModelRegistry";
|
||||
|
||||
jest.mock("@/logger", () => ({
|
||||
logInfo: jest.fn(),
|
||||
logWarn: jest.fn(),
|
||||
logError: jest.fn(),
|
||||
}));
|
||||
|
||||
const PROVIDER_A = "provider-a";
|
||||
const PROVIDER_B = "provider-b";
|
||||
|
||||
function info(id: string, displayName = id): ModelInfo {
|
||||
return { id, displayName };
|
||||
}
|
||||
|
||||
describe("ConfiguredModelRegistry", () => {
|
||||
let registry: ConfiguredModelRegistry;
|
||||
|
||||
beforeEach(() => {
|
||||
resetSettings();
|
||||
registry = new ConfiguredModelRegistry();
|
||||
});
|
||||
|
||||
it("add() mints id, stamps configuredAt, appends row", async () => {
|
||||
const before = Date.now();
|
||||
const id = await registry.add({
|
||||
providerId: PROVIDER_A,
|
||||
info: info("claude-sonnet-4-5", "Claude Sonnet 4.5"),
|
||||
});
|
||||
expect(typeof id).toBe("string");
|
||||
const row = registry.get(id)!;
|
||||
expect(row.providerId).toBe(PROVIDER_A);
|
||||
expect(row.info.id).toBe("claude-sonnet-4-5");
|
||||
expect(row.configuredAt).toBeGreaterThanOrEqual(before);
|
||||
});
|
||||
|
||||
it("add() enforces (providerId, info.id) uniqueness", async () => {
|
||||
await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
await expect(
|
||||
registry.add({ providerId: PROVIDER_A, info: info("m1", "different label") })
|
||||
).rejects.toThrow(/already configured/);
|
||||
// Same wire id under a different provider is allowed.
|
||||
await expect(registry.add({ providerId: PROVIDER_B, info: info("m1") })).resolves.toBeDefined();
|
||||
});
|
||||
|
||||
it("update() merges info patch and rejects unknown id", async () => {
|
||||
const id = await registry.add({
|
||||
providerId: PROVIDER_A,
|
||||
info: info("m1", "Original"),
|
||||
});
|
||||
await registry.update(id, { info: { displayName: "Renamed" } });
|
||||
expect(registry.get(id)!.info.displayName).toBe("Renamed");
|
||||
// Wire id unchanged.
|
||||
expect(registry.get(id)!.info.id).toBe("m1");
|
||||
|
||||
await expect(registry.update("nope", { info: { displayName: "x" } })).rejects.toThrow(
|
||||
/unknown/
|
||||
);
|
||||
});
|
||||
|
||||
it("remove() drops the row; idempotent for unknown id", async () => {
|
||||
const id = await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
await registry.remove(id);
|
||||
expect(registry.get(id)).toBeUndefined();
|
||||
// Idempotent: removing the same id twice is a no-op.
|
||||
await expect(registry.remove(id)).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("removeByProvider() only touches rows under the target provider", async () => {
|
||||
const a1 = await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
const a2 = await registry.add({ providerId: PROVIDER_A, info: info("m2") });
|
||||
const b1 = await registry.add({ providerId: PROVIDER_B, info: info("m1") });
|
||||
await registry.removeByProvider(PROVIDER_A);
|
||||
expect(registry.get(a1)).toBeUndefined();
|
||||
expect(registry.get(a2)).toBeUndefined();
|
||||
expect(registry.get(b1)).toBeDefined();
|
||||
});
|
||||
|
||||
it("listByProvider() returns stable references when settings unchanged", async () => {
|
||||
await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
await registry.add({ providerId: PROVIDER_A, info: info("m2") });
|
||||
const r1 = registry.listByProvider(PROVIDER_A);
|
||||
const r2 = registry.listByProvider(PROVIDER_A);
|
||||
expect(r1).toBe(r2);
|
||||
expect(r1.length).toBe(2);
|
||||
|
||||
// An empty filtered view returns the shared empty array.
|
||||
const e1 = registry.listByProvider(PROVIDER_B);
|
||||
const e2 = registry.listByProvider(PROVIDER_B);
|
||||
expect(e1).toBe(e2);
|
||||
expect(e1.length).toBe(0);
|
||||
});
|
||||
|
||||
it("list() reuses a stable empty reference when no rows are configured", () => {
|
||||
const empty1 = registry.list();
|
||||
const empty2 = registry.list();
|
||||
expect(empty1).toBe(empty2);
|
||||
expect(empty1.length).toBe(0);
|
||||
});
|
||||
|
||||
it("getByWireId() resolves under the right provider only", async () => {
|
||||
const a1 = await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
await registry.add({ providerId: PROVIDER_B, info: info("m1") });
|
||||
expect(registry.getByWireId(PROVIDER_A, "m1")?.configuredModelId).toBe(a1);
|
||||
expect(registry.getByWireId(PROVIDER_A, "missing")).toBeUndefined();
|
||||
});
|
||||
|
||||
it("bulkSet() preserves configuredModelId for existing (providerId, info.id) matches", async () => {
|
||||
const reusedInfo = info("m1", "First");
|
||||
const m1Id = await registry.add({ providerId: PROVIDER_A, info: reusedInfo });
|
||||
const m2Id = await registry.add({ providerId: PROVIDER_A, info: info("m2", "Second") });
|
||||
const m1Row = registry.get(m1Id)!;
|
||||
const m2Row = registry.get(m2Id)!;
|
||||
const m1ConfiguredAt = m1Row.configuredAt;
|
||||
|
||||
// Re-set with m1 reused (same info object), m2 dropped, m3 added.
|
||||
const result = await registry.bulkSet(PROVIDER_A, [reusedInfo, info("m3", "Third")]);
|
||||
|
||||
// m1 preserved -> same id; m3 minted -> new id.
|
||||
expect(result[0]).toBe(m1Id);
|
||||
expect(result[1]).not.toBe(m2Id);
|
||||
expect(registry.get(m2Id)).toBeUndefined();
|
||||
// Same `info` object passed back -> row reference reused.
|
||||
expect(registry.get(m1Id)).toBe(m1Row);
|
||||
expect(registry.get(m1Id)!.configuredAt).toBe(m1ConfiguredAt);
|
||||
// Sanity: untouched reference for m2 is no longer in the list.
|
||||
expect(registry.list()).not.toContain(m2Row);
|
||||
});
|
||||
|
||||
it("bulkSet() refreshes info for reused rows while keeping configuredModelId stable", async () => {
|
||||
const m1Id = await registry.add({ providerId: PROVIDER_A, info: info("m1", "Old name") });
|
||||
const m1ConfiguredAt = registry.get(m1Id)!.configuredAt;
|
||||
|
||||
// Caller passes a fresh `info` with updated displayName (e.g. catalog refresh).
|
||||
const result = await registry.bulkSet(PROVIDER_A, [info("m1", "New name")]);
|
||||
|
||||
// configuredModelId + configuredAt preserved.
|
||||
expect(result[0]).toBe(m1Id);
|
||||
expect(registry.get(m1Id)!.configuredAt).toBe(m1ConfiguredAt);
|
||||
// But the refreshed info lands.
|
||||
expect(registry.get(m1Id)!.info.displayName).toBe("New name");
|
||||
});
|
||||
|
||||
it("bulkSet() reuses the row reference when the fresh info is structurally equal", async () => {
|
||||
const m1Id = await registry.add({ providerId: PROVIDER_A, info: info("m1", "Same name") });
|
||||
const m1Row = registry.get(m1Id)!;
|
||||
|
||||
// Simulate a catalog refresh: caller builds a fresh `info` object
|
||||
// with byte-identical content but a different reference. The row
|
||||
// reference must NOT churn — downstream React/Jotai memoization
|
||||
// counts on row identity remaining stable for no-op refreshes.
|
||||
const result = await registry.bulkSet(PROVIDER_A, [info("m1", "Same name")]);
|
||||
|
||||
expect(result[0]).toBe(m1Id);
|
||||
expect(registry.get(m1Id)).toBe(m1Row);
|
||||
});
|
||||
|
||||
it("bulkSet() silently dedupes duplicate info.id entries in the input", async () => {
|
||||
// Two distinct info objects with the same wire id — must collapse to
|
||||
// one row to preserve the (providerId, info.id) uniqueness invariant
|
||||
// that `add()` enforces.
|
||||
const result = await registry.bulkSet(PROVIDER_A, [info("dup"), info("dup", "shadowed")]);
|
||||
expect(result).toHaveLength(1);
|
||||
const rows = registry.listByProvider(PROVIDER_A);
|
||||
expect(rows).toHaveLength(1);
|
||||
expect(rows[0].configuredModelId).toBe(result[0]);
|
||||
// First occurrence wins; later duplicates are dropped.
|
||||
expect(rows[0].info.displayName).toBe("dup");
|
||||
});
|
||||
|
||||
it("bulkSet() does not touch rows belonging to other providers", async () => {
|
||||
const a1 = await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
const b1 = await registry.add({ providerId: PROVIDER_B, info: info("m1") });
|
||||
await registry.bulkSet(PROVIDER_A, [info("m1"), info("m2")]);
|
||||
// Provider B's row untouched.
|
||||
expect(registry.get(b1)).toBeDefined();
|
||||
// Provider A's m1 reused.
|
||||
expect(registry.getByWireId(PROVIDER_A, "m1")?.configuredModelId).toBe(a1);
|
||||
});
|
||||
|
||||
it("settings reflect mutations atomically", async () => {
|
||||
const id = await registry.add({ providerId: PROVIDER_A, info: info("m1") });
|
||||
expect(getSettings().configuredModels.find((m) => m.configuredModelId === id)).toBeDefined();
|
||||
await registry.remove(id);
|
||||
expect(getSettings().configuredModels.find((m) => m.configuredModelId === id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,18 +1,45 @@
|
|||
/**
|
||||
* Source of truth for `ConfiguredModel` rows.
|
||||
*
|
||||
* Wraps `settings.configuredModels: ConfiguredModel[]` (added by the
|
||||
* settings-wiring follow-up PR) with typed reads and mutations.
|
||||
* Uniqueness invariant: `(providerId, info.id)` — enforced on `add()`
|
||||
* and `bulkSet()` writes.
|
||||
* Wraps `settings.configuredModels: ConfiguredModel[]` with typed reads
|
||||
* and mutations. Uniqueness invariant: `(providerId, info.id)` —
|
||||
* enforced on `add()` and implicit in `bulkSet()` writes.
|
||||
*
|
||||
* React components consume reactive reads through the atoms in
|
||||
* `state/atoms.ts`. This class is for mutations and non-React callers.
|
||||
*
|
||||
* Referential stability — read methods cache their result keyed on the
|
||||
* source-slice reference (`getSettings().configuredModels`). On a cache
|
||||
* hit (slice unchanged since last call) the same array reference is
|
||||
* returned. `bulkSet` preserves `configuredModelId` + `configuredAt`
|
||||
* when `(providerId, info.id)` matches an existing row (so external
|
||||
* refs like `BackendConfig.enabledModels` don't churn), and reuses the
|
||||
* row reference verbatim only when the same `info` object is passed
|
||||
* back in. See AGENTS.md → "Referential stability".
|
||||
*/
|
||||
|
||||
import { v4 as uuidv4 } from "uuid";
|
||||
|
||||
import { getSettings, setSettings } from "@/settings/model";
|
||||
import { frozenOr, sliceMemoByKey } from "@/utils/sliceCache";
|
||||
|
||||
import type { ModelInfo } from "@/modelManagement/types/catalog";
|
||||
import type { ConfiguredModel } from "@/modelManagement/types/persisted";
|
||||
|
||||
// Frozen empty shared across all reads (both `list()` and filtered
|
||||
// views) so consumers see a stable reference for the zero case.
|
||||
const EMPTY_LIST: readonly ConfiguredModel[] = Object.freeze([]);
|
||||
|
||||
export class ConfiguredModelRegistry {
|
||||
// Per-provider cache keyed on the source-slice reference; see
|
||||
// `@/utils/sliceCache` for the underlying pattern.
|
||||
readonly #byProvider = sliceMemoByKey((source: readonly ConfiguredModel[], providerId: string) =>
|
||||
frozenOr(
|
||||
source.filter((m) => m.providerId === providerId),
|
||||
EMPTY_LIST
|
||||
)
|
||||
);
|
||||
|
||||
/**
|
||||
* No constructor args — `settings.configuredModels` is read/written
|
||||
* through the module-level helpers in `@/settings/model`, not
|
||||
|
|
@ -25,22 +52,25 @@ export class ConfiguredModelRegistry {
|
|||
// -------------------------------------------------------------------------
|
||||
|
||||
list(): readonly ConfiguredModel[] {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.list not implemented yet");
|
||||
const source = getSettings().configuredModels;
|
||||
return source.length === 0 ? EMPTY_LIST : source;
|
||||
}
|
||||
|
||||
listByProvider(providerId: string): readonly ConfiguredModel[] {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.listByProvider not implemented yet");
|
||||
return this.#byProvider(getSettings().configuredModels, providerId);
|
||||
}
|
||||
|
||||
get(configuredModelId: string): ConfiguredModel | undefined {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.get not implemented yet");
|
||||
return getSettings().configuredModels.find((m) => m.configuredModelId === configuredModelId);
|
||||
}
|
||||
|
||||
/** Resolve by the wire-form id under a specific provider. Used when
|
||||
* reconciling external references — e.g. an agent picker rehydrating
|
||||
* `<providerId>/<wireId>` strings into rows. */
|
||||
getByWireId(providerId: string, wireModelId: string): ConfiguredModel | undefined {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.getByWireId not implemented yet");
|
||||
return getSettings().configuredModels.find(
|
||||
(m) => m.providerId === providerId && m.info.id === wireModelId
|
||||
);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
@ -49,22 +79,56 @@ export class ConfiguredModelRegistry {
|
|||
|
||||
/** Mints `configuredModelId`, stamps `configuredAt`. Throws if the
|
||||
* `(providerId, info.id)` pair already exists. */
|
||||
add(input: Omit<ConfiguredModel, "configuredModelId" | "configuredAt">): Promise<string> {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.add not implemented yet");
|
||||
async add(input: Omit<ConfiguredModel, "configuredModelId" | "configuredAt">): Promise<string> {
|
||||
const existing = getSettings().configuredModels;
|
||||
if (existing.some((m) => m.providerId === input.providerId && m.info.id === input.info.id)) {
|
||||
throw new Error(
|
||||
`[modelManagement] ConfiguredModelRegistry.add: ` +
|
||||
`model "${input.info.id}" already configured for providerId ${input.providerId}`
|
||||
);
|
||||
}
|
||||
const configuredModelId = uuidv4();
|
||||
const row: ConfiguredModel = {
|
||||
...input,
|
||||
configuredModelId,
|
||||
configuredAt: Date.now(),
|
||||
};
|
||||
setSettings((cur) => ({
|
||||
configuredModels: [...cur.configuredModels, row],
|
||||
}));
|
||||
return configuredModelId;
|
||||
}
|
||||
|
||||
/** Patches `info` only — id / providerId / configuredAt are
|
||||
* immutable. Used by `CopilotPlusSetupApi` to refresh model
|
||||
* metadata when Plus updates context limits / pricing. */
|
||||
update(
|
||||
configuredModelId: string,
|
||||
patch: { info?: Partial<ConfiguredModel["info"]> }
|
||||
): Promise<void> {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.update not implemented yet");
|
||||
async update(configuredModelId: string, patch: { info?: Partial<ModelInfo> }): Promise<void> {
|
||||
const existing = getSettings().configuredModels.find(
|
||||
(m) => m.configuredModelId === configuredModelId
|
||||
);
|
||||
if (!existing) {
|
||||
throw new Error(
|
||||
`[modelManagement] ConfiguredModelRegistry.update: unknown configuredModelId ${configuredModelId}`
|
||||
);
|
||||
}
|
||||
if (!patch.info) return;
|
||||
const next: ConfiguredModel = {
|
||||
...existing,
|
||||
info: { ...existing.info, ...patch.info },
|
||||
};
|
||||
setSettings((cur) => ({
|
||||
configuredModels: cur.configuredModels.map((m) =>
|
||||
m.configuredModelId === configuredModelId ? next : m
|
||||
),
|
||||
}));
|
||||
}
|
||||
|
||||
remove(configuredModelId: string): Promise<void> {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.remove not implemented yet");
|
||||
async remove(configuredModelId: string): Promise<void> {
|
||||
setSettings((cur) => ({
|
||||
configuredModels: cur.configuredModels.filter(
|
||||
(m) => m.configuredModelId !== configuredModelId
|
||||
),
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -75,15 +139,77 @@ export class ConfiguredModelRegistry {
|
|||
* `BackendConfig.enabledModels` refs don't churn. Returns the
|
||||
* resulting `configuredModelId`s in input order.
|
||||
*/
|
||||
bulkSet(providerId: string, infos: readonly ConfiguredModel["info"][]): Promise<string[]> {
|
||||
throw new Error("[modelManagement] ConfiguredModelRegistry.bulkSet not implemented yet");
|
||||
async bulkSet(providerId: string, infos: readonly ModelInfo[]): Promise<string[]> {
|
||||
const current = getSettings().configuredModels;
|
||||
const existingForProvider = new Map<string, ConfiguredModel>();
|
||||
for (const m of current) {
|
||||
if (m.providerId === providerId) existingForProvider.set(m.info.id, m);
|
||||
}
|
||||
|
||||
const resultIds: string[] = [];
|
||||
const reusedOrNew: ConfiguredModel[] = [];
|
||||
// Dedupe within the input: callers must not pass duplicate `info.id`
|
||||
// entries (the `(providerId, info.id)` invariant `add()` enforces).
|
||||
// Silently drop later duplicates rather than emit rows that violate
|
||||
// it.
|
||||
const seenInfoIds = new Set<string>();
|
||||
const now = Date.now();
|
||||
for (const info of infos) {
|
||||
if (seenInfoIds.has(info.id)) continue;
|
||||
seenInfoIds.add(info.id);
|
||||
const reused = existingForProvider.get(info.id);
|
||||
if (reused) {
|
||||
// Preserve `configuredModelId` + `configuredAt` so external refs
|
||||
// (BackendConfig.enabledModels) don't churn, but adopt the
|
||||
// caller-supplied `info` — they may have passed refreshed
|
||||
// catalog metadata (displayName, context limits, pricing).
|
||||
// Reuse the row reference verbatim when the persisted info is
|
||||
// structurally equal to the incoming info, so a catalog refresh
|
||||
// that rebuilds equal-but-fresh info objects doesn't churn the
|
||||
// row identity for every downstream memoization site.
|
||||
reusedOrNew.push(isSameInfo(reused.info, info) ? reused : { ...reused, info });
|
||||
resultIds.push(reused.configuredModelId);
|
||||
} else {
|
||||
const configuredModelId = uuidv4();
|
||||
const row: ConfiguredModel = {
|
||||
configuredModelId,
|
||||
providerId,
|
||||
info,
|
||||
configuredAt: now,
|
||||
};
|
||||
reusedOrNew.push(row);
|
||||
resultIds.push(configuredModelId);
|
||||
}
|
||||
}
|
||||
|
||||
setSettings((cur) => ({
|
||||
configuredModels: [
|
||||
...cur.configuredModels.filter((m) => m.providerId !== providerId),
|
||||
...reusedOrNew,
|
||||
],
|
||||
}));
|
||||
return resultIds;
|
||||
}
|
||||
|
||||
/** Used by `ModelManagementCoordinator.removeProvider` to drop all
|
||||
* models under a provider during cascade. */
|
||||
removeByProvider(providerId: string): Promise<void> {
|
||||
throw new Error(
|
||||
"[modelManagement] ConfiguredModelRegistry.removeByProvider not implemented yet"
|
||||
);
|
||||
async removeByProvider(providerId: string): Promise<void> {
|
||||
setSettings((cur) => ({
|
||||
configuredModels: cur.configuredModels.filter((m) => m.providerId !== providerId),
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Structural equality for `ModelInfo`. Cheap stringify is sufficient
|
||||
* here — the type is a small bag of primitives plus a few flat nested
|
||||
* objects (`modalities`, `limits`, `cost`) whose key order is stable
|
||||
* across both the catalog fetcher (writes them in a fixed order) and
|
||||
* Plus-side refresh paths. A `JSON.stringify` mismatch on equal content
|
||||
* costs at most a fresh row spread; correctness is preserved either
|
||||
* way.
|
||||
*/
|
||||
function isSameInfo(a: ModelInfo, b: ModelInfo): boolean {
|
||||
if (a === b) return true;
|
||||
return JSON.stringify(a) === JSON.stringify(b);
|
||||
}
|
||||
|
|
|
|||
272
src/modelManagement/providers/ProviderRegistry.test.ts
Normal file
272
src/modelManagement/providers/ProviderRegistry.test.ts
Normal file
|
|
@ -0,0 +1,272 @@
|
|||
/**
|
||||
* Tests for `ProviderRegistry`.
|
||||
*
|
||||
* The keychain is mocked via a fake `SecretStorage` mounted on a fake
|
||||
* `App.secretStorage`. The settings store is real (via
|
||||
* `resetSettings` / `setSettings`).
|
||||
*/
|
||||
|
||||
import { resetSettings, getSettings } from "@/settings/model";
|
||||
import { KeychainService } from "@/services/keychainService";
|
||||
|
||||
import type { ProviderAdapter } from "./adapters/ProviderAdapter";
|
||||
import { ProviderAdapterRegistry } from "./adapters/ProviderAdapterRegistry";
|
||||
import { ProviderRegistry } from "./ProviderRegistry";
|
||||
|
||||
import type { App } from "obsidian";
|
||||
import { z } from "zod";
|
||||
|
||||
jest.mock("@/logger", () => ({
|
||||
logInfo: jest.fn(),
|
||||
logWarn: jest.fn(),
|
||||
logError: jest.fn(),
|
||||
}));
|
||||
|
||||
type SecretStore = Map<string, string>;
|
||||
|
||||
function makeFakeApp(): { app: App; secrets: SecretStore } {
|
||||
const secrets: SecretStore = new Map();
|
||||
const app = {
|
||||
secretStorage: {
|
||||
setSecret: (id: string, value: string) => {
|
||||
secrets.set(id, value);
|
||||
},
|
||||
getSecret: (id: string) => (secrets.has(id) ? secrets.get(id)! : null),
|
||||
listSecrets: () => Array.from(secrets.keys()),
|
||||
deleteSecret: (id: string) => {
|
||||
secrets.delete(id);
|
||||
},
|
||||
},
|
||||
vault: {
|
||||
// FileSystemAdapter shape is irrelevant for this test — vaultId
|
||||
// resolution path falls into the random branch and never touches
|
||||
// adapter methods after the first generation.
|
||||
adapter: {},
|
||||
},
|
||||
} as unknown as App;
|
||||
return { app, secrets };
|
||||
}
|
||||
|
||||
const anthropicStub: ProviderAdapter = {
|
||||
providerType: "anthropic",
|
||||
extrasSchema: z.object({}).strict(),
|
||||
buildLangChainClient: () => {
|
||||
throw new Error("not used in test");
|
||||
},
|
||||
verifyCredentials: async () => ({
|
||||
ok: true,
|
||||
message: "stub-ok",
|
||||
checkedAt: 42,
|
||||
}),
|
||||
};
|
||||
|
||||
describe("ProviderRegistry", () => {
|
||||
let app: App;
|
||||
let adapters: ProviderAdapterRegistry;
|
||||
let registry: ProviderRegistry;
|
||||
|
||||
beforeEach(() => {
|
||||
resetSettings();
|
||||
KeychainService.resetInstance();
|
||||
const fake = makeFakeApp();
|
||||
app = fake.app;
|
||||
// Eager init so subsequent KeychainService.getInstance() calls inside
|
||||
// the registry hit the same singleton.
|
||||
KeychainService.getInstance(app);
|
||||
adapters = new ProviderAdapterRegistry();
|
||||
adapters.register(anthropicStub);
|
||||
registry = new ProviderRegistry(app, adapters);
|
||||
});
|
||||
|
||||
it("add() mints id, stamps addedAt, persists the row", async () => {
|
||||
const before = Date.now();
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "Anthropic (prod)",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
expect(typeof id).toBe("string");
|
||||
expect(id.length).toBeGreaterThan(0);
|
||||
|
||||
const row = registry.get(id);
|
||||
expect(row).toBeDefined();
|
||||
expect(row?.displayName).toBe("Anthropic (prod)");
|
||||
expect(row?.providerType).toBe("anthropic");
|
||||
expect(row?.origin).toEqual({ kind: "byok" });
|
||||
expect(row?.addedAt).toBeGreaterThanOrEqual(before);
|
||||
expect(row?.apiKeyKeychainId).toBeNull();
|
||||
});
|
||||
|
||||
it("list() / listByOrigin / listByProviderType return stable references when settings unchanged", async () => {
|
||||
await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "B",
|
||||
origin: { kind: "agent", agentType: "claude-code" },
|
||||
});
|
||||
const list1 = registry.list();
|
||||
const list2 = registry.list();
|
||||
expect(list1).toBe(list2);
|
||||
|
||||
const byok1 = registry.listByOrigin("byok");
|
||||
const byok2 = registry.listByOrigin("byok");
|
||||
expect(byok1).toBe(byok2);
|
||||
expect(byok1.length).toBe(1);
|
||||
|
||||
const ant1 = registry.listByProviderType("anthropic");
|
||||
const ant2 = registry.listByProviderType("anthropic");
|
||||
expect(ant1).toBe(ant2);
|
||||
expect(ant1.length).toBe(2);
|
||||
});
|
||||
|
||||
it("empty filtered views reuse a shared frozen empty array", () => {
|
||||
const empty1 = registry.listByOrigin("byok");
|
||||
const empty2 = registry.listByOrigin("copilot-plus");
|
||||
expect(empty1).toBe(empty2);
|
||||
expect(empty1.length).toBe(0);
|
||||
});
|
||||
|
||||
it("update() merges patch and refuses to mutate providerId / addedAt / providerType / origin", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "Original",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
const originalAddedAt = registry.get(id)!.addedAt;
|
||||
|
||||
// Bypass the typed Omit to verify the runtime guard strips immutable
|
||||
// fields even when callers shove them in via an untyped object.
|
||||
// providerType is the adapter-dispatch key and origin determines
|
||||
// which settings tab owns the row — both must stay pinned to the
|
||||
// values supplied at creation.
|
||||
await registry.update(id, {
|
||||
displayName: "Renamed",
|
||||
baseUrl: "https://example.test",
|
||||
...({
|
||||
providerId: "hacked",
|
||||
addedAt: 1,
|
||||
providerType: "openai",
|
||||
origin: { kind: "agent", agentType: "claude-code" },
|
||||
} as Record<string, unknown>),
|
||||
});
|
||||
const row = registry.get(id)!;
|
||||
expect(row.displayName).toBe("Renamed");
|
||||
expect(row.baseUrl).toBe("https://example.test");
|
||||
expect(row.providerId).toBe(id);
|
||||
expect(row.addedAt).toBe(originalAddedAt);
|
||||
expect(row.providerType).toBe("anthropic");
|
||||
expect(row.origin).toEqual({ kind: "byok" });
|
||||
});
|
||||
|
||||
it("update() throws for unknown providerId", async () => {
|
||||
await expect(registry.update("nope", { displayName: "x" })).rejects.toThrow(/unknown/);
|
||||
});
|
||||
|
||||
it("setApiKey mints apiKeyKeychainId on first call and reuses it on rotation", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
expect(registry.get(id)!.apiKeyKeychainId).toBeNull();
|
||||
|
||||
await registry.setApiKey(id, "sk-first");
|
||||
const firstKeychainId = registry.get(id)!.apiKeyKeychainId;
|
||||
const vaultId = KeychainService.getInstance(app).getVaultId();
|
||||
// Vault-namespaced so `KeychainService.clearAllVaultSecrets()` (which
|
||||
// filters by `copilot-v{vaultId}-`) sweeps these entries.
|
||||
expect(firstKeychainId).toBe(`copilot-v${vaultId}-provider-${id}`);
|
||||
expect(await registry.getApiKey(id)).toBe("sk-first");
|
||||
|
||||
await registry.setApiKey(id, "sk-rotated");
|
||||
expect(registry.get(id)!.apiKeyKeychainId).toBe(firstKeychainId);
|
||||
expect(await registry.getApiKey(id)).toBe("sk-rotated");
|
||||
});
|
||||
|
||||
it("update() ignores attempts to overwrite apiKeyKeychainId", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
await registry.setApiKey(id, "sk-real");
|
||||
const realKeychainId = registry.get(id)!.apiKeyKeychainId;
|
||||
expect(realKeychainId).not.toBeNull();
|
||||
|
||||
// Bypass the typed Omit to verify the runtime strip refuses to move
|
||||
// the keychain pointer (which would orphan the secret or repoint the
|
||||
// row at a keychain entry this registry never wrote).
|
||||
await registry.update(id, {
|
||||
...({ apiKeyKeychainId: "copilot-v0-provider-attacker" } as Record<string, unknown>),
|
||||
});
|
||||
expect(registry.get(id)!.apiKeyKeychainId).toBe(realKeychainId);
|
||||
// The real secret is still readable.
|
||||
expect(await registry.getApiKey(id)).toBe("sk-real");
|
||||
});
|
||||
|
||||
it("getApiKey returns null when the provider has no apiKeyKeychainId", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "Ollama-like",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
expect(await registry.getApiKey(id)).toBeNull();
|
||||
});
|
||||
|
||||
it("clearApiKey drops the keychain entry and clears the pointer", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
await registry.setApiKey(id, "sk-x");
|
||||
await registry.clearApiKey(id);
|
||||
expect(registry.get(id)!.apiKeyKeychainId).toBeNull();
|
||||
expect(await registry.getApiKey(id)).toBeNull();
|
||||
});
|
||||
|
||||
it("remove() drops the row and the keychain entry", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
await registry.setApiKey(id, "sk-x");
|
||||
const keychainId = registry.get(id)!.apiKeyKeychainId!;
|
||||
await registry.remove(id);
|
||||
expect(registry.get(id)).toBeUndefined();
|
||||
// Verify keychain side cleaned up by reading raw storage.
|
||||
expect(KeychainService.getInstance(app).getSecretById(keychainId)).toBeNull();
|
||||
});
|
||||
|
||||
it("verify() dispatches to the adapter for the row's providerType", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
await registry.setApiKey(id, "sk-x");
|
||||
const result = await registry.verify(id);
|
||||
expect(result.ok).toBe(true);
|
||||
expect(result.message).toBe("stub-ok");
|
||||
});
|
||||
|
||||
it("verify() throws for unknown providerId", async () => {
|
||||
await expect(registry.verify("nope")).rejects.toThrow(/unknown/);
|
||||
});
|
||||
|
||||
it("settings reflect mutations atomically", async () => {
|
||||
const id = await registry.add({
|
||||
providerType: "anthropic",
|
||||
displayName: "A",
|
||||
origin: { kind: "byok" },
|
||||
});
|
||||
expect(getSettings().providers[id]).toBeDefined();
|
||||
await registry.remove(id);
|
||||
expect(getSettings().providers[id]).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
@ -1,33 +1,77 @@
|
|||
/**
|
||||
* Source of truth for `Provider` rows.
|
||||
*
|
||||
* Wraps `settings.providers: Record<providerId, Provider>` (added by
|
||||
* the settings-wiring follow-up PR) with typed reads, mutations, and
|
||||
* keychain bridging. React components consume reactive reads through
|
||||
* the atoms in `state/atoms.ts`; this class is for mutations and for
|
||||
* non-React callers (the chat-model factory, the setup APIs, the
|
||||
* coordinator).
|
||||
* Wraps `settings.providers: Record<providerId, Provider>` with typed
|
||||
* reads, mutations, and keychain bridging. React components consume
|
||||
* reactive reads through the atoms in `state/atoms.ts`; this class is
|
||||
* for mutations and for non-React callers (the chat-model factory, the
|
||||
* setup APIs, the coordinator).
|
||||
*
|
||||
* Cascade semantics — `remove()` does NOT cascade to ConfiguredModels
|
||||
* or BackendConfigs on its own. Call `ModelManagementCoordinator.removeProvider`
|
||||
* from UI code; the coordinator orchestrates the cross-slice
|
||||
* removal. This method exists for the coordinator's use.
|
||||
*
|
||||
* Referential stability — read methods cache their result keyed on the
|
||||
* source-slice reference (`getSettings().providers`). On a cache hit
|
||||
* (slice unchanged since last call) the same array reference is
|
||||
* returned, which is what Jotai derived atoms and React memoization
|
||||
* rely on. See AGENTS.md → "Referential stability".
|
||||
*/
|
||||
|
||||
import type { App } from "obsidian";
|
||||
import { v4 as uuidv4 } from "uuid";
|
||||
|
||||
import { logError } from "@/logger";
|
||||
import { KeychainService } from "@/services/keychainService";
|
||||
import { getSettings, setSettings } from "@/settings/model";
|
||||
import { frozenOr, sliceMemo, sliceMemoByKey } from "@/utils/sliceCache";
|
||||
|
||||
import type { ProviderType } from "@/modelManagement/types/catalog";
|
||||
import type { Provider, ProviderOrigin } from "@/modelManagement/types/persisted";
|
||||
import type { VerificationResult } from "@/modelManagement/types/runtime";
|
||||
import type { ProviderAdapterRegistry } from "./adapters/ProviderAdapterRegistry";
|
||||
|
||||
// Frozen empty shared across all filtered views so consumers see a
|
||||
// stable reference even when two distinct filters both yield zero rows.
|
||||
const EMPTY_LIST: readonly Provider[] = Object.freeze([]);
|
||||
|
||||
/** Format the keychain id for a given providerId.
|
||||
*
|
||||
* Reason: vault-namespaced (`copilot-v{vaultId}-...`) so the entry is
|
||||
* picked up by `KeychainService.clearAllVaultSecrets()`, which scopes
|
||||
* cleanup by that exact prefix. A flat `copilot-provider-{id}` id would
|
||||
* silently leak past "Delete All Keys" and vault uninstall. */
|
||||
function providerKeychainId(vaultId: string, providerId: string): string {
|
||||
return `copilot-v${vaultId}-provider-${providerId}`;
|
||||
}
|
||||
|
||||
export class ProviderRegistry {
|
||||
/**
|
||||
* Constructor signature is final. Placeholder doesn't store deps
|
||||
* because all method bodies throw; implementer adds private fields
|
||||
* (`#app`, `#adapters`) when the implementation lands.
|
||||
*/
|
||||
constructor(app: App, adapters: ProviderAdapterRegistry) {}
|
||||
readonly #app: App;
|
||||
readonly #adapters: ProviderAdapterRegistry;
|
||||
|
||||
readonly #list = sliceMemo((source: Record<string, Provider>) =>
|
||||
frozenOr(Object.values(source), EMPTY_LIST)
|
||||
);
|
||||
readonly #byOrigin = sliceMemoByKey(
|
||||
(source: Record<string, Provider>, kind: ProviderOrigin["kind"]) =>
|
||||
frozenOr(
|
||||
Object.values(source).filter((p) => p.origin.kind === kind),
|
||||
EMPTY_LIST
|
||||
)
|
||||
);
|
||||
readonly #byType = sliceMemoByKey(
|
||||
(source: Record<string, Provider>, providerType: ProviderType) =>
|
||||
frozenOr(
|
||||
Object.values(source).filter((p) => p.providerType === providerType),
|
||||
EMPTY_LIST
|
||||
)
|
||||
);
|
||||
|
||||
constructor(app: App, adapters: ProviderAdapterRegistry) {
|
||||
this.#app = app;
|
||||
this.#adapters = adapters;
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Reads — synchronous, backed by `settings.providers`.
|
||||
|
|
@ -36,28 +80,28 @@ export class ProviderRegistry {
|
|||
/** All providers. Use the atoms in `state/atoms.ts` for reactive
|
||||
* React reads; this method is for non-React callers. */
|
||||
list(): readonly Provider[] {
|
||||
throw new Error("[modelManagement] ProviderRegistry.list not implemented yet");
|
||||
return this.#list(getSettings().providers);
|
||||
}
|
||||
|
||||
get(providerId: string): Provider | undefined {
|
||||
throw new Error("[modelManagement] ProviderRegistry.get not implemented yet");
|
||||
return getSettings().providers[providerId];
|
||||
}
|
||||
|
||||
/** Filter helper used by the BYOK tab (origin = "byok"), the agent
|
||||
* setup flows (origin = "agent"), and the Plus sign-in handler
|
||||
* (origin = "copilot-plus"). */
|
||||
listByOrigin(originKind: ProviderOrigin["kind"]): readonly Provider[] {
|
||||
throw new Error("[modelManagement] ProviderRegistry.listByOrigin not implemented yet");
|
||||
return this.#byOrigin(getSettings().providers, originKind);
|
||||
}
|
||||
|
||||
/** Used by the agent-setup idempotency check (one
|
||||
* `(agentType, providerType)` row at most). */
|
||||
listByProviderType(providerType: ProviderType): readonly Provider[] {
|
||||
throw new Error("[modelManagement] ProviderRegistry.listByProviderType not implemented yet");
|
||||
return this.#byType(getSettings().providers, providerType);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Mutations — persist via `updateSetting("providers", …)`.
|
||||
// Mutations — persist via `setSettings` updater form.
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
|
|
@ -68,16 +112,80 @@ export class ProviderRegistry {
|
|||
* shape so callers cannot create a row whose pointer references a
|
||||
* keychain entry this code path never wrote.
|
||||
*/
|
||||
add(input: Omit<Provider, "providerId" | "addedAt" | "apiKeyKeychainId">): Promise<string> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.add not implemented yet");
|
||||
async add(input: Omit<Provider, "providerId" | "addedAt" | "apiKeyKeychainId">): Promise<string> {
|
||||
const providerId = uuidv4();
|
||||
const row: Provider = {
|
||||
...input,
|
||||
providerId,
|
||||
addedAt: Date.now(),
|
||||
apiKeyKeychainId: null,
|
||||
};
|
||||
setSettings((cur) => ({
|
||||
providers: { ...cur.providers, [providerId]: row },
|
||||
}));
|
||||
return providerId;
|
||||
}
|
||||
|
||||
/** Partial update. `providerId` and `addedAt` are immutable. */
|
||||
update(
|
||||
/** Partial update. The following fields are immutable through this
|
||||
* entry point:
|
||||
* - `providerId` / `addedAt`: identity & creation time.
|
||||
* - `apiKeyKeychainId`: owned by `setApiKey` / `clearApiKey`; moving
|
||||
* it via a generic patch would orphan keychain entries or
|
||||
* repoint the row at a secret this registry never wrote.
|
||||
* - `providerType`: the single dispatch field — changing it would
|
||||
* leave the row's keychain entry and `extras` payload (whose
|
||||
* shape is `providerType`-specific) pointing at a different
|
||||
* adapter than the one that originally wrote them.
|
||||
* - `origin`: the BYOK / agent / Plus discriminator — changing it
|
||||
* silently moves the row between settings tabs and lifecycle
|
||||
* owners.
|
||||
* Create a new provider (and re-add models / re-enter the key) if any
|
||||
* of these need to change. */
|
||||
async update(
|
||||
providerId: string,
|
||||
patch: Partial<Omit<Provider, "providerId" | "addedAt">>
|
||||
patch: Partial<
|
||||
Omit<Provider, "providerId" | "addedAt" | "apiKeyKeychainId" | "providerType" | "origin">
|
||||
>
|
||||
): Promise<void> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.update not implemented yet");
|
||||
const existing = getSettings().providers[providerId];
|
||||
if (!existing) {
|
||||
throw new Error(
|
||||
`[modelManagement] ProviderRegistry.update: unknown providerId ${providerId}`
|
||||
);
|
||||
}
|
||||
// Defensive: strip immutable fields if any leaked in at runtime
|
||||
// (TypeScript's Omit covers callers using the typed shape).
|
||||
const safePatch = { ...patch } as Record<string, unknown>;
|
||||
delete safePatch.providerId;
|
||||
delete safePatch.addedAt;
|
||||
delete safePatch.apiKeyKeychainId;
|
||||
delete safePatch.providerType;
|
||||
delete safePatch.origin;
|
||||
if (Object.keys(safePatch).length === 0) return;
|
||||
const next: Provider = { ...existing, ...(safePatch as Partial<Provider>) };
|
||||
setSettings((cur) => ({
|
||||
providers: { ...cur.providers, [providerId]: next },
|
||||
}));
|
||||
}
|
||||
|
||||
/** Internal: writes `apiKeyKeychainId` on the row. Bypasses the public
|
||||
* `update()` strip so only the keychain-bridge methods in this class
|
||||
* can move the pointer. */
|
||||
#setApiKeyKeychainId(providerId: string, apiKeyKeychainId: string | null): void {
|
||||
// Read outside the updater so a row that's been concurrently
|
||||
// removed (or already carries the same pointer) skips the
|
||||
// setSettings call entirely — avoids broadcasting a fresh settings
|
||||
// reference to every subscriber for a no-op write.
|
||||
const existing = getSettings().providers[providerId];
|
||||
if (!existing) return;
|
||||
if (existing.apiKeyKeychainId === apiKeyKeychainId) return;
|
||||
setSettings((cur) => {
|
||||
const current = cur.providers[providerId];
|
||||
if (!current) return {};
|
||||
return {
|
||||
providers: { ...cur.providers, [providerId]: { ...current, apiKeyKeychainId } },
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -85,8 +193,21 @@ export class ProviderRegistry {
|
|||
* entry. Cross-slice cascade (ConfiguredModels + BackendConfig refs)
|
||||
* is the coordinator's job — see class docstring.
|
||||
*/
|
||||
remove(providerId: string): Promise<void> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.remove not implemented yet");
|
||||
async remove(providerId: string): Promise<void> {
|
||||
const existing = getSettings().providers[providerId];
|
||||
if (!existing) return;
|
||||
if (existing.apiKeyKeychainId) {
|
||||
try {
|
||||
KeychainService.getInstance(this.#app).deleteSecretById(existing.apiKeyKeychainId);
|
||||
} catch (err) {
|
||||
logError(`[modelManagement] ProviderRegistry.remove: failed to clear keychain`, err);
|
||||
}
|
||||
}
|
||||
setSettings((cur) => {
|
||||
const next = { ...cur.providers };
|
||||
delete next[providerId];
|
||||
return { providers: next };
|
||||
});
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
@ -97,20 +218,48 @@ export class ProviderRegistry {
|
|||
/** Reads the keychain entry referenced by the row's
|
||||
* `apiKeyKeychainId`. Returns `null` for providers that don't take
|
||||
* an API key (Ollama, LMStudio, agent-owned providers). */
|
||||
getApiKey(providerId: string): Promise<string | null> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.getApiKey not implemented yet");
|
||||
async getApiKey(providerId: string): Promise<string | null> {
|
||||
const row = getSettings().providers[providerId];
|
||||
if (!row || !row.apiKeyKeychainId) return null;
|
||||
return KeychainService.getInstance(this.#app).getSecretById(row.apiKeyKeychainId);
|
||||
}
|
||||
|
||||
/** Generates a fresh `apiKeyKeychainId` if the provider doesn't
|
||||
* yet have one; persists the row. Re-calling with a different key
|
||||
* rotates in place (same keychain id, new value). */
|
||||
setApiKey(providerId: string, apiKey: string): Promise<void> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.setApiKey not implemented yet");
|
||||
async setApiKey(providerId: string, apiKey: string): Promise<void> {
|
||||
const row = getSettings().providers[providerId];
|
||||
if (!row) {
|
||||
throw new Error(
|
||||
`[modelManagement] ProviderRegistry.setApiKey: unknown providerId ${providerId}`
|
||||
);
|
||||
}
|
||||
const keychain = KeychainService.getInstance(this.#app);
|
||||
const keychainId =
|
||||
row.apiKeyKeychainId ?? providerKeychainId(keychain.getVaultId(), providerId);
|
||||
// Persist the row's pointer BEFORE writing to the keychain so a
|
||||
// crash (or a keychain write that throws) between the two leaves a
|
||||
// recoverable dangling pointer (empty keychain → getApiKey returns
|
||||
// null; clearApiKey / remove still know which id to clean up)
|
||||
// rather than an orphaned keychain entry that no row points at.
|
||||
if (row.apiKeyKeychainId !== keychainId) {
|
||||
this.#setApiKeyKeychainId(providerId, keychainId);
|
||||
}
|
||||
keychain.setSecretById(keychainId, apiKey);
|
||||
}
|
||||
|
||||
/** Drops the keychain entry and clears `apiKeyKeychainId` on the row. */
|
||||
clearApiKey(providerId: string): Promise<void> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.clearApiKey not implemented yet");
|
||||
async clearApiKey(providerId: string): Promise<void> {
|
||||
const row = getSettings().providers[providerId];
|
||||
if (!row) return;
|
||||
if (row.apiKeyKeychainId) {
|
||||
try {
|
||||
KeychainService.getInstance(this.#app).deleteSecretById(row.apiKeyKeychainId);
|
||||
} catch (err) {
|
||||
logError(`[modelManagement] ProviderRegistry.clearApiKey: failed to delete keychain`, err);
|
||||
}
|
||||
this.#setApiKeyKeychainId(providerId, null);
|
||||
}
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
@ -119,11 +268,20 @@ export class ProviderRegistry {
|
|||
|
||||
/**
|
||||
* Issues an adapter-defined "ping". Returns the verification
|
||||
* result; does NOT persist it. Persistent
|
||||
* `lastVerifiedAt` / `lastVerificationError` fields are deferred
|
||||
* (see data-model spec §10).
|
||||
* result; does NOT persist it.
|
||||
*/
|
||||
verify(providerId: string): Promise<VerificationResult> {
|
||||
throw new Error("[modelManagement] ProviderRegistry.verify not implemented yet");
|
||||
async verify(providerId: string): Promise<VerificationResult> {
|
||||
const provider = this.get(providerId);
|
||||
if (!provider) {
|
||||
throw new Error(
|
||||
`[modelManagement] ProviderRegistry.verify: unknown providerId ${providerId}`
|
||||
);
|
||||
}
|
||||
const apiKey = await this.getApiKey(providerId);
|
||||
return this.#adapters.verifyCredentials(provider.providerType, {
|
||||
provider,
|
||||
apiKey,
|
||||
extras: provider.extras ?? {},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,16 +10,16 @@
|
|||
* subscribers use `settingsStore.sub(<atom>, listener)`.
|
||||
*
|
||||
* The three persisted slices (`providers`, `configuredModels`,
|
||||
* `backends`) are not yet typed fields on `CopilotSettings` — the
|
||||
* settings-wiring follow-up PR adds them. Until then, the local
|
||||
* helper below reads them with fallbacks so the atoms compile and
|
||||
* downstream UI components can bind to them today.
|
||||
* `backends`) live directly on `CopilotSettings` and are backfilled
|
||||
* with frozen empties by `sanitizeSettings` on load — so derived atoms
|
||||
* never observe a fresh `{}` / `[]` and Jotai's `===` short-circuit
|
||||
* holds across reads. See AGENTS.md → "Referential stability".
|
||||
*/
|
||||
|
||||
import { atom } from "jotai";
|
||||
import { atomFamily } from "jotai/utils";
|
||||
|
||||
import { settingsAtom, type CopilotSettings } from "@/settings/model";
|
||||
import { settingsAtom } from "@/settings/model";
|
||||
|
||||
import type {
|
||||
BackendConfig,
|
||||
|
|
@ -30,51 +30,20 @@ import type {
|
|||
} from "@/modelManagement/types/persisted";
|
||||
import type { EnabledBackendEntry } from "@/modelManagement/types/runtime";
|
||||
|
||||
// Stable empty fallbacks. Allocated once so Jotai's `===` short-circuit
|
||||
// holds when the underlying slices are absent (pre-settings-wiring) or
|
||||
// unchanged between settings writes — otherwise every settings write
|
||||
// would invalidate every derived picker atom.
|
||||
const EMPTY_PROVIDERS: Readonly<Record<string, Provider>> = Object.freeze({});
|
||||
const EMPTY_CONFIGURED_MODELS: readonly ConfiguredModel[] = Object.freeze([]);
|
||||
const EMPTY_BACKENDS: Readonly<Partial<Record<BackendType, BackendConfig>>> = Object.freeze({});
|
||||
|
||||
/**
|
||||
* Reads the three persisted slices with fallbacks. Pre-settings-wiring,
|
||||
* `CopilotSettings` doesn't declare these fields, so we widen via the
|
||||
* type intersection below — the cast is intentional and isolated to
|
||||
* this one helper.
|
||||
*/
|
||||
function readSlices(settings: CopilotSettings): {
|
||||
providers: Readonly<Record<string, Provider>>;
|
||||
configuredModels: readonly ConfiguredModel[];
|
||||
backends: Readonly<Partial<Record<BackendType, BackendConfig>>>;
|
||||
} {
|
||||
const widened = settings as CopilotSettings & {
|
||||
providers?: Record<string, Provider>;
|
||||
configuredModels?: ConfiguredModel[];
|
||||
backends?: Partial<Record<BackendType, BackendConfig>>;
|
||||
};
|
||||
return {
|
||||
providers: widened.providers ?? EMPTY_PROVIDERS,
|
||||
configuredModels: widened.configuredModels ?? EMPTY_CONFIGURED_MODELS,
|
||||
backends: widened.backends ?? EMPTY_BACKENDS,
|
||||
};
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// Raw slice atoms — derived directly from settings.
|
||||
// -----------------------------------------------------------------------------
|
||||
|
||||
export const providersAtom = atom<Readonly<Record<string, Provider>>>(
|
||||
(get) => readSlices(get(settingsAtom)).providers
|
||||
(get) => get(settingsAtom).providers
|
||||
);
|
||||
|
||||
export const configuredModelsAtom = atom<readonly ConfiguredModel[]>(
|
||||
(get) => readSlices(get(settingsAtom)).configuredModels
|
||||
(get) => get(settingsAtom).configuredModels
|
||||
);
|
||||
|
||||
export const backendsAtom = atom<Readonly<Partial<Record<BackendType, BackendConfig>>>>(
|
||||
(get) => readSlices(get(settingsAtom)).backends
|
||||
(get) => get(settingsAtom).backends
|
||||
);
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -291,12 +291,27 @@ export class KeychainService {
|
|||
}
|
||||
}
|
||||
|
||||
/** Write a value directly to the keychain using a pre-computed ID. */
|
||||
/** Write a value directly to the keychain using a pre-computed ID.
|
||||
*
|
||||
* CALLER CONTRACT: `keychainId` must be vault-namespaced — typically
|
||||
* `copilot-v{vaultId}-...` — so it is swept by
|
||||
* `clearAllVaultSecrets()` and isolated from other vaults / plugins.
|
||||
* This method is a low-level bridge; it does not validate the
|
||||
* prefix because some legacy callers compute their own ids. New
|
||||
* code should derive the id via `toKeychainId(getVaultId(), …)` or
|
||||
* an equivalent vault-namespaced helper. */
|
||||
setSecretById(keychainId: string, value: string): void {
|
||||
this.storage.setSecret(keychainId, value);
|
||||
}
|
||||
|
||||
/** Delete a keychain entry by its pre-computed ID. */
|
||||
/** Read a value directly from the keychain using a pre-computed ID.
|
||||
* See `setSecretById` for the caller contract on `keychainId`. */
|
||||
getSecretById(keychainId: string): string | null {
|
||||
return this.storage.getSecret(keychainId);
|
||||
}
|
||||
|
||||
/** Delete a keychain entry by its pre-computed ID.
|
||||
* See `setSecretById` for the caller contract on `keychainId`. */
|
||||
deleteSecretById(keychainId: string): void {
|
||||
this.removeSecret(keychainId);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { v4 as uuidv4 } from "uuid";
|
|||
|
||||
import type { ModelSelection } from "@/agentMode";
|
||||
import { type ChainType } from "@/chainType";
|
||||
import type { BackendConfig, BackendType, ConfiguredModel, Provider } from "@/modelManagement";
|
||||
import { type SortStrategy, isSortStrategy } from "@/utils/recentUsageManager";
|
||||
import {
|
||||
AGENT_MAX_ITERATIONS_LIMIT,
|
||||
|
|
@ -275,6 +276,12 @@ export interface CopilotSettings {
|
|||
importSkipList?: string[];
|
||||
};
|
||||
};
|
||||
/**
|
||||
* Model-management persisted slices.
|
||||
*/
|
||||
providers: Record<string, Provider>;
|
||||
configuredModels: ConfiguredModel[];
|
||||
backends: Partial<Record<BackendType, BackendConfig>>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -351,6 +358,13 @@ export interface OpencodeBackendSettings {
|
|||
export const settingsStore = createStore();
|
||||
export const settingsAtom = atom<CopilotSettings>(DEFAULT_SETTINGS);
|
||||
|
||||
/**
|
||||
* Frozen empty fallbacks for the model-management persisted slices.
|
||||
*/
|
||||
const EMPTY_PROVIDERS = Object.freeze({}) as unknown as Record<string, Provider>;
|
||||
const EMPTY_CONFIGURED_MODELS = Object.freeze([]) as unknown as ConfiguredModel[];
|
||||
const EMPTY_BACKENDS = Object.freeze({}) as unknown as Partial<Record<BackendType, BackendConfig>>;
|
||||
|
||||
/**
|
||||
* Resolve a valid embedding model key for the current settings.
|
||||
*
|
||||
|
|
@ -806,6 +820,24 @@ export function sanitizeSettings(settings: CopilotSettings): CopilotSettings {
|
|||
|
||||
sanitizedSettings.agentMode = sanitizeAgentMode(sanitizedSettings.agentMode);
|
||||
|
||||
if (
|
||||
!sanitizedSettings.providers ||
|
||||
typeof sanitizedSettings.providers !== "object" ||
|
||||
Array.isArray(sanitizedSettings.providers)
|
||||
) {
|
||||
sanitizedSettings.providers = EMPTY_PROVIDERS;
|
||||
}
|
||||
if (!Array.isArray(sanitizedSettings.configuredModels)) {
|
||||
sanitizedSettings.configuredModels = EMPTY_CONFIGURED_MODELS;
|
||||
}
|
||||
if (
|
||||
!sanitizedSettings.backends ||
|
||||
typeof sanitizedSettings.backends !== "object" ||
|
||||
Array.isArray(sanitizedSettings.backends)
|
||||
) {
|
||||
sanitizedSettings.backends = EMPTY_BACKENDS;
|
||||
}
|
||||
|
||||
return sanitizedSettings;
|
||||
}
|
||||
|
||||
|
|
|
|||
97
src/utils/sliceCache.test.ts
Normal file
97
src/utils/sliceCache.test.ts
Normal file
|
|
@ -0,0 +1,97 @@
|
|||
import { frozenOr, sliceMemo, sliceMemoByKey } from "./sliceCache";
|
||||
|
||||
describe("sliceMemo", () => {
|
||||
it("returns the same value reference when source reference is unchanged", () => {
|
||||
const compute = jest.fn((src: { rows: number[] }) => src.rows.map((n) => n * 2));
|
||||
const memo = sliceMemo(compute);
|
||||
const source = { rows: [1, 2, 3] };
|
||||
|
||||
const a = memo(source);
|
||||
const b = memo(source);
|
||||
|
||||
expect(a).toBe(b);
|
||||
expect(compute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("recomputes when the source reference changes, even if contents are equal", () => {
|
||||
const compute = jest.fn((src: { rows: number[] }) => [...src.rows]);
|
||||
const memo = sliceMemo(compute);
|
||||
|
||||
const a = memo({ rows: [1, 2, 3] });
|
||||
const b = memo({ rows: [1, 2, 3] });
|
||||
|
||||
expect(a).not.toBe(b);
|
||||
expect(compute).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("sliceMemoByKey", () => {
|
||||
it("caches independently per key and reuses on source-reference match", () => {
|
||||
const compute = jest.fn((src: Record<string, number>, key: string) => src[key] ?? 0);
|
||||
const memo = sliceMemoByKey(compute);
|
||||
const source = { a: 1, b: 2 };
|
||||
|
||||
const a1 = memo(source, "a");
|
||||
const b1 = memo(source, "b");
|
||||
const a2 = memo(source, "a");
|
||||
|
||||
expect(a1).toBe(a2);
|
||||
expect(a1).toBe(1);
|
||||
expect(b1).toBe(2);
|
||||
expect(compute).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("recomputes a key when source reference changes", () => {
|
||||
const compute = jest.fn((src: { rows: number[] }, key: number) => src.rows[key]);
|
||||
const memo = sliceMemoByKey(compute);
|
||||
|
||||
const v1 = memo({ rows: [10, 20] }, 0);
|
||||
const v2 = memo({ rows: [10, 20] }, 0);
|
||||
|
||||
expect(v1).toBe(10);
|
||||
expect(v2).toBe(10);
|
||||
expect(compute).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("invalidates all keys when the source reference flips", () => {
|
||||
const compute = jest.fn((src: Record<string, string[]>, key: string) => src[key] ?? []);
|
||||
const memo = sliceMemoByKey(compute);
|
||||
|
||||
const src1 = { x: ["a"], y: ["b"] };
|
||||
memo(src1, "x");
|
||||
memo(src1, "y");
|
||||
expect(compute).toHaveBeenCalledTimes(2);
|
||||
|
||||
const src2 = { x: ["a"], y: ["b"] };
|
||||
memo(src2, "x");
|
||||
memo(src2, "y");
|
||||
expect(compute).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
});
|
||||
|
||||
describe("frozenOr", () => {
|
||||
it("returns the provided empty constant when input is empty", () => {
|
||||
const EMPTY: readonly number[] = Object.freeze([]);
|
||||
expect(frozenOr([], EMPTY)).toBe(EMPTY);
|
||||
});
|
||||
|
||||
it("returns a frozen copy (not the same reference) when input is non-empty", () => {
|
||||
const EMPTY: readonly number[] = Object.freeze([]);
|
||||
const input = [1, 2, 3];
|
||||
const result = frozenOr(input, EMPTY);
|
||||
|
||||
expect(result).not.toBe(input);
|
||||
expect(result).toEqual([1, 2, 3]);
|
||||
expect(Object.isFrozen(result)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not mutate or freeze the input array", () => {
|
||||
const EMPTY: readonly number[] = Object.freeze([]);
|
||||
const input = [1, 2, 3];
|
||||
frozenOr(input, EMPTY);
|
||||
|
||||
expect(Object.isFrozen(input)).toBe(false);
|
||||
input.push(4);
|
||||
expect(input).toEqual([1, 2, 3, 4]);
|
||||
});
|
||||
});
|
||||
61
src/utils/sliceCache.ts
Normal file
61
src/utils/sliceCache.ts
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
/**
|
||||
* Tiny memoization helpers for derived views over a settings slice.
|
||||
*
|
||||
* The pattern: given a source slice (e.g. `getSettings().providers`)
|
||||
* that callers re-read every access, cache the derived value (a
|
||||
* filtered list, a frozen empty, etc.) keyed on the source-slice
|
||||
* reference. `setSettings` always swaps the slice reference when it
|
||||
* writes, so a `===` match guarantees the underlying data is
|
||||
* unchanged. Returning the same reference on cache hits is what lets
|
||||
* downstream Jotai derived atoms and React memoization short-circuit.
|
||||
*
|
||||
* See AGENTS.md → "Referential stability" for the full rationale.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Memoize a single-arg derivation over a settings slice. Returns the
|
||||
* cached value when the source reference hasn't changed since the
|
||||
* previous call; recomputes otherwise.
|
||||
*/
|
||||
export function sliceMemo<S extends object, V>(compute: (source: S) => V): (source: S) => V {
|
||||
let cache: { source: S; value: V } | null = null;
|
||||
return (source) => {
|
||||
if (cache && cache.source === source) return cache.value;
|
||||
const value = compute(source);
|
||||
cache = { source, value };
|
||||
return value;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Memoize a two-arg derivation `(source, key) → value`. Stores one
|
||||
* cache entry per distinct `key`, each validated against the source
|
||||
* reference at read time.
|
||||
*
|
||||
* Note: the internal `Map` is unbounded — callers are responsible for
|
||||
* ensuring `K` has finite cardinality (an enum, a small fixed set) or
|
||||
* grows at most O(domain size). No LRU.
|
||||
*/
|
||||
export function sliceMemoByKey<S extends object, K, V>(
|
||||
compute: (source: S, key: K) => V
|
||||
): (source: S, key: K) => V {
|
||||
const cache = new Map<K, { source: S; value: V }>();
|
||||
return (source, key) => {
|
||||
const hit = cache.get(key);
|
||||
if (hit && hit.source === source) return hit.value;
|
||||
const value = compute(source, key);
|
||||
cache.set(key, { source, value });
|
||||
return value;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the shared `empty` constant when `arr` is empty; otherwise
|
||||
* a frozen shallow copy of `arr`. The copy step is defensive — callers
|
||||
* commonly pass `someSettingsArr.filter(...)` (already a fresh array
|
||||
* that's safe to freeze), but accepting `readonly T[]` means we can't
|
||||
* assume that, and freezing the settings array itself would be a bug.
|
||||
*/
|
||||
export function frozenOr<T>(arr: readonly T[], empty: readonly T[]): readonly T[] {
|
||||
return arr.length === 0 ? empty : Object.freeze(arr.slice());
|
||||
}
|
||||
Loading…
Reference in a new issue