Merge pull request #459 from opticon454/feature/run-menu-picker-currently-loaded-model

feat(custom-model): promote the currently-loaded/last-used model in the Run-menu picker
2026-10-05 06:59:42 +02:00 · 2026-09-21 04:23:14 +02:00
parent a7452dc046 88e5b7b200
commit ecb95b5d67
6 changed files with 339 additions and 14 deletions
@@ -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();
  });
 });

@@ -272,6 +272,146 @@ describe('Custom Model Endpoint Profiles: the "which model" picker', () => {
    expect(win.document.getElementById('customModelPickList')!.textContent).toContain('Default');
  });

+  it('promotes the model llama-swap currently has loaded and ready to the top of the list, tagged', async () => {
+    const { win, app } = bootApp({
+      hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3', 'phi4'] }],
+    });
+    const origApiJson = app._apiJson;
+    app._apiJson = async (path: string) => {
+      if (path === '/api/model-endpoints/llama-box/running-status') {
+        return { isLlamaSwap: true, running: [{ model: 'phi4', state: 'ready' }] };
+      }
+      return origApiJson(path);
+    };
+
+    await app.selectCustomModelEntry('claude', 'llama-box');
+
+    const buttons = [...win.document.getElementById('customModelPickList')!.querySelectorAll('button')];
+    expect(buttons.map((b) => b.textContent)).toHaveLength(3);
+    expect(buttons[0].textContent).toContain('phi4');
+    expect(buttons[0].textContent).toContain('Currently loaded');
+    // Nothing else got relabelled or reordered past the promoted row.
+    expect(buttons[1].textContent).toContain('qwen3');
+    expect(buttons[2].textContent).toContain('llama3');
+  });
+
+  it('falls back to the last model launched on this (harness, endpoint) pair when nothing is currently loaded', async () => {
+    const { win, app } = bootApp({
+      hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3', 'phi4'] }],
+    });
+    const origApiJson = app._apiJson;
+    app._apiJson = async (path: string) => {
+      if (path === '/api/model-endpoints/llama-box/running-status') return { isLlamaSwap: false, running: [] };
+      return origApiJson(path);
+    };
+    // Simulate a prior launch on this exact (harness, endpoint) pair having picked llama3.
+    win.localStorage.setItem('codeman:customModelLastUsed:claude:llama-box', 'llama3');
+
+    await app.selectCustomModelEntry('claude', 'llama-box');
+
+    const buttons = [...win.document.getElementById('customModelPickList')!.querySelectorAll('button')];
+    expect(buttons[0].textContent).toContain('llama3');
+    expect(buttons[0].textContent).toContain('Last used');
+    expect(buttons[0].textContent).not.toContain('Currently loaded');
+  });
+
+  it('prefers the currently-loaded model over a stale "last used" entry when both are present', async () => {
+    const { win, app } = bootApp({
+      hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3', 'phi4'] }],
+    });
+    const origApiJson = app._apiJson;
+    app._apiJson = async (path: string) => {
+      if (path === '/api/model-endpoints/llama-box/running-status') {
+        return { isLlamaSwap: true, running: [{ model: 'phi4', state: 'ready' }] };
+      }
+      return origApiJson(path);
+    };
+    win.localStorage.setItem('codeman:customModelLastUsed:claude:llama-box', 'llama3');
+
+    await app.selectCustomModelEntry('claude', 'llama-box');
+
+    const buttons = [...win.document.getElementById('customModelPickList')!.querySelectorAll('button')];
+    expect(buttons[0].textContent).toContain('phi4');
+    expect(buttons[0].textContent).toContain('Currently loaded');
+  });
+
+  it('is not fooled by a model llama-swap reports loaded but not yet ready, or one this host no longer lists', async () => {
+    const { win, app } = bootApp({
+      hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://x', models: ['qwen3', 'llama3'] }],
+    });
+    const origApiJson = app._apiJson;
+    app._apiJson = async (path: string) => {
+      if (path === '/api/model-endpoints/llama-box/running-status') {
+        // "loading", not "ready" — and a model id this host's own /v1/models no longer serves.
+        return { isLlamaSwap: true, running: [{ model: 'ghost-model', state: 'loading' }] };
+      }
+      return origApiJson(path);
+    };
+
+    await app.selectCustomModelEntry('claude', 'llama-box');
+
+    const list = win.document.getElementById('customModelPickList')!;
+    expect(list.textContent).not.toContain('Currently loaded');
+    expect(list.textContent).not.toContain('ghost-model');
+    const buttons = [...list.querySelectorAll('button')];
+    expect(buttons[0].textContent).toContain('qwen3');
+  });
+
+  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.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');
+
+    expect(win.localStorage.getItem('codeman:customModelLastUsed:claude:llama-box')).toBe('qwen3');
+  });
+
+  it('a slower currently-loaded probe for an earlier pick must never clobber a faster, later pick for a different endpoint', async () => {
+    const { win, app } = bootApp({});
+    const hostA = { id: 'host-a', label: 'Host A', baseUrl: 'http://a', models: ['a1', 'a2'] };
+    const hostB = { id: 'host-b', label: 'Host B', baseUrl: 'http://b', models: ['b1', 'b2'] };
+    let resolveA!: (v: unknown) => void;
+    const pendingA = new Promise((resolve) => {
+      resolveA = resolve;
+    });
+    app._apiJson = async (path: string) => {
+      if (path === '/api/model-endpoints/host-a/running-status') return pendingA;
+      if (path === '/api/model-endpoints/host-b/running-status') return { isLlamaSwap: false, running: [] };
+      return null;
+    };
+
+    // Host A's picker opens first but its probe never resolves until we say so below —
+    // Host B's opens second and resolves immediately, so it renders first.
+    const openA = app._openCustomModelPickModal('claude', hostA);
+    await app._openCustomModelPickModal('claude', hostB);
+
+    expect(win.document.getElementById('customModelPickHint')!.textContent).toContain('Host B');
+    expect(app._pendingCustomModelPick).toEqual({ mode: 'claude', endpointId: 'host-b' });
+
+    // Host A's probe finally answers, after Host B has already rendered.
+    resolveA({ isLlamaSwap: false, running: [] });
+    await openA;
+
+    // The late-arriving Host A response must be a no-op: still Host B on screen.
+    expect(win.document.getElementById('customModelPickHint')!.textContent).toContain('Host B');
+    expect(app._pendingCustomModelPick).toEqual({ mode: 'claude', endpointId: 'host-b' });
+    const listText = win.document.getElementById('customModelPickList')!.textContent;
+    expect(listText).toContain('b1');
+    expect(listText).toContain('b2');
+    expect(listText).not.toContain('a1');
+    expect(listText).not.toContain('a2');
+  });
+
  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'] }],
@@ -333,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({});
@@ -1025,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 },
    ]);
@@ -1043,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;
@@ -1059,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();
  });
 });