Address PR review comments
- Update test assertions for bare model IDs - Add deduplication guard for models with same bare ID - Add validation for empty model IDs after prefix stripping - Add test for API key forwarded as env var to spawn - Add test for deduplication and empty model ID guard - Extract shared handleOpencodeGoApiKeySaved helper - Add changeset for the published package
This commit is contained in:
@@ -9,7 +9,7 @@ vi.mock("node:child_process", () => ({
|
||||
spawn: mockSpawn,
|
||||
}));
|
||||
|
||||
import { parseOpencodeModelsOutput, refreshOpencodeGoModels, syncStartupModels } from "../startup-model-sync.js";
|
||||
import { normalizeOpencodeGoModel, parseOpencodeModelsOutput, refreshOpencodeGoModels, syncStartupModels } from "../startup-model-sync.js";
|
||||
|
||||
type MockProcess = EventEmitter & {
|
||||
stdout: EventEmitter;
|
||||
@@ -75,8 +75,8 @@ describe("startup-model-sync", () => {
|
||||
expect(registerProvider).toHaveBeenCalledWith("openrouter", expect.objectContaining({ models: expect.any(Array) }));
|
||||
expect(registerProvider).toHaveBeenCalledWith("opencode-go", expect.objectContaining({
|
||||
models: expect.arrayContaining([
|
||||
expect.objectContaining({ id: "opencode-go/gpt-5" }),
|
||||
expect.objectContaining({ id: "opencode-go/custom" }),
|
||||
expect.objectContaining({ id: "gpt-5" }),
|
||||
expect.objectContaining({ id: "custom" }),
|
||||
]),
|
||||
}));
|
||||
expect(log).toHaveBeenCalledWith("openrouter", expect.stringContaining("Synced"));
|
||||
@@ -257,7 +257,7 @@ describe("startup-model-sync", () => {
|
||||
|
||||
expect(result).toEqual({ registeredCount: 1 });
|
||||
expect(registerProvider).toHaveBeenCalledWith("opencode-go", expect.objectContaining({
|
||||
models: [expect.objectContaining({ id: "opencode-go/gpt-5" })],
|
||||
models: [expect.objectContaining({ id: "gpt-5" })],
|
||||
}));
|
||||
});
|
||||
|
||||
@@ -319,4 +319,52 @@ describe("startup-model-sync", () => {
|
||||
"opencode-go/custom",
|
||||
]);
|
||||
});
|
||||
|
||||
it("deduplicates models when CLI emits both prefix forms", async () => {
|
||||
mockSpawn.mockImplementation(() => {
|
||||
const proc = createSpawnProcess();
|
||||
queueMicrotask(() => {
|
||||
proc.stdout.emit("data", Buffer.from("opencode/foo\nopencode-go/foo\nopencode/bar\n"));
|
||||
proc.emit("exit", 0);
|
||||
});
|
||||
return proc;
|
||||
});
|
||||
|
||||
const registerProvider = vi.fn();
|
||||
await refreshOpencodeGoModels({ modelRegistry: { registerProvider }, log: vi.fn() });
|
||||
|
||||
expect(registerProvider).toHaveBeenCalledWith("opencode-go", expect.objectContaining({
|
||||
models: [
|
||||
expect.objectContaining({ id: "foo" }),
|
||||
expect.objectContaining({ id: "bar" }),
|
||||
],
|
||||
}));
|
||||
});
|
||||
|
||||
it("throws on empty model ID after prefix stripping", () => {
|
||||
expect(() => normalizeOpencodeGoModel("opencode/")).toThrow("no model name");
|
||||
expect(() => normalizeOpencodeGoModel("opencode-go/")).toThrow("no model name");
|
||||
});
|
||||
|
||||
it("accepts apiKey and passes it as env var to spawn", async () => {
|
||||
mockSpawn.mockImplementation(() => {
|
||||
const proc = createSpawnProcess();
|
||||
queueMicrotask(() => {
|
||||
proc.stdout.emit("data", Buffer.from("opencode/foo\n"));
|
||||
proc.emit("exit", 0);
|
||||
});
|
||||
return proc;
|
||||
});
|
||||
|
||||
const registerProvider = vi.fn();
|
||||
await refreshOpencodeGoModels({ modelRegistry: { registerProvider }, log: vi.fn(), apiKey: "test-key" });
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
"opencode",
|
||||
["models", "opencode", "--refresh"],
|
||||
expect.objectContaining({
|
||||
env: expect.objectContaining({ OPENCODE_API_KEY: "test-key" }),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -71,7 +71,7 @@ import { createReadOnlyAuthFileStorage, mergeAuthStorageReads, wrapAuthStorageWi
|
||||
import { getClaudeCodeCredentialPaths, getCodexCliAuthPath, getFusionAuthPath, getLegacyAuthPaths, getModelRegistryModelsPath, getPackageManagerAgentDir } from "./auth-paths.js";
|
||||
import { resolveProject } from "../project-context.js";
|
||||
import { ensureBundledDependencyGraphPluginInstalled } from "../plugins/bundled-plugin-install.js";
|
||||
import { refreshOpencodeGoModels, syncStartupModels } from "./startup-model-sync.js";
|
||||
import { handleOpencodeGoApiKeySaved, syncStartupModels } from "./startup-model-sync.js";
|
||||
import { ensureCwdProjectRegistered } from "./ensure-project-registered.js";
|
||||
|
||||
const DIAGNOSTIC_INTERVAL_MS = 30 * 60 * 1000; // 30 minutes
|
||||
@@ -720,16 +720,12 @@ export async function runDaemon(opts: DaemonOptions = {}) {
|
||||
if (providerId !== "opencode" && providerId !== "opencode-go") {
|
||||
return undefined;
|
||||
}
|
||||
const settings = await store.getSettings();
|
||||
if (settings.opencodeGoModelSync === false) {
|
||||
return { registeredCount: 0, reason: "disabled-by-settings" };
|
||||
}
|
||||
const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode");
|
||||
return await refreshOpencodeGoModels({
|
||||
return await handleOpencodeGoApiKeySaved(
|
||||
dashboardAuthStorage,
|
||||
store,
|
||||
modelRegistry,
|
||||
log: (scope, message) => console.log(`[${scope}] ${message}`),
|
||||
apiKey: opencodeGoKey,
|
||||
});
|
||||
(scope, message) => console.log(`[${scope}] ${message}`),
|
||||
);
|
||||
},
|
||||
getClaudeCliExtensionStatus: () => {
|
||||
const r = getCachedClaudeCliResolution();
|
||||
|
||||
@@ -83,7 +83,7 @@ import { getCachedUpdateStatus, isUpdateCheckEnabled } from "../update-cache.js"
|
||||
import { resolveSelfExtension } from "./self-extension.js";
|
||||
import { ensureBundledDependencyGraphPluginInstalled, ensureBundledPluginInstalled, isBundledPluginId } from "../plugins/bundled-plugin-install.js";
|
||||
import { registerCustomProviders, reregisterCustomProviders } from "./custom-provider-registry.js";
|
||||
import { refreshOpencodeGoModels, syncStartupModels } from "./startup-model-sync.js";
|
||||
import { handleOpencodeGoApiKeySaved, syncStartupModels } from "./startup-model-sync.js";
|
||||
import { DashboardTUI, DashboardLogSink, isTTYAvailable, type SystemInfo, type GitStatus, type GitCommit, type GitCommitDetail, type GitBranch, type GitWorktree, type FileEntry, type FileReadResult, type TaskStep as TUITaskStep, type TaskLogEntry as TUITaskLogEntry, type TaskDetailData, type TaskEvent } from "./dashboard-tui/index.js";
|
||||
import { DASHBOARD_STARTUP_STATUS, runTuiStartupPrelude } from "./dashboard-startup-chain.js";
|
||||
|
||||
@@ -1760,16 +1760,12 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
if (providerId !== "opencode" && providerId !== "opencode-go") {
|
||||
return undefined;
|
||||
}
|
||||
const settings = await store.getSettings();
|
||||
if (settings.opencodeGoModelSync === false) {
|
||||
return { registeredCount: 0, reason: "disabled-by-settings" };
|
||||
}
|
||||
const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode");
|
||||
return await refreshOpencodeGoModels({
|
||||
return await handleOpencodeGoApiKeySaved(
|
||||
dashboardAuthStorage,
|
||||
store,
|
||||
modelRegistry,
|
||||
log: (scope, message) => logSink.log(message, scope),
|
||||
apiKey: opencodeGoKey,
|
||||
});
|
||||
(scope, message) => logSink.log(message, scope),
|
||||
);
|
||||
},
|
||||
getClaudeCliExtensionStatus: () => {
|
||||
const r = getCachedClaudeCliResolution();
|
||||
@@ -2083,16 +2079,12 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
if (providerId !== "opencode" && providerId !== "opencode-go") {
|
||||
return undefined;
|
||||
}
|
||||
const settings = await store.getSettings();
|
||||
if (settings.opencodeGoModelSync === false) {
|
||||
return { registeredCount: 0, reason: "disabled-by-settings" };
|
||||
}
|
||||
const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode");
|
||||
return await refreshOpencodeGoModels({
|
||||
return await handleOpencodeGoApiKeySaved(
|
||||
dashboardAuthStorage,
|
||||
store,
|
||||
modelRegistry,
|
||||
log: (scope, message) => logSink.log(message, scope),
|
||||
apiKey: opencodeGoKey,
|
||||
});
|
||||
(scope, message) => logSink.log(message, scope),
|
||||
);
|
||||
},
|
||||
getClaudeCliExtensionStatus: () => {
|
||||
const r = getCachedClaudeCliResolution();
|
||||
|
||||
@@ -71,7 +71,7 @@ import {
|
||||
} from "./llama-cpp-extension.js";
|
||||
import { resolveSelfExtension } from "./self-extension.js";
|
||||
import { registerCustomProviders, reregisterCustomProviders } from "./custom-provider-registry.js";
|
||||
import { refreshOpencodeGoModels, syncStartupModels } from "./startup-model-sync.js";
|
||||
import { handleOpencodeGoApiKeySaved, syncStartupModels } from "./startup-model-sync.js";
|
||||
import { ensureBundledDependencyGraphPluginInstalled, ensureBundledPluginInstalled, isBundledPluginId } from "../plugins/bundled-plugin-install.js";
|
||||
import { ensureCwdProjectRegistered } from "./ensure-project-registered.js";
|
||||
|
||||
@@ -827,16 +827,12 @@ export async function runServe(
|
||||
if (providerId !== "opencode" && providerId !== "opencode-go") {
|
||||
return undefined;
|
||||
}
|
||||
const settings = await store.getSettings();
|
||||
if (settings.opencodeGoModelSync === false) {
|
||||
return { registeredCount: 0, reason: "disabled-by-settings" };
|
||||
}
|
||||
const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode");
|
||||
return await refreshOpencodeGoModels({
|
||||
return await handleOpencodeGoApiKeySaved(
|
||||
dashboardAuthStorage,
|
||||
store,
|
||||
modelRegistry,
|
||||
log: (scope, message) => console.log(`[${scope}] ${message}`),
|
||||
apiKey: opencodeGoKey,
|
||||
});
|
||||
(scope, message) => console.log(`[${scope}] ${message}`),
|
||||
);
|
||||
},
|
||||
getClaudeCliExtensionStatus: () => {
|
||||
const r = getCachedClaudeCliResolution();
|
||||
|
||||
@@ -214,6 +214,10 @@ export function normalizeOpencodeGoModel(modelId: string): ModelConfig {
|
||||
? trimmed.slice("opencode/".length)
|
||||
: trimmed;
|
||||
|
||||
if (!bareModel) {
|
||||
throw new Error(`Invalid opencode-go model ID: "${modelId}" has no model name after provider prefix`);
|
||||
}
|
||||
|
||||
return {
|
||||
id: bareModel,
|
||||
name: bareModel,
|
||||
@@ -290,7 +294,15 @@ export async function refreshOpencodeGoModels(options: {
|
||||
return { registeredCount: 0, reason: "no-models-from-cli" };
|
||||
}
|
||||
|
||||
const models = modelIds.map(normalizeOpencodeGoModel);
|
||||
const normalized = modelIds.map(normalizeOpencodeGoModel);
|
||||
// Deduplicate: CLI can emit both "opencode/foo" and "opencode-go/foo"
|
||||
// which normalize to the same bare ID.
|
||||
const seen = new Set<string>();
|
||||
const models = normalized.filter((m) => {
|
||||
if (seen.has(m.id)) return false;
|
||||
seen.add(m.id);
|
||||
return true;
|
||||
});
|
||||
modelRegistry.registerProvider("opencode-go", {
|
||||
baseUrl: "https://api.opencode.ai/v1",
|
||||
apiKey: "OPENCODE_API_KEY",
|
||||
@@ -323,3 +335,23 @@ export async function syncStartupModels(options: StartupSyncOptions): Promise<vo
|
||||
await refreshOpencodeGoModels({ modelRegistry: options.modelRegistry, log: options.log, apiKey: opencodeGoApiKey });
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared handler for the onApiKeySaved callback used by serve, daemon, and
|
||||
* dashboard. Resolves the opencode-go API key from auth storage (falling back
|
||||
* to the "opencode" provider ID) and triggers a model refresh, respecting the
|
||||
* opencodeGoModelSync setting.
|
||||
*/
|
||||
export async function handleOpencodeGoApiKeySaved(
|
||||
dashboardAuthStorage: AuthStorageLike,
|
||||
store: { getSettings: () => Promise<SettingsLike> },
|
||||
modelRegistry: ModelRegistryLike,
|
||||
log: (scope: string, message: string) => void,
|
||||
): Promise<OpencodeGoRefreshResult | undefined> {
|
||||
const settings = await store.getSettings();
|
||||
if (settings.opencodeGoModelSync === false) {
|
||||
return { registeredCount: 0, reason: "disabled-by-settings" };
|
||||
}
|
||||
const opencodeGoKey = await dashboardAuthStorage.getApiKey("opencode-go") ?? await dashboardAuthStorage.getApiKey("opencode");
|
||||
return await refreshOpencodeGoModels({ modelRegistry, log, apiKey: opencodeGoKey });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user