feat(custom-model): generate Run-menu entries from saved endpoint profiles

[opticon454]

Follow-up to #393. Picks up exactly what @Ark0N invited in the merge comment there:

On your Run-menu idea from Saturday: yes, that is where I would put it too. [...] generate those entries from the saved profiles rather than a fixed duplicate per harness, and put it in a follow-up PR so this one stays the backend.

What this adds

Run menu: a "Custom Endpoints" section generates one entry per (harness that supports customModelInjection, saved endpoint) pair, e.g. "Claude Code (llama.cpp)". The harness list comes from window.__codemanCustomModelClis, injected at page render straight off the CLI registry's own capabilities.customModelInjection (never a hardcoded id list in the frontend), so a CLI whose injection recipe lands later shows up with no frontend change. Picking an entry runs that harness's existing run*() function unmodified (case creation, env overrides, everything — forced to a single instance) and then applies the endpoint's default model to the session it creates, via the existing POST /api/sessions/:id/custom-model route. Entries are hidden entirely for a remote/docker active case, since that route already refuses both.

Settings: App Settings -> Models gets a "Custom model endpoints" group wiring up the customModelEndpointsEnabled toggle (declared in #393, read by nothing until now) plus CRUD against the existing /api/model-endpoints routes: list, add/edit (inline form), delete, discover models.

Backend: CustomModelHost gains an optional defaultModelId, the model the picker applies with no further choice per endpoint (one generated menu entry per CLI+endpoint pair, not per CLI+endpoint+model). The route refuses a value that isn't one of the endpoint's own discovered models, and a fresh discovery drops a default that no longer appears rather than carrying an invalid one forward.

Docs

docs/custom-model-endpoints.md describes the new picker and settings panel. CLAUDE.md's Custom Model Endpoint Profiles entry drops the "backend-only" status note and documents the picker's generation mechanism.

Tests

Four new route tests cover defaultModelId validation, acceptance, and the drop/keep behaviour across a re-discovery. A new render-index-html test pins the __codemanCustomModelClis injection (present, agent CLIs supporting the capability, antigravity and shell excluded) and its solo-window skip.

npm run typecheck, npm run lint, and node scripts/check-frontend-syntax.mjs are all clean. npm test shows no regressions versus master (every failure on my machine is pre-existing Windows-environment noise — missing npx/tmux, EPERM on fs.watch, HEIC tooling — unrelated to this diff; confirmed by diffing the fail list against a clean master checkout).

Known gap: no browser test for the picker or the settings CRUD panel — this box has no tmux, so test:browser/test:mobile couldn't be exercised here. Worth a Playwright pass before merge, same as any other frontend PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RqZeHrRS6DYcGcGX2p9EwG

Wiki

docs/wiki/Custom-Model-Endpoints.md is a new page (auto-synced to the live GitHub wiki on push to master, per docs/wiki/Contributing.md) covering how the feature works end-to-end: turning it on, adding/discovering an endpoint, what picking a Run-menu entry actually does, the per-harness confidence table, and what it deliberately doesn't do yet (remote/Docker sessions, live hot-swap). Linked from the sidebar, Agent-CLIs.md, and Settings-Reference.md.

[Ark0N]

Thanks for picking this up, and for reading the merge comment on #393 as an actual invitation rather than a pleasantry. The shape is right: generating the entries off capabilities.customModelInjection instead of hardcoding a duplicate per harness is exactly what I meant, injecting the capable-CLI list at render so the frontend never carries an id list is better than what I had in mind, and hiding the section for remote and docker cases because the apply route already refuses both is the correct instinct.

It is still a draft so I am reviewing it as one. The backend, the docs and the architecture are sound. The frontend half does not currently work, and I want to be specific rather than vague about it, because none of it needs a redesign. Every item below is small and local.

You were honest up front that no browser test was possible on your box ("this box has no tmux"). That is exactly where the damage landed, so it is worth saying plainly: three of these would have shown up on a single page load.

First, credit where it is due: your last commit (38e1acfe) already fixed what was the third blocker, the red test/server-index-title.test.ts. CI is green on the current head. Three remain.

1. Every generated inline onclick is unparseable, so nothing is clickable (session-ui.js:585, and the three per-row buttons in settings-ui.js).

onclick="app.runCustomModelEntry(${JSON.stringify(cli.id)}, ...)"

JSON.stringify emits double quotes and they sit inside a double-quoted HTML attribute, so the attribute terminates at the first one. Parsed with jsdom the button comes out with onclick="app.runCustomModelEntry(" and the rest of the call shredded into junk attribute names. That does not compile, btn.onclick is null, and a click fires nothing. This hits every entry the picker generates and all three of Discover, Edit and Delete.

The repo already has the right idiom four lines away in the same file: onclick="app.deleteCase(${escapeHtml(JSON.stringify(c.name))})" (session-ui.js:3878, also :3870, :3874, :4050, :4057). escapeHtml turns the quotes into ", which the attribute survives and the JS parser sees as quotes again.

Fix it that way rather than by reordering quotes, because there is a second reason: modelId is the only value in that button reaching HTML unescaped, and it comes from the remote endpoint's own /v1/models response. A model id containing > terminates the <button> early and whatever follows parses as markup. The endpoint is admin-configured so this is not a remote-attacker path, but it is live HTML injection through data the server does not control, and escapeHtml closes it for free.

2. The endpoint list is read as a bare array, but the wire carries the envelope (session-ui.js:571,576 and settings-ui.js:2509,2511). GET /api/model-endpoints returns a bare array from the handler, and then the preSerialization hook in server.ts:769-784 wraps every /api payload that is not already an envelope, arrays included. I replayed that exact hook against a Fastify route returning an array: the wire body is {"success":true,"data":[...]} and Array.isArray(body) is false. So Array.isArray(hosts) is always false in production, the Custom Endpoints section hides itself unconditionally, and _customModelHosts is always [], which means the settings panel permanently reads "No endpoints yet". Even with item 1 fixed the feature is invisible.

const hosts = await this._apiJson('/api/model-endpoints'); fixes it; api-client.js:44 exists for this and unwraps {success,data} already. CLAUDE.md states the rule directly under External CLI modes: "run*() in session-ui.js MUST unwrap the {success,data} envelope; reading the raw shape silently breaks the run."

Worth knowing why no test caught it, since it is not your fault: createRouteTestHarness builds a bare Fastify instance with only the route module and the error handler, no preSerialization hook, so custom-model-routes.test.ts:37's expect(res.json()).toEqual([]) is correct in the harness and wrong on the wire. That is a real gap in the harness and I will look at it separately.

3. A failed launch applies the endpoint to whatever session was already open, and restarts it (session-ui.js:628-651). The comment assumes a failed run*() leaves activeSessionId null. It does not: every run*() handles its own errors and returns normally. runDeepSeek() returns early when dsh is missing or has no pane-capable profile; runClaude() wraps its body in try/catch and ends with _reportSessionLaunchError. In both cases no session was created, and the code then POSTs /api/sessions/<the session the user was already looking at>/custom-model, which points that unrelated session at the endpoint and calls restartCli(), killing the pane and relaunching the CLI. isBusy() blocks it mid-turn, but an idle session, which is most of them, gets silently re-pointed and restarted while the toast says "Pointed at ..., restarting" for a launch that never happened.

Either have the runner hand back the id it created, or snapshot before and require it to have changed:

const before = this.activeSessionId;
await runner();
const sessionId = this.activeSessionId;
if (!sessionId || sessionId === before) return;

The snapshot form is a heuristic (it also declines if a run legitimately re-selects the same session), but declining to apply is the safe side of that trade.

Two majors:

4. It bypasses the Run launch in-flight lock. runCustomModelEntry calls runClaude() and friends directly instead of going through run(), so _runInFlight is never set and #runBtn is never disabled. CLAUDE.md, Run launch synchronization: the lock exists so a double click cannot create duplicate sessions with the same w<n>-<case> name. Closing the menu at the top makes a double click on the entry itself hard to hit, but the lock guards the other direction too: clicking the main Run button while a custom-endpoint launch is still resolving starts a second concurrent launch. Set and clear _runInFlight around the call, or route through run(). Related, same function: mutating #tabCount to '1' and restoring in a finally works, but it visibly flips the user's input for the duration, and if two launches ever overlap (which the missing lock allows) the restore can stomp.

5. No test for any of the new frontend behaviour. Three of the four blockers are DOM-level facts that need no Playwright and no tmux. test/home-sessions.test.ts is the precedent: it loads a frontend module with node:vm against a fake DOM and runs inside the CI gate. A test in that shape over _refreshCustomModelRunOptions, given a fake menu, a stubbed fetch and two endpoints, asserting that a button exists and its onclick attribute parses, would have caught items 1 and 2 on your own machine.

Minors, worth doing while you are in here:

• The hardcoded runners map contradicts the PR's own design. The point of injecting the capable list off the registry is that a CLI whose injection recipe lands later needs no frontend change, but the click handler dispatches through a hardcoded eight-entry object, so such a CLI gets a generated entry that toasts "No run function for mode X". It matches the eight capable CLIs today, so this is latent rather than broken. run() already owns this dispatch.
• Generated entries ignore whether the CLI is installed. _refreshRunModeAvailability hides a stock entry when isCliAvailable(mode) is false; the generated ones are built afterward and never gated, so on a box with no codex the stock Codex entry is hidden while "Codex (llama.cpp)" is still offered and fails at launch. A .filter((cli) => this.isCliAvailable(cli.id)) matches existing behaviour.
• The CRUD panel is not gated on the toggle, but both docs say it is. docs/custom-model-endpoints.md says turning the setting on reveals the panel; nothing reads customModelEndpointsEnabled for visibility, so the list, the Add button and the form always render. Gate it, rather than rewording the docs: with the feature off, the panel is a list of things that do nothing. Also loadCustomModelEndpointsForSettings() is called unconditionally from openAppSettings(), so every settings open fires the GET even with the feature off.
• The design doc still describes the superseded UI. docs/custom-model-endpoints-plan.md section 4 still specifies a separate #customModelBtn toolbar selector, and CLAUDE.md points readers at that file as the design reference. A "superseded by the Run-menu picker" note at the top of that section keeps it honest.
• The API key round-trips through the browser on every edit. GET /api/model-endpoints returns hosts verbatim including apiKey, and the editor re-sends it to implement "blank means unchanged". The comment saying the key is never round-tripped back is true of the input element but not of the request. That store is 0600 precisely because it holds credentials. This is pre-existing route behaviour from feat: Custom Model Endpoint Profiles (local or cloud, all harnesses) #393 rather than something you added, and the exposure is bounded, so it is not a blocker, but the clean fix lives on the server: let PUT treat an absent apiKey as "keep the stored one" and stop returning it on GET. Worth doing while this area is open. Same function: a blank field can only keep a key, never clear one.
• No way to un-point a session. The apply route accepts {clear: true} and the bookkeeping supports it, but no UI reaches it, so the only way back to the native backend is curl or deleting the session. Your docstring knows this; the wiki page does not mention it under "What it does not do".
• The panel offers writes to non-admins in multi-user mode. Endpoint writes are admin-only, but Add/Edit/Delete render for everyone and a non-admin gets a 403 toast. The list is already empty for them, so hiding the controls when the list is empty and the user is not an admin matches how remote and Docker hosts behave.
• Invisible on phones. mobile-overview.js builds its own run picker from mo-mode entries rather than reusing #runModeMenu, so the section does not appear there. Not a regression and not claimed, but the docs should not say "the Run menu" without qualification.

Nits: the index.html comment names _renderCustomModelRunOptions() and the function is _refreshCustomModelRunOptions(); styles.css:16221 uses rgba(0,0,0,0.12) on .set-inline-form, and CLAUDE.md records that hardcoded black alphas turned the settings preview into a grey slab on the light skins, so use a skin token; .run-mode-custom-models gets no CSS so the menu's gap: 2px does not apply between generated entries; server.ts:1609 does JSON.stringify() into a <script> body without escaping </script>, and CliEntry.label is a 60-char string a user's own clis.json could set, so .replace(/</g, '\\u003c') costs nothing (the neighbouring __codemanCliAvailable injection is booleans only, which is why it never needed it); no zh-CN entries for the new settings group; and /api/model-endpoints plus defaultModelId are still absent from docs/api-reference.md.

The backend piece (defaultModelId, refusing a value that is not one of the endpoint's discovered models, dropping it on a fresh discovery) is good and I have no notes on it.

Items 1, 2 and 3 are what I need before this comes out of draft. It is not going into the release I am assembling now, which is fine for a draft. Ping me when it is ready and I will take another pass.