mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 14:09:42 +02:00
feat(custom-model): ask which model on launch when an endpoint has more than one, and re-discover models every 5 minutes
Two enhancements requested after live-validating PR #430 against a real llama.cpp server: 1. Model picker dialog. Picking a Run-menu Custom Endpoints entry used to apply the endpoint's defaultModelId (or the first discovered model) silently. Now, via the new selectCustomModelEntry() (session-ui.js): - exactly one discovered model launches straight away, same as before - two or more open a new #customModelPickModal listing every discovered model; defaultModelId (if set) is marked but never auto-chosen, since the point of asking is letting ONE launch deliberately differ from the saved default, not just confirming it The endpoint is re-fetched at click time rather than trusting anything cached from the dropdown's own render, since the model list can have changed (the sweep below, or a settings-panel edit) since it opened. runCustomModelEntry() itself — the actual launch, routed through run() for the in-flight lock, snapshot-guarded against applying to the wrong session — is unchanged; it now just always receives an explicit model id from one of these two paths instead of computing one itself. 2. Periodic re-discovery. Every saved endpoint's models now refresh automatically every 5 minutes in the background (CUSTOM_MODEL_REDISCOVER_INTERVAL_MS, server.ts, registered the same way as the Codex plan-usage poll it sits beside — this.cleanup.setInterval, off under testMode), so a model the server starts or stops serving shows up without another manual "Discover" click. The manual POST .../discover-models route and the new refreshAllCustomModelHosts() sweep (custom-model-routes.ts) now share one pure merge step (applyDiscoveredModels: stamps lastDiscoveredAt, drops a defaultModelId that no longer appears) rather than two copies that could drift. The sweep is best-effort per host — one endpoint being unreachable on a cycle never blocks the others — and re-reads the store before each host's write, keyed by id, so a concurrent edit or delete from the settings panel always wins over a sweep that started before it. Tests: test/custom-model-endpoint-rediscovery.test.ts is a new, dedicated file for the sweep (kept separate from custom-model-routes.test.ts because that file's data dir is shared across every test in it — one temp HOME per FILE, not per test — which would make a sweep-touches-every-host assertion meaningless there). test/custom-model-run-menu-ui.test.ts gained a new describe block driving the real picker modal through JSDOM: single-model bypass, multi-model dialog with the default marked-not-chosen, picking a row closes the modal and launches with that exact model, the endpoint re-fetch, and the two "vanished by click time" toast paths. Docs: docs/custom-model-endpoints.md, docs/wiki/Custom-Model-Endpoints.md, docs/api-reference.md and CLAUDE.md's dense feature paragraph all updated — the last of these also caught up two sentences that had gone stale after the draft-review fixes landed (the picker routes through run() now, not a raw run*() call). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RqZeHrRS6DYcGcGX2p9EwG
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
60e1bd52f7
commit
5a9ff07f57
@@ -0,0 +1,126 @@
|
||||
/**
|
||||
* @fileoverview Tests for `refreshAllCustomModelHosts()`, the periodic
|
||||
* background sweep behind server.ts's "custom model endpoint re-discovery"
|
||||
* timer (docs/custom-model-endpoints-plan.md). Kept in its own file rather
|
||||
* than folded into test/routes/custom-model-routes.test.ts: that file's data
|
||||
* dir is shared across every test in it (one temp HOME per FILE, not per
|
||||
* test — test/setup.ts), and a sweep that walks every saved host would pick
|
||||
* up every host any other test in that file happened to create, making an
|
||||
* exact call-count or exact-host assertion meaningless. A dedicated file
|
||||
* gets its own clean temp HOME.
|
||||
*
|
||||
* Port: N/A (no server; drives readCustomModelHosts/writeCustomModelHosts
|
||||
* directly plus the mocked webviewFetch dispatcher).
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { getDataDir } from '../src/config/instance.js';
|
||||
import { readCustomModelHosts, writeCustomModelHosts, type CustomModelHost } from '../src/custom-model-hosts.js';
|
||||
import { refreshAllCustomModelHosts } from '../src/web/routes/custom-model-routes.js';
|
||||
import { webviewFetch } from '../src/web/webview-egress.js';
|
||||
|
||||
vi.mock('../src/web/webview-egress.js', async () => {
|
||||
const actual = await vi.importActual<typeof import('../src/web/webview-egress.js')>('../src/web/webview-egress.js');
|
||||
return { ...actual, webviewFetch: vi.fn() };
|
||||
});
|
||||
|
||||
const fetchMock = vi.mocked(webviewFetch);
|
||||
|
||||
function host(overrides: Partial<CustomModelHost> & Pick<CustomModelHost, 'id' | 'baseUrl'>): CustomModelHost {
|
||||
return { label: overrides.id, ...overrides };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
fetchMock.mockReset();
|
||||
});
|
||||
|
||||
describe('refreshAllCustomModelHosts (the periodic re-discovery sweep)', () => {
|
||||
it('refreshes every saved endpoint, best-effort — one unreachable host does not stop the others', async () => {
|
||||
const dir = getDataDir();
|
||||
await writeCustomModelHosts(dir, [
|
||||
host({ id: 'ok', baseUrl: 'http://localhost:8080' }),
|
||||
host({ id: 'down', baseUrl: 'http://localhost:8081' }),
|
||||
]);
|
||||
fetchMock.mockImplementation(async (url: URL) => {
|
||||
if (url.href.includes('8081')) throw new TypeError('fetch failed', { cause: new Error('ECONNREFUSED') });
|
||||
return new Response(JSON.stringify({ data: [{ id: 'qwen3' }] }), { status: 200 });
|
||||
});
|
||||
|
||||
await refreshAllCustomModelHosts();
|
||||
|
||||
const hosts = await readCustomModelHosts(dir);
|
||||
const ok = hosts.find((h) => h.id === 'ok');
|
||||
const down = hosts.find((h) => h.id === 'down');
|
||||
expect(ok?.models).toEqual(['qwen3']);
|
||||
expect(ok?.lastDiscoveredAt).toBeTruthy();
|
||||
expect(down?.models ?? []).toEqual([]);
|
||||
expect(down?.lastDiscoveredAt).toBeFalsy();
|
||||
});
|
||||
|
||||
it('skips a host whose baseUrl is blocked, without making a request', async () => {
|
||||
const dir = getDataDir();
|
||||
// Written directly rather than through the POST route, which already
|
||||
// refuses this at save time — this simulates a record that pre-dates the
|
||||
// guard, or was hand-edited on disk. The sweep must not trust it either.
|
||||
await writeCustomModelHosts(dir, [host({ id: 'meta', baseUrl: 'http://169.254.169.254/' })]);
|
||||
|
||||
fetchMock.mockResolvedValue(new Response(JSON.stringify({ data: [{ id: 'x' }] }), { status: 200 }));
|
||||
await refreshAllCustomModelHosts();
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('drops a stale default and preserves lastDiscoveredAt semantics, same as manual discovery', async () => {
|
||||
const dir = getDataDir();
|
||||
await writeCustomModelHosts(dir, [
|
||||
host({ id: 'ep', baseUrl: 'http://localhost:8080', models: ['qwen3'], defaultModelId: 'qwen3' }),
|
||||
]);
|
||||
fetchMock.mockResolvedValue(new Response(JSON.stringify({ data: [{ id: 'llama3' }] }), { status: 200 }));
|
||||
|
||||
await refreshAllCustomModelHosts();
|
||||
|
||||
const [updated] = await readCustomModelHosts(dir);
|
||||
expect(updated.models).toEqual(['llama3']);
|
||||
expect(updated.defaultModelId).toBeUndefined();
|
||||
expect(updated.lastDiscoveredAt).toBeTruthy();
|
||||
});
|
||||
|
||||
it('keeps a default that is still present after the sweep', async () => {
|
||||
const dir = getDataDir();
|
||||
await writeCustomModelHosts(dir, [
|
||||
host({ id: 'ep', baseUrl: 'http://localhost:8080', models: ['qwen3'], defaultModelId: 'qwen3' }),
|
||||
]);
|
||||
fetchMock.mockResolvedValue(
|
||||
new Response(JSON.stringify({ data: [{ id: 'qwen3' }, { id: 'llama3' }] }), { status: 200 })
|
||||
);
|
||||
|
||||
await refreshAllCustomModelHosts();
|
||||
|
||||
const [updated] = await readCustomModelHosts(dir);
|
||||
expect(updated.defaultModelId).toBe('qwen3');
|
||||
});
|
||||
|
||||
it('does not resurrect an endpoint deleted while the sweep was in flight', async () => {
|
||||
const dir = getDataDir();
|
||||
await writeCustomModelHosts(dir, [host({ id: 'deleted', baseUrl: 'http://localhost:8080' })]);
|
||||
|
||||
fetchMock.mockImplementation(async () => {
|
||||
// Simulate an admin deleting the endpoint between the sweep's fetch and
|
||||
// its read-modify-write — the delete must win, not be overwritten by a
|
||||
// refresh that started before it.
|
||||
const current = await readCustomModelHosts(dir);
|
||||
await writeCustomModelHosts(
|
||||
dir,
|
||||
current.filter((h) => h.id !== 'deleted')
|
||||
);
|
||||
return new Response(JSON.stringify({ data: [{ id: 'qwen3' }] }), { status: 200 });
|
||||
});
|
||||
|
||||
await expect(refreshAllCustomModelHosts()).resolves.toBeUndefined();
|
||||
const hosts = await readCustomModelHosts(dir);
|
||||
expect(hosts.find((h) => h.id === 'deleted')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('leaves the store untouched when there are no saved endpoints at all', async () => {
|
||||
await expect(refreshAllCustomModelHosts()).resolves.toBeUndefined();
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -49,6 +49,11 @@ function bootApp(
|
||||
<div id="runModeCustomModelHeader" style="display:none"></div>
|
||||
<div id="runModeCustomModels"></div>
|
||||
</div>
|
||||
<div class="modal" id="customModelPickModal">
|
||||
<h3 id="customModelPickTitle"></h3>
|
||||
<p id="customModelPickHint"></p>
|
||||
<div id="customModelPickList"></div>
|
||||
</div>
|
||||
</body>`,
|
||||
{ url: 'http://localhost/', runScripts: 'dangerously' }
|
||||
);
|
||||
@@ -68,6 +73,10 @@ function bootApp(
|
||||
app.loadAppSettingsFromStorage = () => ({ customModelEndpointsEnabled: options.settingsEnabled ?? true });
|
||||
app.isCliAvailable = options.cliAvailable ?? (() => true);
|
||||
app.showToast = () => {};
|
||||
// Default no-op so a button's onclick (selectCustomModelEntry -> possibly
|
||||
// straight to runCustomModelEntry for a single-model host) never rejects
|
||||
// with "this.run is not a function"; tests of the launch itself override it.
|
||||
app.run = async () => {};
|
||||
// _apiJson unwraps the {success,data} envelope for real against a live
|
||||
// server; here it stands in for that, driven from a fixed `hosts` fixture
|
||||
// so these tests exercise the picker's OWN code, not the envelope helper.
|
||||
@@ -172,6 +181,122 @@ describe('Custom Model Endpoint Profiles: Run-menu picker generation', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Custom Model Endpoint Profiles: the "which model" picker', () => {
|
||||
it('launches straight away for a host with exactly one discovered model, no dialog', async () => {
|
||||
const { win, app } = bootApp({
|
||||
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3'] }],
|
||||
});
|
||||
let launched: unknown[] | null = null;
|
||||
app.runCustomModelEntry = async (...args: unknown[]) => {
|
||||
launched = args;
|
||||
};
|
||||
|
||||
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||
|
||||
expect(launched).toEqual(['claude', 'llama-box', 'qwen3']);
|
||||
expect(win.document.getElementById('customModelPickModal')!.classList.contains('active')).toBe(false);
|
||||
});
|
||||
|
||||
it('opens the picker for a host with more than one discovered model, rather than launching directly', async () => {
|
||||
const { win, app } = bootApp({
|
||||
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3', 'llama3'] }],
|
||||
});
|
||||
let launched = false;
|
||||
app.runCustomModelEntry = async () => {
|
||||
launched = true;
|
||||
};
|
||||
|
||||
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||
|
||||
expect(launched).toBe(false);
|
||||
const modal = win.document.getElementById('customModelPickModal')!;
|
||||
expect(modal.classList.contains('active')).toBe(true);
|
||||
const list = win.document.getElementById('customModelPickList')!;
|
||||
expect(list.querySelectorAll('button').length).toBe(2);
|
||||
expect(list.textContent).toContain('qwen3');
|
||||
expect(list.textContent).toContain('llama3');
|
||||
});
|
||||
|
||||
it('always asks with 2+ models, even when a defaultModelId is set — the point is letting this launch differ', async () => {
|
||||
const { win, app } = bootApp({
|
||||
hosts: [
|
||||
{
|
||||
id: 'llama-box',
|
||||
label: 'llama.cpp',
|
||||
baseUrl: 'http://localhost:8080',
|
||||
models: ['qwen3', 'llama3'],
|
||||
defaultModelId: 'qwen3',
|
||||
},
|
||||
],
|
||||
});
|
||||
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||
const modal = win.document.getElementById('customModelPickModal')!;
|
||||
expect(modal.classList.contains('active')).toBe(true);
|
||||
// The default is marked, not auto-chosen.
|
||||
expect(win.document.getElementById('customModelPickList')!.textContent).toContain('Default');
|
||||
});
|
||||
|
||||
it('picking a row in the modal closes it and launches with that exact model', async () => {
|
||||
const { win, app } = bootApp({
|
||||
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3', 'llama3'] }],
|
||||
});
|
||||
let launched: unknown[] | null = null;
|
||||
app.runCustomModelEntry = async (...args: unknown[]) => {
|
||||
launched = args;
|
||||
};
|
||||
win.app = app;
|
||||
|
||||
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||
const buttons = win.document.getElementById('customModelPickList')!.querySelectorAll('button');
|
||||
const llama3Btn = [...buttons].find((b) => b.textContent?.includes('llama3')) as unknown as HTMLButtonElement & {
|
||||
onclick: (e: unknown) => void;
|
||||
};
|
||||
expect(typeof llama3Btn.onclick).toBe('function');
|
||||
llama3Btn.onclick(new (win as any).Event('click'));
|
||||
|
||||
expect(launched).toEqual(['claude', 'llama-box', 'llama3']);
|
||||
expect(win.document.getElementById('customModelPickModal')!.classList.contains('active')).toBe(false);
|
||||
});
|
||||
|
||||
it('re-fetches the endpoint at click time rather than trusting anything cached from the menu render', async () => {
|
||||
// The background re-discovery sweep (server-side, every 5 minutes) or a
|
||||
// settings-panel edit can change the model list between opening the
|
||||
// dropdown and clicking a row — the picker must reflect what is current.
|
||||
let fetchCount = 0;
|
||||
const { win, app } = bootApp({});
|
||||
app._apiJson = async (path: string) => {
|
||||
if (path !== '/api/model-endpoints') return null;
|
||||
fetchCount += 1;
|
||||
return [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3', 'phi4'] }];
|
||||
};
|
||||
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||
expect(fetchCount).toBe(1);
|
||||
expect(win.document.getElementById('customModelPickList')!.querySelectorAll('button').length).toBe(3);
|
||||
});
|
||||
|
||||
it('toasts and does nothing when the endpoint has vanished by click time', async () => {
|
||||
const { app } = bootApp({ hosts: [] });
|
||||
let toastMessage: string | null = null;
|
||||
app.showToast = (msg: string) => {
|
||||
toastMessage = msg;
|
||||
};
|
||||
await app.selectCustomModelEntry('claude', 'ghost-endpoint');
|
||||
expect(toastMessage).toMatch(/no longer exists/i);
|
||||
});
|
||||
|
||||
it('toasts and does nothing when the endpoint has zero discovered models by click time', async () => {
|
||||
const { app } = bootApp({
|
||||
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: [] }],
|
||||
});
|
||||
let toastMessage: string | null = null;
|
||||
app.showToast = (msg: string) => {
|
||||
toastMessage = msg;
|
||||
};
|
||||
await app.selectCustomModelEntry('claude', 'llama-box');
|
||||
expect(toastMessage).toMatch(/no models discovered/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Custom Model Endpoint Profiles: applying a picked entry', () => {
|
||||
it('does not apply the endpoint to a session that was already open when the launch fails', async () => {
|
||||
const { app } = bootApp({});
|
||||
|
||||
Reference in New Issue
Block a user