fix(custom-model): address Ark0N's PR review — client-side probe timeout, defer "last used" past confirmation, docs, zh-CN

Four things from the maintainer's review on PR #459, all fixed:

1. Bound _getCustomModelCurrentlyLoaded's probe client-side (~800ms via
   Promise.race, on top of — never instead of — the route's own 5s
   server-side timeout). Without it, an asleep/firewalled endpoint behind
   a saved model list left the picker completely invisible for up to 5s
   after the Run menu had already closed, with no spinner or toast.
   `timeoutMs` is an optional param (default 800, real callers never pass
   it) so a test can drive it in milliseconds, same pattern as
   `_watchLlamaSwapLoading`'s own `pollIntervalMs` — this code runs in a
   JSDOM window's own realm, whose setTimeout vi.useFakeTimers() cannot
   patch.

2. "Last used" is now written only once a launch actually applies, never
   on the mere click. It moved out of runCustomModelEntry (unconditional)
   and into each path's own success point: _quickStartWithCustomModelConfirm
   after the final post succeeds, and _runCustomModelEntryViaRestart right
   after the apply's success check. A context-window-warning decline means
   this exact model cannot work with this CLI at all, so the old
   unconditional write would promote, next time the picker opened, the one
   model guaranteed to fail again.

3. Documented the promotion/tag precedence and the new
   codeman:customModelLastUsed:<mode>:<endpointId> localStorage key in both
   CLAUDE.md's Custom Model Endpoint Profiles section and
   docs/custom-model-endpoints.md's Run-menu picker section.

4. Added zh-CN entries for "Currently loaded" and "Last used" in i18n.js,
   next to this modal's existing "Choose a model"/"Custom Endpoints" pair.

New tests: the client-side timeout (endpoint that never answers, one that
answers within the bound, and a rejected-after-timeout probe settling
quietly), and "last used" recording on success vs. NOT recording on either
confirmation's decline, for both the restart and one-shot paths.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD
This commit is contained in:
Devvyn
2026-09-20 21:19:42 +08:00
co-authored by Claude Sonnet 5
parent 73607663fd
commit 88e5b7b200
6 changed files with 142 additions and 25 deletions
+13 -3
View File
@@ -142,7 +142,7 @@ describe('_quickStartWithCustomModelConfirm', () => {
})) as unknown as typeof fetch;
}
it('returns the response directly when no confirmation is needed', async () => {
it('returns the response directly when no confirmation is needed, and records "last used"', async () => {
const { win, app } = bootApp();
withFetch(win, (body) => ({ success: true, data: { sessionId: 's1', modelSwapInProgress: false, body } }));
const data = await app._quickStartWithCustomModelConfirm({
@@ -152,9 +152,17 @@ describe('_quickStartWithCustomModelConfirm', () => {
expect(data.success).toBe(true);
expect(data.data.sessionId).toBe('s1');
expect(app._lastCustomModelLaunchResult).toEqual(data.data);
expect(win.localStorage.getItem('codeman:customModelLastUsed:codex:e')).toBe('m');
});
it('confirming re-sends with confirmedSwap and returns the second response', async () => {
it('a plain launch with no customModel at all never touches the "last used" key (undefined endpointId/modelId would otherwise silently no-op it)', async () => {
const { win, app } = bootApp();
withFetch(win, () => ({ success: true, data: { sessionId: 's1' } }));
await app._quickStartWithCustomModelConfirm({ mode: 'codex' });
expect(win.localStorage.getItem('codeman:customModelLastUsed:codex:undefined')).toBeNull();
});
it('confirming re-sends with confirmedSwap, returns the second response, and only THEN records "last used"', async () => {
const { win, app } = bootApp();
app._confirmModelSwap = async () => true;
let calls = 0;
@@ -183,9 +191,10 @@ describe('_quickStartWithCustomModelConfirm', () => {
expect(calls).toBe(2);
expect(data.data.sessionId).toBe('s1');
expect(app._lastCustomModelLaunchResult.modelSwapInProgress).toBe(true);
expect(win.localStorage.getItem('codeman:customModelLastUsed:codex:e')).toBe('m');
});
it('cancelling never re-sends, and reports a cancellation error', async () => {
it('cancelling never re-sends, reports a cancellation error, and must NEVER record "last used" for a launch that never happened', async () => {
const { win, app } = bootApp();
app._confirmModelSwap = async () => false;
let calls = 0;
@@ -208,6 +217,7 @@ describe('_quickStartWithCustomModelConfirm', () => {
expect(data.success).toBe(false);
expect(data.error).toMatch(/cancelled/i);
expect(app._lastCustomModelLaunchResult).toBeUndefined();
expect(win.localStorage.getItem('codeman:customModelLastUsed:codex:e')).toBeNull();
});
});
+60 -7
View File
@@ -357,10 +357,19 @@ describe('Custom Model Endpoint Profiles: the "which model" picker', () => {
expect(buttons[0].textContent).toContain('qwen3');
});
it('remembers the launched model as "last used" for this (harness, endpoint) pair', async () => {
it('remembers the launched model as "last used" only once the apply actually succeeds, not on the mere attempt', async () => {
const { win, app } = bootApp({});
app.run = async () => {};
app._api = async () => ({ ok: true, json: async () => ({ success: true, data: {} }) });
app.activeSessionId = 'old-session';
// A real new session, and a real successful apply with no questions asked — the
// restart path only reaches its _setCustomModelLastUsed call past both.
app.run = async () => {
app.activeSessionId = 'new-session';
};
app._api = async () => ({
ok: true,
status: 200,
json: async () => ({ success: true, data: { customModel: { endpointId: 'llama-box' }, restarted: true } }),
});
await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
@@ -464,6 +473,48 @@ describe('Custom Model Endpoint Profiles: the "which model" picker', () => {
});
});
describe('Custom Model Endpoint Profiles: _getCustomModelCurrentlyLoaded is client-side bounded', () => {
// `timeoutMs` driven in milliseconds rather than the real 800 — same reasoning as
// `_watchLlamaSwapLoading`'s own `pollIntervalMs` a few describe blocks down: this
// code runs inside the JSDOM window's own realm, whose setTimeout vi.useFakeTimers()
// does not patch, so this is the only way to test the bound without actually waiting
// on it (or, worse, hanging on a promise that deliberately never resolves).
it('never lets an endpoint that never answers keep the picker waiting past the client-side bound', async () => {
const { app } = bootApp({
hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3'] }],
});
// A `running-status` probe that simply never resolves — the exact shape of an
// endpoint that is asleep or firewalled, distinct from one that answers an error.
app._apiJson = () => new Promise(() => {});
const result = await app._getCustomModelCurrentlyLoaded({ id: 'llama-box', models: ['qwen3'] }, 5);
expect(result).toBeNull();
});
it('an endpoint that answers well within the bound is unaffected by it', async () => {
const { app } = bootApp({});
app._apiJson = async () => ({ isLlamaSwap: true, running: [{ model: 'qwen3', state: 'ready' }] });
const result = await app._getCustomModelCurrentlyLoaded({ id: 'llama-box', models: ['qwen3'] }, 5);
expect(result).toBe('qwen3');
});
it('a rejected probe settles quietly to null rather than leaving an unhandled rejection once the timeout has already won the race', async () => {
const { app } = bootApp({});
app._apiJson = () => new Promise((_resolve, reject) => setTimeout(() => reject(new Error('boom')), 10));
const result = await app._getCustomModelCurrentlyLoaded({ id: 'llama-box', models: ['qwen3'] }, 2);
expect(result).toBeNull();
// Give the loser of the race a turn to actually reject and hit its own .catch —
// an unswallowed rejection here would surface as an "Unhandled Errors" failure
// for the whole test file, not a failed assertion in this test.
await new Promise((resolve) => setTimeout(resolve, 20));
});
});
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({});
@@ -1156,8 +1207,8 @@ describe("Custom Model Endpoint Profiles: requiresContextWarning (this CLI's own
return { win, app, applyBodies };
}
it('confirming the in-app context-warning modal re-sends the apply with confirmed:true', async () => {
const { app, applyBodies } = launchHarness([
it('confirming the in-app context-warning modal re-sends the apply with confirmed:true, and only THEN records "last used"', async () => {
const { win, app, applyBodies } = launchHarness([
{ requiresContextWarning: true, modelId: 'qwen3', contextLength: 16384, minSafeContextTokens: 40000 },
{ customModel: { endpointId: 'llama-box' }, restarted: true, modelSwapInProgress: false },
]);
@@ -1174,10 +1225,11 @@ describe("Custom Model Endpoint Profiles: requiresContextWarning (this CLI's own
{ endpointId: 'llama-box', modelId: 'qwen3' },
{ endpointId: 'llama-box', modelId: 'qwen3', confirmedContext: true },
]);
expect(win.localStorage.getItem('codeman:customModelLastUsed:claude:llama-box')).toBe('qwen3');
});
it('declining the in-app context-warning modal keeps the native backend and never re-sends the apply', async () => {
const { app, applyBodies } = launchHarness([
it('declining the in-app context-warning modal keeps the native backend, never re-sends the apply, and must NEVER record this model as "last used" — it cannot work with this CLI at all', async () => {
const { win, app, applyBodies } = launchHarness([
{ requiresContextWarning: true, modelId: 'qwen3', contextLength: 16384, minSafeContextTokens: 40000 },
]);
app._confirmContextWarning = async () => false;
@@ -1190,6 +1242,7 @@ describe("Custom Model Endpoint Profiles: requiresContextWarning (this CLI's own
expect(applyBodies).toHaveLength(1); // no second (confirmed) call
expect(toastMessage).toMatch(/context window too small/i);
expect(win.localStorage.getItem('codeman:customModelLastUsed:claude:llama-box')).toBeNull();
});
});