diff --git a/.changeset/custom-model-picker-in-use.md b/.changeset/custom-model-picker-in-use.md new file mode 100644 index 00000000..d8b27bba --- /dev/null +++ b/.changeset/custom-model-picker-in-use.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +The "Choose a model" picker now says which model is actually in use on an endpoint that cannot report it. Only llama-swap answers that question, so behind a gateway such as Bifrost (and behind a bare llama.cpp, Ollama or vLLM) nothing was marked and the list fell back to plain discovery order, apart from the one CLI that happened to have launched there before. A live session whose own injected endpoint is that host now promotes its model as **"In use by a session"**, under its own tag and behind llama-swap's own "Currently loaded" answer, reading only what Codeman injected rather than text scraped off a terminal, and never a stopped or exited session. When nothing is known the picker says so instead of leaving an unmarked wall; its wording claims only that nothing was reported, which stays true for an endpoint that is down, since the route answers every probe failure as `isLlamaSwap: false`. Pinned by test/custom-model-run-menu-ui.test.ts. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 310f8945..3b827d91 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -640,7 +640,7 @@ Adding the header changed the preamble, so `CODEMAN_PREAMBLE` was bumped (1.22.0 ⚠️ **The Run-menu picker generates entries from `window.__codemanCustomModelClis`** (`server.ts`, injected at page render from `enabledClis().filter(kind==='agent' && customModelInjection.kind!=='unsupported')`, JSON-escaped against a literal `` via the exported `escapeScriptJson()` since `label` is a user-`clis.json`-settable string unlike the neighbouring booleans-only `__codemanCliAvailable`), never a hardcoded per-CLI id list in the frontend — the same "no branching on CLI id outside stock.ts" discipline the registry itself enforces. One entry per (capable, INSTALLED CLI, saved endpoint) pair, e.g. "Claude Code (llama.cpp)", filtered through `isCliAvailable()` like the stock entries. Clicking one calls `selectCustomModelEntry(mode, endpointId)` (`session-ui.js`), which re-fetches the endpoint (never trusts anything cached from the dropdown's render — the 5-minute sweep below or a settings edit may have changed it since) and decides the model: exactly one discovered model launches straight away, two or more open `#customModelPickModal` to ask, with `defaultModelId` marked but never auto-chosen (asking exists so ONE launch can deliberately differ from the saved default). -⚠️ **The picker promotes exactly one row to the top**: whichever model llama-swap reports `ready` right now (tagged "Currently loaded", queried via `GET /api/model-endpoints/:id/running-status`, client-side bounded to ~800ms via `Promise.race` so a sleeping/firewalled endpoint cannot leave the modal invisible for the route's own 5s server-side timeout) beats a merely remembered choice, and — only when nothing is currently loaded — the model actually launched last for this exact (harness, endpoint) pair (tagged "Last used", read from the per-device `codeman:customModelLastUsed::` localStorage key). Neither tag reorders past the top, and the "Default" pill is a SEPARATE span rather than a third value of the same slot, so a promoted row that is also the endpoint's `defaultModelId` shows both (on a single-purpose GPU box that is the common case; an exclusive slot silently dropped the Default marking for exactly that row). "Last used" is written by `_runCustomModelEntryViaRestart` (claude) and `_quickStartWithCustomModelConfirm` (every one-shot launch; `runCustomModelEntry` itself only dispatches between the two) only once the model is actually applied — never on the mere click — because a context-window-warning decline means this exact model cannot work with this CLI at all, and promoting a model that cannot launch would be actively wrong, not just premature. +⚠️ **The picker promotes exactly one row to the top**: whichever model llama-swap reports `ready` right now (tagged "Currently loaded", queried via `GET /api/model-endpoints/:id/running-status`, client-side bounded to ~800ms via `Promise.race` so a sleeping/firewalled endpoint cannot leave the modal invisible for the route's own 5s server-side timeout) beats a live session's pin (tagged "In use by a session": `_getCustomModelInUseBySession` walks `this.sessions` for one of your own sessions whose `customModel.endpointId` is that host, newest `createdAt` wins), which applies only when nothing is currently loaded and beats a merely remembered choice, the model actually launched last for this exact (harness, endpoint) pair (tagged "Last used", read from the per-device `codeman:customModelLastUsed::` localStorage key). ⚠️ The pin reads only `customModel`, never `displayModel` (screen-scraped display text: a terminal that merely printed a matching string is not evidence), and never counts a session with `status: 'stopped'`, a null `pid` (its PTY exited) or a recorded `paneExit`, the same live-pane test keep-warm applies. ⚠️ When nothing was promoted and the probe answered `isLlamaSwap: false` (the shape `getLlamaSwapStatus` gives for EVERY failure it catches, a down or refused endpoint included, so the line claims only that nothing was reported), the static `#customModelPickUnknown` line says so out loud; its `[hidden]` rule must keep `display: none !important` scoped to `#customModelPickModal`, because `.form-hint`'s `display: block` outranks the UA `[hidden]` rule. Neither tag reorders past the top, and the "Default" pill is a SEPARATE span rather than a third value of the same slot, so a promoted row that is also the endpoint's `defaultModelId` shows both (on a single-purpose GPU box that is the common case; an exclusive slot silently dropped the Default marking for exactly that row). "Last used" is written by `_runCustomModelEntryViaRestart` (claude) and `_quickStartWithCustomModelConfirm` (every one-shot launch; `runCustomModelEntry` itself only dispatches between the two) only once the model is actually applied — never on the mere click — because a context-window-warning decline means this exact model cannot work with this CLI at all, and promoting a model that cannot launch would be actively wrong, not just premature. Either way the actual launch (`runCustomModelEntry`) routes through `run()` itself via a temporary `_runMode` swap — never `setRunMode()`, which would persist it as the user's new default — rather than a parallel dispatch table, which is what gives a custom-model launch the same `_runInFlight` lock every other Run click gets and means a CLI whose injection recipe lands later needs no update here. It then GETs `/api/sessions/:id/wait?until=idle&timeout=20000` on that session BEFORE applying — measured live, a freshly launched CLI reports itself `busy` for its own startup (boot spinner, workspace-trust check) well before the apply call would otherwise reach it, and the apply route's `isBusy()` guard correctly can't tell that apart from a real turn in progress, so every fresh launch failed with `SESSION_BUSY` until this wait was added. A timeout there is a normal 200 per the wait endpoint's own contract, never an error, so a session still busy after 20s just reaches the apply call anyway and gets that route's own honest error. It then calls `POST /api/sessions/:id/custom-model` on the session `run()` produced, guarded by snapshotting `activeSessionId` before the call and requiring it to have actually changed after — every `run*()` handles its own failure internally and returns normally rather than throwing, so a declined/failed launch must not silently re-point and restart whatever session was already open. diff --git a/docs/custom-model-endpoints.md b/docs/custom-model-endpoints.md index ac968659..81bf7add 100644 --- a/docs/custom-model-endpoints.md +++ b/docs/custom-model-endpoints.md @@ -200,32 +200,66 @@ saved default, not just confirming it. The modal promotes exactly one row to the top of the list rather than always showing raw discovery order, so the zero-wait choice is the one -under your thumb: +under your thumb. Three kinds of knowledge feed that promotion, best +evidence first, and the first one that answers wins: -- **"Currently loaded"** — a model from this host's own list that +- **"Currently loaded"** is a model from this host's own list that llama-swap reports `ready` right now, queried via `GET /api/model-endpoints/:id/running-status`. Bounded client-side to ~800ms (`Promise.race`), on top of the route's own 5s server-side timeout, so an endpoint that is asleep or firewalled cannot leave the modal invisible for the full 5s after the Run menu has already closed. -- **"Last used"** — shown only when nothing is currently loaded: the model - actually launched last for this exact (harness, endpoint) pair, read - from the per-device `codeman:customModelLastUsed::` - localStorage key. Written by `_runCustomModelEntryViaRestart` (claude) - and `_quickStartWithCustomModelConfirm` (every one-shot launch; the +- **"In use by a session"** is the fallback for every endpoint that + cannot report its own loaded model, which is every server except + llama-swap: a bare llama.cpp, Ollama, vLLM, and a gateway such as + Bifrost fronting a whole fleet all answer `isLlamaSwap: false`. + `_getCustomModelInUseBySession` walks `this.sessions` for one of your own + sessions whose `customModel.endpointId` is this host, newest + `createdAt` wins, and promotes its model. A live session is first-hand + evidence of a model in use on the endpoint, because Codeman put it + there, so it earns the promotion and the tag says where the knowledge + came from. Two fences keep it honest: it reads only `customModel`, + never `displayModel` (display text scraped off a CLI footer, so a + terminal that merely printed a similar string must not mark a row), + and a session with `status: 'stopped'`, a null `pid` (its PTY exited, + the same live-pane test keep-warm applies) or a recorded `paneExit` + does not count, since nothing is on the endpoint any more. Sessions on other + endpoints, and models the host no longer discovers, are skipped. +- **"Last used"** appears only when neither of the above answers: the + model actually launched last for this exact (harness, endpoint) pair, + read from the per-device localStorage key + `codeman:customModelLastUsed::`. That key is per CLI + on purpose, so it can only + ever promote the CLI that launched there before, which is why a gateway + box with no session running shows plain discovery order for the other + CLIs. Written by `_runCustomModelEntryViaRestart` (claude) and + `_quickStartWithCustomModelConfirm` (every one-shot launch; the `runCustomModelEntry` entry point itself only dispatches between the - two) only once the model is actually applied, never on the mere click — + two) only once the model is actually applied, never on the mere click: declining the context-window warning means this exact model cannot work - with this CLI at all, so promoting it next time would be actively wrong, - not just premature. + with this CLI at all, so promoting it next time would be actively + wrong, not just premature. -Neither tag reorders anything past that one promoted row. The "Default" +No promotion reorders anything past that one promoted row. The "Default" pill is a separate span, not a third value of the same slot: a promoted row that is also the endpoint's `defaultModelId` shows both tags (on a single-purpose GPU box that is the common case, and an exclusive slot silently dropped the Default marking for exactly that row), and a row with neither promotion nor default shows no tag at all. +When nothing is known about any row, `#customModelPickUnknown` says so +out loud instead of leaving an alphabetical wall that reads as the app +ignoring the question. It shows when the probe answered +`isLlamaSwap: false`, which the route gives for every failure it +catches, so one answer covers a server that cannot report and an +endpoint that is down or refused alike; the line's wording therefore +claims only that nothing was reported. A probe that never got any +answer (`_getCustomModelRunningStatus` resolving `null`, Codeman's own +request failed or the client-side bound won) keeps the line hidden. +Like its sibling no-matches line it needs an +explicit `[hidden]` rule scoped to `#customModelPickModal`, because +`.form-hint`'s `display: block` outranks the UA `[hidden]` rule. + **How the launch itself applies the endpoint depends on the harness.** For opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP (`runCustomModelEntry` → `_runCustomModelEntryOneShot`), the endpoint/model is folded into the SAME diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index 845c4e5a..10b0dbd3 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -627,7 +627,10 @@ 'Custom Endpoints': '自定义端点', 'Choose a model': '选择模型', 'Currently loaded': '当前已加载', + 'In use by a session': '会话正在使用', 'Last used': '上次使用', + 'Nothing is marked: this endpoint did not report which model it has loaded, and none of your running sessions uses one. Pick the model to launch.': + '未标记任何模型：此端点没有报告它已加载了哪个模型，你运行中的会话也没有在使用它。请选择要启动的模型。', 'That endpoint no longer exists': '该端点已不存在', 'No models discovered for this endpoint yet': '此端点尚未发现任何模型', // Model picker search box (index.html placeholder/aria-label and the no-matches line) diff --git a/src/web/public/index.html b/src/web/public/index.html index f7ecbfd8..26eca72a 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1041,6 +1041,11 @@ autocomplete="off" autocapitalize="off" spellcheck="false" enterkeyhint="search" oninput="app.filterCustomModelPick(this.value)"> + +
diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 7ce1dadf..9eda724b 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -1002,12 +1002,23 @@ Object.assign(CodemanApp.prototype, { }, /** - * Best-effort lookup of the model llama-swap currently has loaded and ready on this - * endpoint, so the picker can offer it first instead of making the user remember what - * they picked last time it mattered. Mirrors `_watchLlamaSwapLoading`'s own - * `state === 'ready'` check. Returns null for a plain (non-llama-swap) server, an - * unreachable endpoint, or a loaded model this host no longer lists as discovered — - * never throws, since a failed probe should just skip promotion, not break the picker. + * Best-effort lookup of what the endpoint itself says it has loaded and ready, so the + * picker can offer it first instead of making the user remember what they picked last + * time it mattered. Only llama-swap answers this question (the route reads its + * `/running`); every other OpenAI-compatible server (a bare llama.cpp, Ollama, vLLM, + * or a gateway such as Bifrost fronting a whole fleet) replies `isLlamaSwap: false`, + * which is why `_getCustomModelInUseBySession` exists. `state === 'ready'` mirrors + * `_watchLlamaSwapLoading`'s own check. + * + * Resolves the raw route payload rather than a model id so the caller can tell the + * three answers apart: llama-swap naming a model; `isLlamaSwap: false`, which is what + * the route answers for EVERY failure it catches (a server without a `/running` + * route, a refused or unresolvable endpoint, a 401 from a wrong key) — safe to + * explain in the UI as long as the wording also fits an endpoint that is down; and + * `null`, only when Codeman's own request to the route failed or the client-side + * bound below won the race, which stays silent, since the endpoint may merely have + * been asleep. Never throws: a failed probe should + * just skip promotion, not break the picker. * * ⚠️ Client-side bounded to ~800ms via Promise.race, on top of (never instead of) the * route's own 5s server-side timeout (`RUNNING_TIMEOUT_MS`, custom-model-routes.ts) — @@ -1024,17 +1035,58 @@ Object.assign(CodemanApp.prototype, { * `vi.useFakeTimers()` patches, so a param is the only way to test the timeout without * actually waiting on it. Real callers never pass it. */ - async _getCustomModelCurrentlyLoaded(host, timeoutMs = 800) { + async _getCustomModelRunningStatus(host, timeoutMs = 800) { const probe = this._apiJson(`/api/model-endpoints/${encodeURIComponent(host.id)}/running-status`).catch( () => null ); const timeout = new Promise((resolve) => setTimeout(() => resolve(null), timeoutMs)); - const status = await Promise.race([probe, timeout]); + return Promise.race([probe, timeout]); + }, + + /** + * The model llama-swap reports `ready` on this endpoint AND that this host still lists + * as discovered (an undropped-but-undiscovered model has no row to promote), else null. + */ + _pickReadyRunningModel(status, host) { if (!status?.isLlamaSwap) return null; const ready = (status.running || []).find((r) => r.state === 'ready' && (host.models || []).includes(r.model)); return ready?.model || null; }, + /** + * The model a LIVE session of this instance is pointed at on `host`, or null. It is the + * fallback for every endpoint that cannot answer "what is loaded?" about itself. + * A session already running on the endpoint is first-hand evidence of a model in use + * there: Codeman itself injected that endpoint and model into it, so it earns the same + * promotion llama-swap's own answer gets, under its own honest tag. + * + * ⚠️ Reads ONLY `customModel` (what Codeman injected), never `displayModel`: that field + * is display text scraped off a CLI's footer or statusline for a session header (pane + * derived, `source: 'screen'`), and promoting a row because a terminal printed a + * similar string is worse than showing nothing. + * ⚠️ Requires a live pane: `status: 'stopped'`, a null `pid` (the PTY exited; the + * same live-pane test keep-warm applies, `src/prompt-cache-keepwarm.ts`) or a recorded + * `paneExit` means nothing is on the endpoint any more, and a dead session's stale pin + * is exactly what an "in use" claim must not resurrect. `session:updated` keeps it + * current, so a session cleared back to the CLI's own backend or moved to another + * endpoint stops counting. + * ⚠️ A since-deleted endpoint simply matches no host, so no extra liveness check is + * needed here. + */ + _getCustomModelInUseBySession(host) { + const models = host?.models || []; + if (models.length === 0) return null; + let newest = null; + for (const [, session] of this.sessions || []) { + if (!session || session.status === 'stopped' || session.pid === null || session.paneExit) continue; + const pinned = session.customModel; + if (!pinned || pinned.endpointId !== host.id) continue; + if (!models.includes(pinned.modelId)) continue; // undiscovered since: no row to promote + if (!newest || (newest.createdAt || 0) <= (session.createdAt || 0)) newest = session; + } + return newest?.customModel?.modelId || null; + }, + /** * Renders the "which model" picker for a (harness, endpoint) pair with more than one * discovered model. Async since it now awaits the currently-loaded-model probe below, @@ -1055,15 +1107,17 @@ Object.assign(CodemanApp.prototype, { const generation = (this._customModelPickGeneration = (this._customModelPickGeneration || 0) + 1); const isCurrent = () => this._customModelPickGeneration === generation; - // Whichever model llama-swap actually has loaded right now beats a merely - // remembered choice — it's what a launch would attach to with zero wait, while - // "last used" might have been swapped out by another session since. Neither - // reorders past the top: exactly one model is promoted, everything else keeps - // its discovery order. - const currentlyLoaded = await this._getCustomModelCurrentlyLoaded(host); + // Three kinds of knowledge, best first: what llama-swap says it has loaded right + // now (zero wait to attach to), what a live session of this instance is pointed at + // on that endpoint (the endpoint itself cannot say, but Codeman put it there), and + // only then this device's memory of the last launch. One wins, and it never reorders + // anything else: every other row keeps its discovery order. + const runningStatus = await this._getCustomModelRunningStatus(host); if (!isCurrent()) return; // a newer pick opened (and possibly already rendered) while this probe was in flight - const lastUsed = currentlyLoaded ? null : this._getCustomModelLastUsed(mode, host.id); - const promoted = currentlyLoaded || lastUsed; + const currentlyLoaded = this._pickReadyRunningModel(runningStatus, host); + const inUse = currentlyLoaded ? null : this._getCustomModelInUseBySession(host); + const lastUsed = currentlyLoaded || inUse ? null : this._getCustomModelLastUsed(mode, host.id); + const promoted = currentlyLoaded || inUse || lastUsed; const models = [...(host.models || [])]; if (promoted && models.includes(promoted)) { models.splice(models.indexOf(promoted), 1); @@ -1078,14 +1132,37 @@ Object.assign(CodemanApp.prototype, { document.getElementById('customModelPickTitle').textContent = 'Choose a model'; document.getElementById('customModelPickHint').textContent = `${cliLabel} → ${host.label} — ${(host.models || []).length} models discovered.`; + // A plain static line (its markup lives in index.html), shown when NOTHING is known + // about any row: 40 alphabetical rows with no mark on any of them otherwise read as + // the app ignoring the question. `isLlamaSwap: false` is what the route answers for + // every failure it catches: a server that cannot report AND a down or refused one + // alike (`getLlamaSwapStatus`, custom-model-routes.ts), so the line's wording + // promises only that nothing was reported, which stays true either way. A probe + // that never got an answer at all (`runningStatus === null`: Codeman's own request + // failed or the 800ms bound won) keeps the line hidden, the box may merely be + // asleep. + // A remembered id that has since vanished from host.models is no promotion: count + // only a promoted id that is actually a row, or a stale "last used" would both skip + // tagging any row and hide this line, leaving 40 unmarked rows with no explanation. + const nothingKnown = !(promoted && models.includes(promoted)) && runningStatus?.isLlamaSwap === false; + const unknownLine = document.getElementById('customModelPickUnknown'); + if (unknownLine) unknownLine.hidden = !nothingKnown; list.innerHTML = models .map((m) => { - // Two independent tags, never one exclusive slot: the promotion tag says what - // llama-swap (or this device's history) knows about the model, the Default pill - // says what the saved endpoint says about it, and on a single-purpose GPU box the - // promoted model IS the default more often than not. One slot holding whichever - // applied first silently dropped the Default marking for exactly that row. - const promotion = m === currentlyLoaded ? 'Currently loaded' : m === lastUsed ? 'Last used' : null; + // Two independent tags, never one exclusive slot: the promotion tag says what the + // endpoint, a live session, or this device's history knows about the model, the + // Default pill says what the saved endpoint says about it, and on a + // single-purpose GPU box the promoted model IS the default more often than not. + // One slot holding whichever applied first silently dropped the Default marking + // for exactly that row. + const promotion = + m === currentlyLoaded + ? 'Currently loaded' + : m === inUse + ? 'In use by a session' + : m === lastUsed + ? 'Last used' + : null; const tags = [promotion, m === host.defaultModelId ? 'Default' : null].filter(Boolean); const arg = escapeHtml(JSON.stringify(m)); // data-model-id is the filter's haystack (escapeHtml covers `"` as well as diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 6208f123..11f843bc 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -7831,6 +7831,17 @@ html[data-skin="og"] .welcome-primary:hover { padding: 1rem 0; } +/* "Nothing is marked: this endpoint did not report which model it has loaded": the same + `.form-hint` `display: block` beats `[hidden]` trap the two rules below fix for the + no-matches line and the filtered rows, so the attribute needs the same re-assertion. */ +#customModelPickModal .custom-model-pick-unknown { + padding-bottom: 0.15rem; +} + +#customModelPickModal .custom-model-pick-unknown[hidden] { + display: none !important; +} + /* Same class of bug as the row rule below: the no-matches line carries `.form-hint`, whose `display: block` outranks the UA `[hidden]` rule, so `filterCustomModelPick()` toggling `hidden` changed nothing on screen and diff --git a/test/custom-model-run-menu-ui.test.ts b/test/custom-model-run-menu-ui.test.ts index 0197e824..65e50be2 100644 --- a/test/custom-model-run-menu-ui.test.ts +++ b/test/custom-model-run-menu-ui.test.ts @@ -44,6 +44,8 @@ function bootApp( cliAvailable?: (id: string) => boolean; activeCase?: { location?: string } | null; settingsEnabled?: boolean; + /** Live sessions for the "in use by a session" fallback, in `this.sessions` Map shape. */ + sessions?: Array>; } = {} ) { const dom = new JSDOM( @@ -62,6 +64,7 @@ function bootApp( +