From 60e1bd52f71131f84d2e9662e1ebef2b1a696f4a Mon Sep 17 00:00:00 2001
From: Devvyn <22340871+opticon454@users.noreply.github.com>
Date: Wed, 16 Sep 2026 07:00:55 +0800
Subject: [PATCH] =?UTF-8?q?fix(custom-model):=20act=20on=20the=20draft=20r?=
=?UTF-8?q?eview=20=E2=80=94=20unparseable=20onclick,=20unwrapped=20envelo?=
=?UTF-8?q?pe,=20wrong-session=20apply,=20missing=20lock,=20no=20tests?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Addresses every blocker, both majors, and all but one minor from the
maintainer's review of the draft PR.
Blockers:
1. Every generated inline onclick was unparseable. JSON.stringify's own
double quotes terminated the double-quoted HTML attribute at the first
one, leaving btn.onclick null on every picker entry and every Discover/
Edit/Delete button. Fixed with escapeHtml(JSON.stringify(...)) per
argument, the same idiom deleteCase's onclick already uses four lines
away in session-ui.js. This also closes the live-HTML-injection route
through modelId (server-controlled, from the endpoint's own /v1/models
reply): with quoting intact, a `>` inside it can no longer terminate the
@@ -2216,42 +2216,46 @@
Enable custom model endpointsAdds a per-endpoint entry to the Run menu for every harness that can redirect to one.
-
+
-
-
-
-
Add endpoint
-
-
IdShort, stable — used in URLs, never shown to the CLI.
-
-
-
-
Label
-
-
-
-
Base URL
-
-
-
-
API keyOptional. Left blank on edit keeps the existing key.
-
-
-
-
Auth headerNever send both — some servers hang indefinitely.
-
-
-
-
Default modelWhat the Run-menu picker applies for this endpoint. Discover models first.
-
-
-
-
-
+
+
+
+
+
+
Add endpoint
+
+
IdShort, stable — used in URLs, never shown to the CLI.
+
+
+
+
Label
+
+
+
+
Base URL
+
+
+
+
API keyOptional. Left blank on edit keeps the existing key.
+
+
+
+
Auth headerNever send both — some servers hang indefinitely.
+
+
+
+
Default modelWhat the Run-menu picker applies for this endpoint. Discover models first.
+
+
+
+
+
+
diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js
index de783959..c14dcb3d 100644
--- a/src/web/public/session-ui.js
+++ b/src/web/public/session-ui.js
@@ -559,20 +559,22 @@ Object.assign(CodemanApp.prototype, {
};
const settings = this.loadAppSettingsFromStorage();
- const capableClis = window.__codemanCustomModelClis || [];
+ // Matches _refreshRunModeAvailability's own gate: a stock entry for an
+ // uninstalled CLI is hidden, so a generated one must be too, or a box with
+ // no codex still offers "Codex (llama.cpp)" and fails at launch.
+ const capableClis = (window.__codemanCustomModelClis || []).filter((cli) => this.isCliAvailable(cli.id));
if (!settings.customModelEndpointsEnabled || capableClis.length === 0) return hide();
const caseName = document.getElementById('quickStartCase')?.value;
const activeCase = caseName ? (this.cases || []).find((c) => c.name === caseName) : null;
if (activeCase?.location === 'remote' || activeCase?.location === 'docker') return hide();
- let hosts;
- try {
- const res = await fetch('/api/model-endpoints');
- hosts = await res.json();
- } catch {
- return hide();
- }
+ // GET /api/model-endpoints wraps its body in the { success, data } envelope
+ // like every other /api route (server.ts's preSerialization hook applies to
+ // arrays too) — _apiJson() unwraps it. A raw fetch().json() here would
+ // silently see the envelope object instead of the array and hide this
+ // section unconditionally.
+ const hosts = await this._apiJson('/api/model-endpoints');
if (!Array.isArray(hosts) || hosts.length === 0) return hide();
const rows = [];
@@ -580,9 +582,17 @@ Object.assign(CodemanApp.prototype, {
const modelId = host.defaultModelId || (host.models || [])[0];
if (!modelId) continue; // nothing discovered yet — the settings panel explains why
for (const cli of capableClis) {
+ // escapeHtml(JSON.stringify(...)) on EVERY arg, not just the untrusted
+ // one: JSON.stringify's own double quotes would otherwise terminate this
+ // double-quoted attribute at the first one, and everything after parses
+ // as raw tag content rather than a quoted string — which is what turns
+ // modelId (server-controlled, from the endpoint's own /v1/models reply,
+ // not this box's) into markup instead of inert data. Same idiom as
+ // deleteCase's onclick a few hundred lines down.
+ const args = [cli.id, host.id, modelId].map((v) => escapeHtml(JSON.stringify(v))).join(', ');
rows.push(`
`);
@@ -598,59 +608,57 @@ Object.assign(CodemanApp.prototype, {
* Runs a session on `mode` and immediately applies `endpointId`/`modelId` to it
* via POST /api/sessions/:id/custom-model (see session-routes.ts) — the same
* restart-in-place apply path the (not-yet-built) endpoint-management surface
- * would use for an already-running session. Reuses the existing per-mode run*()
- * functions wholesale (case creation, env overrides, the works) rather than a
- * parallel create path, forcing a single instance: a custom-model run is a
- * one-off "try this endpoint" action, not a batch spawn.
+ * would use for an already-running session. A custom-model run is a one-off
+ * "try this endpoint" action, not a sticky mode.
+ *
+ * Routes through run() itself, via a temporary `_runMode` swap, rather than a
+ * parallel dispatch table: that is what gives this the same in-flight lock
+ * every other Run click gets (CLAUDE.md, Run launch synchronization — the lock
+ * exists so a double click cannot create duplicate sessions with the same
+ * `w-` name, and it guards the OTHER direction too: without it, the
+ * main Run button could start a second concurrent launch while this one was
+ * still resolving), and it means a CLI whose customModelInjection recipe
+ * lands later needs no update here, only in run()'s own dispatch. The swap
+ * never persists — setRunMode() would sync it to the server as the user's new
+ * default, which a one-off endpoint run must not do — and is restored in
+ * `finally` even if run() throws.
*/
async runCustomModelEntry(mode, endpointId, modelId) {
document.getElementById('runModeMenu')?.classList.remove('active');
- const runners = {
- claude: () => this.runClaude(),
- opencode: () => this.runOpenCode(),
- codex: () => this.runCodex(),
- gemini: () => this.runGemini(),
- pi: () => this.runPi(),
- grok: () => this.runGrok(),
- deepseek: () => this.runDeepSeek(),
- omp: () => this.runOmp(),
- };
- const runner = runners[mode];
- if (!runner) {
- this.showToast(`No run function for mode ${mode}`, 'error');
- return;
- }
+ const previousRunMode = this._runMode;
+ const before = this.activeSessionId;
const tabCountEl = document.getElementById('tabCount');
const prevTabCount = tabCountEl?.value;
+ this._runMode = mode;
if (tabCountEl) tabCountEl.value = '1';
try {
- await runner();
+ await this.run();
} finally {
+ this._runMode = previousRunMode;
if (tabCountEl && prevTabCount !== undefined) tabCountEl.value = prevTabCount;
}
- // Every run*() ends by selecting the session it just created, so the active
- // session at this point IS the new one — see runClaude/runShell's own comments
- // on why selectSession must run before this reads activeSessionId.
+ // run() reports its own errors via toast. Every run*() function handles its
+ // own failure internally and returns normally rather than throwing or
+ // leaving activeSessionId null, so a declined/failed launch (missing CLI, a
+ // caught exception, isBusy on the session the launch would have targeted)
+ // falls through to here with the PREVIOUSLY active session still active.
+ // Requiring the id to have actually changed — not just to be non-null — is
+ // what stops that case from silently re-pointing and restarting whatever
+ // session the user was already looking at.
const sessionId = this.activeSessionId;
- if (!sessionId) return; // run() already reported its own error via toast
+ if (!sessionId || sessionId === before) return;
- try {
- const res = await fetch(`/api/sessions/${sessionId}/custom-model`, {
- method: 'POST',
- headers: { 'Content-Type': 'application/json' },
- body: JSON.stringify({ endpointId, modelId }),
- });
- const data = await res.json();
- if (!data.success) {
- this.showToast(`Session started on the native backend — could not apply the custom endpoint: ${data.error}`, 'warning');
- return;
- }
- this.showToast(`Pointed at ${endpointId} — restarting the session...`, 'info');
- } catch (err) {
- this.showToast(`Session started, but applying the custom endpoint failed: ${err.message}`, 'warning');
+ const data = await this._apiJson(`/api/sessions/${sessionId}/custom-model`, {
+ method: 'POST',
+ body: { endpointId, modelId },
+ });
+ if (!data) {
+ this.showToast(`Session started on the native backend — could not apply the custom endpoint`, 'warning');
+ return;
}
+ this.showToast(`Pointed at ${endpointId} — restarting the session...`, 'info');
},
/**
diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js
index 19a2309b..1ade54a4 100644
--- a/src/web/public/settings-ui.js
+++ b/src/web/public/settings-ui.js
@@ -397,8 +397,11 @@ Object.assign(CodemanApp.prototype, {
document.getElementById('appSettingsApprovalsInbox').checked = settings.approvalsInboxEnabled === true;
// Custom Model Endpoint Profiles: synced, default OFF. The toggle governs both
// the Run-menu picker's generated entries and this settings panel's visibility;
- // the endpoint list itself is server state, loaded separately below.
+ // the endpoint list itself is server state, loaded on demand below.
document.getElementById('appSettingsCustomModelEndpoints').checked = settings.customModelEndpointsEnabled === true;
+ // Assigning .checked above does not fire onchange, so the body's visibility
+ // (and its lazy load) needs an explicit sync on every open, not just a save.
+ this.applyCustomModelEndpointsVisibility();
// Read My Mind: synced, default OFF (opt-in; capture + prediction cost real tokens).
document.getElementById('appSettingsReadMyMind').checked = settings.readMyMindEnabled === true;
document.getElementById('appSettingsUltracodeFloatingWindows').checked =
@@ -513,10 +516,9 @@ Object.assign(CodemanApp.prototype, {
document.getElementById('appSettingsNiceValue').value = niceSettings.niceValue ?? 10;
// Model configuration (loaded from server)
this.loadModelConfigForSettings();
- // Custom Model Endpoint Profiles: server state, own load path (mirrors the
- // model-config pair above) rather than the settings payload — endpoints are
- // infra records (CRUD'd via /api/model-endpoints), not user preferences.
- this.loadCustomModelEndpointsForSettings();
+ // Custom Model Endpoint Profiles' own load is gated on the toggle above (see
+ // applyCustomModelEndpointsVisibility) — unlike model config, this GET is
+ // pointless work with the feature off, so it is not fired unconditionally.
// Notification settings
const notifPrefs = this.notificationManager?.preferences || {};
document.getElementById('appSettingsNotifEnabled').checked = notifPrefs.enabled ?? true;
@@ -2507,15 +2509,51 @@ Object.assign(CodemanApp.prototype, {
// toggle itself goes through that path.
// ═══════════════════════════════════════════════════════════════
+ /**
+ * Toggles the endpoint-management body's visibility to match the setting and,
+ * turning it on, lazily loads the endpoint list. Assigning `.checked` (as the
+ * settings load path does) fires no `change` event, so this must be called
+ * explicitly on open as well as wired to the checkbox's own onchange — a
+ * gate that only worked one of those two ways would show a stale "off"
+ * body right after opening, or a stale "on" one right after saving it off.
+ * With the feature off the body is a list of controls that do nothing, so it
+ * is hidden entirely rather than shown disabled.
+ */
+ applyCustomModelEndpointsVisibility() {
+ const enabled = document.getElementById('appSettingsCustomModelEndpoints').checked;
+ const body = document.getElementById('customModelEndpointsBody');
+ if (body) body.style.display = enabled ? '' : 'none';
+ if (enabled) this.loadCustomModelEndpointsForSettings();
+ else this.closeCustomModelHostEditor();
+ this._applyCustomModelAdminGate();
+ },
+
+ /**
+ * Endpoint writes are admin-only in multi-user mode (custom-model-routes.ts),
+ * and GET already answers a non-admin with an empty list, which hides every
+ * per-row Edit/Discover/Delete button on its own. The "+ Add endpoint" button
+ * has no row to hide behind, so it needs its own gate — otherwise a non-admin
+ * can open the form, fill it in, and get a 403 toast on Save. Wired to the
+ * `codeman:me` event (admin-ui.js) as well as called from
+ * applyCustomModelEndpointsVisibility(), because `window.__codemanUser`'s
+ * real role can resolve AFTER settings have already been opened once.
+ */
+ _applyCustomModelAdminGate() {
+ const addBtn = document.getElementById('customModelHostAddBtn');
+ if (!addBtn) return;
+ const me = window.__codemanUser || {};
+ const blocked = me.multiUser && me.role !== 'admin';
+ addBtn.style.display = blocked ? 'none' : '';
+ },
+
async loadCustomModelEndpointsForSettings() {
- try {
- const res = await fetch('/api/model-endpoints');
- const hosts = await res.json();
- this._customModelHosts = Array.isArray(hosts) ? hosts : [];
- } catch (err) {
- console.warn('Failed to load model endpoints:', err);
- this._customModelHosts = this._customModelHosts || [];
- }
+ // GET /api/model-endpoints wraps its body in the { success, data } envelope
+ // like every other /api route (server.ts's preSerialization hook applies to
+ // arrays too) — _apiJson() unwraps it. A raw fetch().json() here would
+ // silently see the envelope object instead of the array and this panel
+ // would read as "No endpoints yet" forever, even with endpoints saved.
+ const hosts = await this._apiJson('/api/model-endpoints');
+ this._customModelHosts = Array.isArray(hosts) ? hosts : [];
this.renderCustomModelHostsList();
},
@@ -2533,6 +2571,14 @@ Object.assign(CodemanApp.prototype, {
const modelSummary = modelCount === 0
? 'No models discovered yet'
: `${modelCount} model${modelCount === 1 ? '' : 's'}${h.defaultModelId ? ` · default: ${escapeHtml(h.defaultModelId)}` : ' · no default set'}`;
+ // escapeHtml(JSON.stringify(h.id)) — not JSON.stringify(h.id) alone —
+ // because JSON.stringify's own double quotes would otherwise terminate
+ // this double-quoted attribute at the first one, and everything after
+ // parses as raw tag content rather than the rest of the quoted string.
+ // Same idiom as deleteCase's onclick in session-ui.js. h.id is
+ // regex-constrained server-side (safe either way) but the pattern must
+ // match everywhere it is used, including where the argument is not.
+ const idArg = escapeHtml(JSON.stringify(h.id));
return `
`;
})
@@ -2558,8 +2604,8 @@ Object.assign(CodemanApp.prototype, {
document.getElementById('customModelHostId').disabled = !!host; // id is immutable once created
document.getElementById('customModelHostLabel').value = host?.label || '';
document.getElementById('customModelHostBaseUrl').value = host?.baseUrl || '';
- document.getElementById('customModelHostApiKey').value = ''; // never round-tripped back into the field
- document.getElementById('customModelHostApiKey').placeholder = host?.apiKey ? '•••••••• (unchanged if left blank)' : '';
+ document.getElementById('customModelHostApiKey').value = ''; // the server never returns the real value (apiKeySet is a bool)
+ document.getElementById('customModelHostApiKey').placeholder = host?.apiKeySet ? '•••••••• (unchanged if left blank)' : '';
document.getElementById('customModelHostAuthStyle').value = host?.authStyle || 'bearer';
this._populateCustomModelDefaultSelect(host);
document.getElementById('customModelHostEditor').style.display = '';
@@ -2592,6 +2638,13 @@ Object.assign(CodemanApp.prototype, {
return;
}
const editing = this._editingCustomModelHostId;
+ // PUT (server-side) treats an absent apiKey as "keep the stored one" — the
+ // browser never holds the real value to resend deliberately unchanged (see
+ // openCustomModelHostEditor and custom-model-routes.ts's applyStoredApiKey),
+ // so a blank field here means omitting the key entirely, not resending
+ // something we do not have. models/lastDiscoveredAt DO still need
+ // re-sending: PUT replaces the whole record, and this cached copy still
+ // carries both (only apiKey is redacted from what GET hands back).
const existing = editing ? (this._customModelHosts || []).find((h) => h.id === editing) : null;
const body = {
id,
@@ -2599,10 +2652,7 @@ Object.assign(CodemanApp.prototype, {
baseUrl,
authStyle,
defaultModelId,
- // A blank key on EDIT means "leave it alone", never "clear it" — the field
- // is never pre-filled with the real value (see openCustomModelHostEditor),
- // so an unedited save must not silently wipe a working credential.
- apiKey: apiKeyInput ? apiKeyInput : existing?.apiKey,
+ apiKey: apiKeyInput || undefined,
models: existing?.models,
lastDiscoveredAt: existing?.lastDiscoveredAt,
};
@@ -3707,3 +3757,15 @@ Object.assign(CodemanApp.prototype, {
this.subagentPanelVisible = false;
},
});
+
+// window.__codemanUser's real role can resolve after settings have already been
+// opened once (admin-ui.js fetches /api/me asynchronously and dispatches this on
+// arrival), so the Custom Model Endpoints admin gate needs to be re-applied when
+// it does, not just when the modal opens. Optional chaining on addEventListener
+// itself: several frontend tests (run-mode-ui.test.ts) load this file into a vm
+// context with a minimal fake `document` that has no event-target methods at
+// all, and a module-level statement that throws there fails the whole file's
+// evaluation, not just this feature.
+document.addEventListener?.('codeman:me', () => {
+ window.app?._applyCustomModelAdminGate?.();
+});
diff --git a/src/web/public/styles.css b/src/web/public/styles.css
index 2fe9fb65..7c5b1cca 100644
--- a/src/web/public/styles.css
+++ b/src/web/public/styles.css
@@ -15134,6 +15134,12 @@ html[data-skin="daylight-blue"] .welcome-btn-tunnel.active:hover {
.run-mode-dot.web { background: #38bdf8; }
.run-mode-webviews { max-height: 180px; overflow-y: auto; }
+/* Custom Model Endpoint Profiles' generated entries: `.run-mode-menu.active`'s
+ own `gap: 2px` only spaces its DIRECT children, and this container (like
+ `.run-mode-webviews` above) is one such child holding several buttons of
+ its own, so it needs the same gap repeated one level down or its rows sit
+ flush against each other. */
+.run-mode-custom-models { display: flex; flex-direction: column; gap: 2px; }
/* A saved URL is a ROW: open on the left, edit + delete on the right, so a URL can
be changed or removed without first opening it as a tab. The side buttons stay
@@ -16209,7 +16215,10 @@ html[data-tab-orientation='vertical'] .home-sessions {
/* Custom Model Endpoint Profiles' inline add/edit form: a nested panel rather
than a modal, so it needs its own border to read as a distinct sub-section
- inside .set-group-body's flat row stack. */
+ inside .set-group-body's flat row stack. `--control-bg` rather than a
+ hardcoded black alpha — CLAUDE.md records that literal fill turning the
+ settings live preview into a grey slab on the light skins, and this panel
+ sits in the very same modal. */
:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-inline-form {
display: flex;
flex-direction: column;
@@ -16218,7 +16227,7 @@ html[data-tab-orientation='vertical'] .home-sessions {
padding: 10px 12px;
border: 1px solid var(--border);
border-radius: 8px;
- background: rgba(0, 0, 0, 0.12);
+ background: var(--control-bg);
}
:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-inline-form h5 {
diff --git a/src/web/routes/custom-model-routes.ts b/src/web/routes/custom-model-routes.ts
index 0fc1b1f2..5519b99e 100644
--- a/src/web/routes/custom-model-routes.ts
+++ b/src/web/routes/custom-model-routes.ts
@@ -48,6 +48,30 @@ function invalidDefaultModel(host: Pick & { apiKeySet: boolean } {
+ const { apiKey, ...rest } = host;
+ return { ...rest, apiKeySet: !!apiKey };
+}
+
+/**
+ * A PUT body with no `apiKey` (or a blank one) means "leave it alone", never
+ * "clear it": the editor never receives the real value to resend deliberately
+ * unchanged (see redactApiKey), so the only way it can tell the two apart is
+ * by omission. There is deliberately no way to CLEAR a key back to unset this
+ * way — a pre-existing limitation, not something this changes.
+ */
+function applyStoredApiKey(incoming: CustomModelHost, existing: CustomModelHost): CustomModelHost {
+ return incoming.apiKey ? incoming : { ...incoming, apiKey: existing.apiKey };
+}
+
async function discoverModels(host: Pick): Promise {
const headers: Record = {};
const apiKey = host.apiKey?.trim();
@@ -81,12 +105,16 @@ function describeFetchError(err: unknown): string {
return message;
}
-export function registerCustomModelRoutes(app: FastifyInstance): void {
- app.get('/api/model-endpoints', async (req) =>
- isMultiUserMode() && !isAdmin(req) ? [] : readCustomModelHosts(CODEMAN_CONFIG_DIR)
- );
+type RedactedHost = ReturnType;
- app.post('/api/model-endpoints', async (req, reply): Promise> => {
+export function registerCustomModelRoutes(app: FastifyInstance): void {
+ app.get('/api/model-endpoints', async (req): Promise => {
+ if (isMultiUserMode() && !isAdmin(req)) return [];
+ const hosts = await readCustomModelHosts(CODEMAN_CONFIG_DIR);
+ return hosts.map(redactApiKey);
+ });
+
+ app.post('/api/model-endpoints', async (req, reply): Promise> => {
const denied = adminOnly(req, reply);
if (denied) return denied;
const host = parseBody(CustomModelHostSchema, req.body);
@@ -100,26 +128,27 @@ export function registerCustomModelRoutes(app: FastifyInstance): void {
return createErrorResponse(ApiErrorCode.ALREADY_EXISTS, 'Model endpoint already exists');
}
await writeCustomModelHosts(CODEMAN_CONFIG_DIR, [...hosts, host]);
- return { success: true, data: { host } };
+ return { success: true, data: { host: redactApiKey(host) } };
});
- app.put('/api/model-endpoints/:id', async (req, reply): Promise> => {
+ app.put('/api/model-endpoints/:id', async (req, reply): Promise> => {
const denied = adminOnly(req, reply);
if (denied) return denied;
const { id } = req.params as { id: string };
- const host = parseBody(CustomModelHostSchema, { ...(req.body as object), id });
- if (isBlockedWebviewUrl(host.baseUrl)) {
+ const incoming = parseBody(CustomModelHostSchema, { ...(req.body as object), id });
+ if (isBlockedWebviewUrl(incoming.baseUrl)) {
return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Endpoint base URL is not allowed');
}
- const badDefault = invalidDefaultModel(host);
+ const badDefault = invalidDefaultModel(incoming);
if (badDefault) return badDefault;
const hosts = await readCustomModelHosts(CODEMAN_CONFIG_DIR);
const index = hosts.findIndex((item) => item.id === id);
if (index === -1) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Model endpoint not found');
+ const host = applyStoredApiKey(incoming, hosts[index]);
const next = [...hosts];
next[index] = host;
await writeCustomModelHosts(CODEMAN_CONFIG_DIR, next);
- return { success: true, data: { host } };
+ return { success: true, data: { host: redactApiKey(host) } };
});
app.delete('/api/model-endpoints/:id', async (req, reply): Promise> => {
diff --git a/src/web/server.ts b/src/web/server.ts
index 54c6b4ab..0852b7f6 100644
--- a/src/web/server.ts
+++ b/src/web/server.ts
@@ -207,6 +207,20 @@ function escapeHtmlText(value: string): string {
return value.replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>');
}
+/**
+ * Escapes a JSON string for safe embedding as the body of an inline `` would otherwise close the tag early
+ * and turn the rest of the document into inert script-body text. Exported so
+ * it unit-tests without constructing a WebServer (which needs a real tmux).
+ */
+export function escapeScriptJson(json: string): string {
+ return json.replace(/ entry.kind === 'agent' && entry.capabilities.customModelInjection.kind !== 'unsupported')
.map((entry) => ({ id: entry.id, label: entry.label }));
+ // Unlike the boolean-only __codemanCliAvailable above, this payload carries
+ // `label`, a string a user's own clis.json can set (CliEntry.label, up to 60
+ // chars) — see escapeScriptJson's own doc comment for why that needs escaping
+ // and __codemanCliAvailable's booleans never did.
+ const customModelClisJson = escapeScriptJson(JSON.stringify(customModelClis));
html = html.replace(
'',
- `\n`
+ `\n`
);
}
if (!soloSessionId && process.env.CODEMAN_GESTURE === '1') {
diff --git a/test/custom-model-run-menu-ui.test.ts b/test/custom-model-run-menu-ui.test.ts
new file mode 100644
index 00000000..0994e00b
--- /dev/null
+++ b/test/custom-model-run-menu-ui.test.ts
@@ -0,0 +1,243 @@
+/**
+ * @fileoverview Frontend tests for the Custom Model Endpoint Profiles Run-menu
+ * picker (docs/custom-model-endpoints-plan.md): the generated entries in
+ * session-ui.js's `_refreshCustomModelRunOptions()` / `runCustomModelEntry()`.
+ *
+ * These are DOM-level facts that need no Playwright and no tmux — `runScripts:
+ * "dangerously"` is used deliberately (this JSDOM only ever parses markup this
+ * module itself generated, never live user input) so that a broken inline
+ * `onclick` attribute shows up as a genuinely uncallable handler, the same way
+ * it would in a real browser, rather than merely as a string this test parses
+ * by eye. `test/admin-ui.test.ts` and `test/home-sessions.test.ts` are the
+ * precedent for driving a real frontend module against a JSDOM window rather
+ * than a live server.
+ *
+ * Port: none.
+ */
+import { readFileSync } from 'node:fs';
+import { JSDOM } from 'jsdom';
+import { describe, expect, it } from 'vitest';
+
+const CONSTANTS_JS = readFileSync(new URL('../src/web/public/constants.js', import.meta.url), 'utf-8');
+const SESSION_UI_JS = readFileSync(new URL('../src/web/public/session-ui.js', import.meta.url), 'utf-8');
+
+function resp(body: unknown, ok = true) {
+ return { ok, json: async () => body };
+}
+
+/**
+ * Boots a minimal CodemanApp instance with constants.js + session-ui.js
+ * evaluated against a real JSDOM window, so escapeHtml and the picker's own
+ * innerHTML-building code run exactly as they do in the browser.
+ */
+function bootApp(
+ options: {
+ customModelClis?: Array<{ id: string; label: string }>;
+ hosts?: unknown;
+ cliAvailable?: (id: string) => boolean;
+ activeCase?: { location?: string } | null;
+ settingsEnabled?: boolean;
+ } = {}
+) {
+ const dom = new JSDOM(
+ `
+
+
+
+
+
+
+
+
+ `,
+ { url: 'http://localhost/', runScripts: 'dangerously' }
+ );
+ const win = dom.window as unknown as Window &
+ typeof globalThis & {
+ CodemanApp: new () => any;
+ __codemanCustomModelClis?: Array<{ id: string; label: string }>;
+ };
+ (win as unknown as { eval: (s: string) => void }).eval('window.CodemanApp = function CodemanApp() {};');
+ (win as unknown as { eval: (s: string) => void }).eval(CONSTANTS_JS);
+ (win as unknown as { eval: (s: string) => void }).eval(SESSION_UI_JS);
+
+ win.__codemanCustomModelClis = options.customModelClis ?? [{ id: 'claude', label: 'Claude Code' }];
+
+ const app = new win.CodemanApp();
+ app.cases = options.activeCase ? [{ name: 'testcase', ...options.activeCase }] : [{ name: 'testcase' }];
+ app.loadAppSettingsFromStorage = () => ({ customModelEndpointsEnabled: options.settingsEnabled ?? true });
+ app.isCliAvailable = options.cliAvailable ?? (() => true);
+ app.showToast = () => {};
+ // _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.
+ app._apiJson = async (path: string) => {
+ if (path === '/api/model-endpoints') return options.hosts ?? [];
+ return null;
+ };
+ return { dom, win, app };
+}
+
+describe('Custom Model Endpoint Profiles: Run-menu picker generation', () => {
+ it('generates a real, clickable button per (capable CLI, endpoint) pair', async () => {
+ const { win, app } = bootApp({
+ hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3'] }],
+ });
+ const menu = win.document.getElementById('runModeMenu')!;
+ await app._refreshCustomModelRunOptions(menu);
+
+ const container = win.document.getElementById('runModeCustomModels')!;
+ const buttons = container.querySelectorAll('button');
+ expect(buttons.length).toBe(1);
+
+ const btn = buttons[0] as unknown as HTMLButtonElement & { onclick: unknown };
+ // The real bug: JSON.stringify's own double quotes terminate the
+ // double-quoted onclick attribute at the first one, so btn.onclick comes
+ // back null and the parsed attribute is garbage. With escapeHtml wrapping
+ // each stringified argument, jsdom (which compiles inline handlers under
+ // runScripts:"dangerously" exactly like a real browser) parses it as a
+ // real, callable function.
+ expect(typeof btn.onclick).toBe('function');
+
+ win.app = app;
+ expect(() => btn.onclick!(new (win as any).Event('click'))).not.toThrow();
+ });
+
+ it('escapes a model id containing HTML-significant characters instead of letting it break out of the tag', async () => {
+ // modelId comes from the endpoint's OWN /v1/models reply, which this box
+ // does not control — a live-HTML-injection vector if it ever reaches the
+ // markup unescaped, distinct from (and on top of) the quoting bug above.
+ const dangerousModel = '">';
+ const { win, app } = bootApp({
+ hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: [dangerousModel] }],
+ });
+ const menu = win.document.getElementById('runModeMenu')!;
+ await app._refreshCustomModelRunOptions(menu);
+
+ const container = win.document.getElementById('runModeCustomModels')!;
+ // The injected markup must never have produced a live element: if it
+ // did, the attacker-controlled tag closed the button early and escaped
+ // into sibling markup instead of staying inert string data.
+ expect(container.querySelector('img')).toBeNull();
+ expect(container.querySelectorAll('button').length).toBe(1);
+ });
+
+ it('is hidden when the feature setting is off, even with capable CLIs and endpoints present', async () => {
+ const { win, app } = bootApp({
+ settingsEnabled: false,
+ hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3'] }],
+ });
+ const menu = win.document.getElementById('runModeMenu')!;
+ await app._refreshCustomModelRunOptions(menu);
+ expect(win.document.getElementById('runModeCustomModels')!.innerHTML).toBe('');
+ expect((win.document.getElementById('runModeCustomModelSep') as HTMLElement).style.display).toBe('none');
+ });
+
+ it('is hidden for a remote or Docker active case, since the apply route refuses both', async () => {
+ for (const location of ['remote', 'docker']) {
+ const { win, app } = bootApp({
+ activeCase: { location },
+ hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3'] }],
+ });
+ const menu = win.document.getElementById('runModeMenu')!;
+ await app._refreshCustomModelRunOptions(menu);
+ expect(win.document.getElementById('runModeCustomModels')!.innerHTML, location).toBe('');
+ }
+ });
+
+ it('skips an endpoint with no discovered model and no default, rather than generating a dead entry', async () => {
+ const { win, app } = bootApp({
+ hosts: [{ id: 'undiscovered', label: 'Not discovered yet', baseUrl: 'http://localhost:8080', models: [] }],
+ });
+ const menu = win.document.getElementById('runModeMenu')!;
+ await app._refreshCustomModelRunOptions(menu);
+ expect(win.document.getElementById('runModeCustomModels')!.innerHTML).toBe('');
+ });
+
+ it('omits a CLI the host does not have installed, matching the stock entries’ own gating', async () => {
+ const { win, app } = bootApp({
+ customModelClis: [
+ { id: 'claude', label: 'Claude Code' },
+ { id: 'codex', label: 'Codex' },
+ ],
+ hosts: [{ id: 'llama-box', label: 'llama.cpp', baseUrl: 'http://localhost:8080', models: ['qwen3'] }],
+ cliAvailable: (id: string) => id === 'claude',
+ });
+ const menu = win.document.getElementById('runModeMenu')!;
+ await app._refreshCustomModelRunOptions(menu);
+ const container = win.document.getElementById('runModeCustomModels')!;
+ expect(container.querySelectorAll('button').length).toBe(1);
+ expect(container.textContent).toContain('Claude Code');
+ expect(container.textContent).not.toContain('Codex');
+ });
+});
+
+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({});
+ app.activeSessionId = 'already-open-session';
+ // Simulate every run*() function's own documented behaviour: a declined or
+ // failed launch handles its own error and returns normally without ever
+ // changing activeSessionId — it does NOT throw and does NOT leave it null.
+ app.run = async () => {};
+ app._runInFlight = false;
+ let applyCalled = false;
+ const realApiJson = app._apiJson.bind(app);
+ app._apiJson = async (path: string, opts?: unknown) => {
+ if (path.includes('/custom-model')) applyCalled = true;
+ return realApiJson(path, opts as never);
+ };
+
+ await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
+
+ expect(applyCalled).toBe(false);
+ expect(app.activeSessionId).toBe('already-open-session');
+ });
+
+ it('applies the endpoint once run() actually produces a NEW active session', async () => {
+ const { app } = bootApp({});
+ app.activeSessionId = 'old-session';
+ app.run = async () => {
+ app.activeSessionId = 'new-session';
+ };
+ const calls: Array<{ path: string; body: unknown }> = [];
+ app._apiJson = async (path: string, opts?: { body?: unknown }) => {
+ calls.push({ path, body: opts?.body });
+ return { customModel: { endpointId: 'llama-box' }, restarted: true };
+ };
+
+ await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
+
+ expect(calls).toHaveLength(1);
+ expect(calls[0].path).toBe('/api/sessions/new-session/custom-model');
+ expect(calls[0].body).toEqual({ endpointId: 'llama-box', modelId: 'qwen3' });
+ });
+
+ it('routes through run() itself, so the Run in-flight lock actually engages', async () => {
+ // CLAUDE.md, Run launch synchronization: the lock exists so a double click
+ // cannot create duplicate sessions. A hardcoded dispatch table bypassing
+ // run() would never set _runInFlight, which is what this pins.
+ const { app } = bootApp({});
+ let sawInFlight = false;
+ app.run = async function (this: typeof app) {
+ if (this._runInFlight) return;
+ this._runInFlight = true;
+ sawInFlight = true;
+ this._runInFlight = false;
+ };
+ await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
+ expect(sawInFlight).toBe(true);
+ });
+
+ it('restores the previous _runMode after a one-off custom-model launch, never persisting it', async () => {
+ const { app } = bootApp({});
+ app._runMode = 'opencode';
+ let modeDuringRun: string | undefined;
+ app.run = async function (this: typeof app) {
+ modeDuringRun = this._runMode;
+ };
+ await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
+ expect(modeDuringRun).toBe('claude');
+ expect(app._runMode).toBe('opencode');
+ });
+});
diff --git a/test/render-index-html.test.ts b/test/render-index-html.test.ts
index df5fb417..bc3de380 100644
--- a/test/render-index-html.test.ts
+++ b/test/render-index-html.test.ts
@@ -11,7 +11,7 @@
* Port: N/A (no server start).
*/
import { describe, it, expect, afterEach, vi } from 'vitest';
-import { WebServer } from '../src/web/server.js';
+import { WebServer, escapeScriptJson } from '../src/web/server.js';
import { isClaudeAvailable } from '../src/utils/claude-cli-resolver.js';
import { isOpenCodeAvailable } from '../src/utils/opencode-cli-resolver.js';
import { isCodexAvailable } from '../src/utils/codex-cli-resolver.js';
@@ -209,6 +209,19 @@ describe('WebServer.renderIndexHtml', () => {
}
});
+ it('escapeScriptJson neutralizes a literal , and still round-trips as a JS literal', () => {
+ // CliEntry.label is a plain string a user's own clis.json can set (up to 60
+ // chars), unlike __codemanCliAvailable's booleans-only payload, so this is
+ // the one injection that needs it. Exported so this tests the pure
+ // function directly rather than needing a real WebServer (which needs tmux).
+ const dangerous = JSON.stringify([{ id: 'x', label: '' }]);
+ const escaped = escapeScriptJson(dangerous);
+ expect(escaped).not.toContain('".
+ expect(eval(escaped)[0].label).toBe('');
+ });
+
it('still emits the object when nothing at all is installed', async () => {
// The all-false case is the one that matters most and the easiest to get
// wrong by only injecting when something resolves.
diff --git a/test/routes/custom-model-routes.test.ts b/test/routes/custom-model-routes.test.ts
index 059f210d..16b7011f 100644
--- a/test/routes/custom-model-routes.test.ts
+++ b/test/routes/custom-model-routes.test.ts
@@ -295,3 +295,97 @@ describe('defaultModelId — the Run-menu picker’s per-endpoint default', () =
expect(stored?.defaultModelId).toBe('qwen3');
});
});
+
+describe('apiKey is never handed back to the browser', () => {
+ afterEach(() => {
+ fetchMock.mockReset();
+ });
+
+ it('POST, GET and PUT responses all carry apiKeySet instead of the real key', async () => {
+ const { app } = await setup();
+ const create = await app.inject({
+ method: 'POST',
+ url: '/api/model-endpoints',
+ payload: { id: 'ep-secret', label: 'A', baseUrl: 'http://localhost:8080', apiKey: 'super-secret' },
+ });
+ expect(create.json().data.host.apiKey).toBeUndefined();
+ expect(create.json().data.host.apiKeySet).toBe(true);
+
+ const list = await app.inject({ method: 'GET', url: '/api/model-endpoints' });
+ const listed = (list.json() as Array<{ id: string; apiKey?: string; apiKeySet?: boolean }>).find(
+ (h) => h.id === 'ep-secret'
+ );
+ expect(listed?.apiKey).toBeUndefined();
+ expect(listed?.apiKeySet).toBe(true);
+ expect(JSON.stringify(list.json())).not.toContain('super-secret');
+
+ const update = await app.inject({
+ method: 'PUT',
+ url: '/api/model-endpoints/ep-secret',
+ payload: { label: 'Renamed', baseUrl: 'http://localhost:8080' },
+ });
+ expect(update.json().data.host.apiKey).toBeUndefined();
+ expect(update.json().data.host.apiKeySet).toBe(true);
+ expect(JSON.stringify(update.json())).not.toContain('super-secret');
+ });
+
+ it('a host with no key set at all reports apiKeySet: false', async () => {
+ const { app } = await setup();
+ const create = await app.inject({
+ method: 'POST',
+ url: '/api/model-endpoints',
+ payload: { id: 'ep-nokey', label: 'A', baseUrl: 'http://localhost:8080' },
+ });
+ expect(create.json().data.host.apiKeySet).toBe(false);
+ });
+
+ it('PUT with no apiKey keeps the stored one, rather than clearing it', async () => {
+ const { app } = await setup();
+ await app.inject({
+ method: 'POST',
+ url: '/api/model-endpoints',
+ payload: { id: 'ep-keep-key', label: 'A', baseUrl: 'http://localhost:8080', apiKey: 'original-key' },
+ });
+ // Edit without touching the API key field — the real bug this guards: a
+ // browser round-trip that only ever sees apiKeySet, never the real value,
+ // must not accidentally send an empty string and wipe a working credential.
+ const update = await app.inject({
+ method: 'PUT',
+ url: '/api/model-endpoints/ep-keep-key',
+ payload: { label: 'Renamed', baseUrl: 'http://localhost:8080' },
+ });
+ expect(update.json().data.host.apiKeySet).toBe(true);
+
+ // Prove it by observing the auth header discovery actually sends.
+ fetchMock.mockImplementation(async (_url: URL, init?: RequestInit) => {
+ const headers = init?.headers as Record;
+ expect(headers.Authorization).toBe('Bearer original-key');
+ return new Response(JSON.stringify({ data: [] }), { status: 200 });
+ });
+ const discover = await app.inject({ method: 'POST', url: '/api/model-endpoints/ep-keep-key/discover-models' });
+ expect(discover.json().success).toBe(true);
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+
+ it('PUT with a new apiKey replaces the stored one', async () => {
+ const { app } = await setup();
+ await app.inject({
+ method: 'POST',
+ url: '/api/model-endpoints',
+ payload: { id: 'ep-replace-key', label: 'A', baseUrl: 'http://localhost:8080', apiKey: 'old-key' },
+ });
+ await app.inject({
+ method: 'PUT',
+ url: '/api/model-endpoints/ep-replace-key',
+ payload: { label: 'A', baseUrl: 'http://localhost:8080', apiKey: 'new-key' },
+ });
+
+ fetchMock.mockImplementation(async (_url: URL, init?: RequestInit) => {
+ const headers = init?.headers as Record;
+ expect(headers.Authorization).toBe('Bearer new-key');
+ return new Response(JSON.stringify({ data: [] }), { status: 200 });
+ });
+ await app.inject({ method: 'POST', url: '/api/model-endpoints/ep-replace-key/discover-models' });
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+});