diff --git a/.changeset/run-instance-count-non-claude.md b/.changeset/run-instance-count-non-claude.md index b0ff890f..308d4833 100644 --- a/.changeset/run-instance-count-non-claude.md +++ b/.changeset/run-instance-count-non-claude.md @@ -8,4 +8,5 @@ The Instance count stepper next to the Run button only ever applied to Claude. Setting it to 3 and launching OpenCode, Codex, Gemini, Antigravity, Pi, OMP, Grok or DeepSeek started exactly one session, with no error and no hint that the control had done nothing. All eight now launch the count you asked for, and the opening banner -says how many are starting. +says how many are starting. The one exception is a launch started from the Custom +Endpoints section of the Run menu, which always starts a single session. diff --git a/.changeset/run-menu-custom-model-picker.md b/.changeset/run-menu-custom-model-picker.md index 4a8b8b66..9e4bf476 100644 --- a/.changeset/run-menu-custom-model-picker.md +++ b/.changeset/run-menu-custom-model-picker.md @@ -31,5 +31,17 @@ swap window, so a prompt sent mid-swap reads as loading rather than as an answer whatever was loaded a moment ago. A background sweep also catches the reverse: your session's model being evicted later by somebody else's ordinary use. +Two things worth knowing if you drive this over the HTTP API or run multi-user. The two +questions an apply can ask (the model's context window is too small, and loading it will +unload the model another session is using) are now answered by separate +`confirmedContext` and `confirmedSwap` fields rather than one `confirmed`. They shared a +flag until now, and since the context check runs first, confirming that one silently +agreed to evict another session's model as well. The old `confirmed` still means both. +And `CLAUDE_CONFIG_DIR` is now admin-only in multi-user mode: it joined claude's +privileged env keys, so a non-granted owner can no longer set it through `envOverrides`, +and an already-persisted one is dropped on reboot-restore, which returns that session to +the default Claude account rather than the per-client one it was pointed at. Single-user +installs are unaffected. + Remote SSH and Docker sessions are refused for now, since their restart reattaches a durable tmux rather than relaunching the agent. diff --git a/CLAUDE.md b/CLAUDE.md index 75470f58..a2601b7f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -231,7 +231,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Custom Model Endpoint Profiles** (opt-in, `customModelEndpointsEnabled`, SYNCED, default OFF; `docs/custom-model-endpoints.md`, design doc `docs/custom-model-endpoints-plan.md`; full stack — settings-panel CRUD + the Run-menu picker, on top of the backend below): points a session at a user-configured custom OpenAI-compatible endpoint — local (llama.cpp, DGX Spark, Strix Halo) or cloud (Azure AI Foundry, OpenRouter) — instead of its harness's native cloud backend. Endpoints are a read/write-array store (`custom-model-hosts.ts`, `~/.codeman/custom-model-hosts.json`) discovered via `GET /v1/models`; `CustomModelHost.authStyle` is `'bearer'` (default, `Authorization: Bearer`) or `'api-key'` (Azure's convention) — **never both**, live-tested against a real server: sending both headers on one request reliably hangs it indefinitely, reproduced 3×. ⚠️ The actual per-CLI redirect is `capabilities.customModelInjection` on the CLI registry (four kinds: `env` for claude/gemini/deepseek, `configContentEnv` reusing opencode's existing `OPENCODE_CONFIG_CONTENT`, `configDir` for codex/pi/grok/omp — writes an isolated per-session config file, NEVER the user's real `~/.codex`/`~/.pi`/`~/.omp`/grok config — and `unsupported` for antigravity, which has no known mechanism), computed by the pure `custom-model-injection.ts` (mirrors `session-cli-builder.ts`'s no-IO discipline). ⚠️ `PI_CONFIG_DIR` does NOTHING for pi or omp (grepped pi's entire bundled JS source — the string appears nowhere); both hardcode `~/.pi/agent/models.json` / `~/.omp/agent/models.yml` with no dedicated override, so the real redirect for both is the child process's own **`HOME`**, and both need `models` as an ARRAY of `{id}` objects (an object keyed by id silently loads zero models). Grok's real mechanism turned out to be a `config.toml` `[model.]` block redirected via `GROK_HOME` — its original env-var-based recipe was flat-out wrong (produced "Not signed in" against a real binary), not just unverified. ⚠️ **Two launch paths, chosen by mechanism, not preference — see the second paragraph below for why**: opencode/codex/gemini/pi/grok/deepseek/omp apply the selection ONE-SHOT, before the session/process ever exists, with no restart at all; claude alone still applies a selection by **restarting the session's CLI process in place** via `Session.restartCli()` — a de-restricted `reattachRemote()` reusing the same `respawn-pane -k` primitive local/remote respawns already share — because every one of these harnesses reads its endpoint config at process start, never per-turn, so there is no live hot-swap; `Session.setCustomModel()` undoes the PREVIOUS selection's env keys (and deletes its old `configDir`) before merging the new ones in, so switching endpoints or clearing back to native cloud never leaves a stale key behind. ⚠️ Deleting a key from `_envOverrides` is NOT enough on its own: `tmux setenv` persists at the tmux-session level and is inherited by `respawn-pane` (measured: `setenv FOO bar` survived two successive `respawn-pane -k`), so the retired keys are queued (`_pendingEnvUnsets`) and ride `RespawnPaneOptions.unsetEnvKeys` into `applyEnvOverrides()`, which `setenv -u`s them BEFORE re-applying the live overrides. ⚠️ `restartCli()` kills a WORKING pane, so a CLI whose launch declares a `fallback` chain (claude) gets the live conversation id pinned as `resumeSessionId` for that one respawn: `--session-id ` refuses an id that already has a transcript (`Session ID ... is already in use`), and without the `--resume || --session-id ` shape the docker/remote pane commands already use, applying a model killed the pane and lost the session. ⚠️ pi, omp and grok need the config file AND a `model` launch param (`custom/` for pi/omp, grok's `[model.codeman-custom]` block name): that is the registry's `customModelInjection.launchModel` template, applied onto the respawn options through `legacyConfigField` by `_withCustomModelLaunchModel()`, never by id, and a model id the CLI's `model` token pattern cannot carry is refused with a 400 rather than silently dropped by the argv engine. ⚠️ Remote (SSH) and Docker sessions are REFUSED (400): their `restartCli()` reattaches a durable tmux rather than restarting the agent and the env lands on the local pane, so they used to report `restarted:true` and change nothing. The selection survives a Codeman restart as the disk-only `__customModel` (bookkeeping: env KEYS, config dir, launch model; never the values, which carry the API key and are re-derived from the endpoint store on recovery), the config dir is removed with the session, and every secret-bearing file (`custom-model-hosts.json`, the per-session config dir) is written 0600. ⚠️ **Security**: every env var this feature can redirect (`ANTHROPIC_BASE_URL`, `GOOGLE_GEMINI_BASE_URL`, `CODEX_HOME`, `GROK_HOME`, `HOME` for pi/omp, `OPENCODE_CONFIG_CONTENT`, etc.) is in that CLI's `privilegedEnvKeys` — several of these were reachable via the generic `envOverrides` field's prefix allowlist BEFORE this feature existed (the env allowlist is global and prefix-based, not per-CLI-scoped), so building this surfaced and closed a pre-existing gap rather than opening a new one. `ANTHROPIC_*` is deliberately NOT in claude's `allowedPrefixes` at all — Anthropic-traffic redirection can only happen through this feature's own admin-configured, SSRF-guarded route, never a plain client-supplied `envOverrides`. **Confidence, verified end-to-end against a real llama-swap server via the DYNAMIC `scripts/test-local-llm-harnesses.ts`** (reads the live CLI registry, so a registry change needs zero script edits): claude/opencode/pi/grok/omp **PASS**; codex config structure is correct, and codex only speaks the Responses API since Feb 2026 (`wire_api = "responses"`) — re-verified live against a llama-swap deployment that DOES answer `/v1/responses` (an earlier test's harder failure against a different deployment does not reproduce everywhere): a plain, no-tool-call chat turn gets a real reply, but a real tool-call attempt came back as `agent_message` TEXT (the tool-call JSON printed as the answer) rather than an executable `function_call` item — confirmed via `codex exec --json`'s raw event stream. Tool execution is what makes codex a coding agent, so it remains not usable for real work either way, just with a more precise failure mode than a flat protocol break; gemini fails with `Invalid auth method selected` (an undocumented `GATEWAY` AuthType gemini-cli selects once `GOOGLE_GEMINI_BASE_URL` is set — unresolved after real investigation); deepseek's originally-reported `HTTP_404` is root-caused and fixed — its bundled `@deepseek-ai/dsh-llm-deepseek` module builds `${DEEPSEEK_BASE_URL}/chat/completions` with no `/v1` of its own (confirmed by installing the real package and reading its source), so a new `appendV1Suffix` flag on its registry entry (alone — claude/gemini must not get it) runs `endpoint.baseUrl` through `withV1Suffix()` before writing it, live-confirmed against llama-swap (`.../chat/completions` 404s, `.../v1/chat/completions` succeeds) though not yet re-run through an actual `dsh` binary, which isn't installable in this environment; antigravity has no known mechanism at all. See the confidence table in `docs/custom-model-endpoints-plan.md` for the full detail on each. ⚠️ **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). 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. ⚠️ The apply call reads the response body itself (`_api()`) rather than `_apiJson()`, which unwraps success but silently discards a failure body — losing the one thing (`error`) that distinguishes "still busy", "remote/Docker session" and everything else the route can report (⚠️ neither route validates `modelId` against the endpoint's discovered list, deliberately: discovery can be up to 5 minutes stale, so a 400 there would refuse a launch that works — a typo'd id fails on the CLI's own first request instead); the resulting toast is `type: 'error'` with an explicit `duration: 0` (no auto-dismiss, an explicit close button) at that one call site — not a blanket sticky-error default, which stacked unbounded on `.toast-container` with no cap or eviction — precisely so a message worth diagnosing survives long enough to be read instead of vanishing on the usual 3s timer. Entries are hidden for a remote/docker active case (the apply route refuses both) and for an endpoint with no discovered models at all (nothing to launch with). ⚠️ **Every saved endpoint's models also re-discover themselves automatically**, a `this.cleanup.setInterval` in `server.ts` (`CUSTOM_MODEL_REDISCOVER_INTERVAL_MS`, 5 minutes, off under `testMode` like the Codex plan-usage poll beside it) calling the exported `refreshAllCustomModelHosts()` (`custom-model-routes.ts`) — one endpoint unreachable on a cycle never blocks the others, and a read-modify-write PER HOST (re-reading the store before each splice, keyed by id) means an admin's concurrent edit or delete wins over a sweep that started before it, never the reverse. -**Everything below landed after the initial backend + picker cut, each confirmed live against a real llama-swap deployment.** ⚠️ **llama-swap runs one model at a time, and switching can disrupt ANOTHER live session** — before applying, both apply routes call llama-swap's own `GET /running` (feature-detected via `getLlamaSwapStatus()`, `custom-model-routes.ts`; a plain llama.cpp/OpenAI-compatible server has no such endpoint and is simply never checked). If a different model is loaded and ready AND another live session's own selection is using it, the apply returns `{requiresConfirmation, currentlyLoadedModel, affectedSessions}` instead of switching silently; retrying with `confirmed: true` skips the check, and switching with nothing else affected proceeds immediately. llama-swap also has no dedicated "switch model" endpoint — the only thing that actually starts a swap is a real inference request naming the model (confirmed live: applying a selection alone never reached llama-swap's own logs, since nothing had asked it to load anything) — so both routes also fire `triggerLlamaSwapLoad()`, a fire-and-forget `POST /v1/chat/completions` with `max_tokens: 1`, whenever the target model isn't already loaded and ready. ⚠️ **That launch-time check cannot catch a swap caused by a DIFFERENT session's LATER, ordinary use** — confirmed live: a second Codex session picking a different model launched with no warning at all (nothing conflicted at that exact instant), yet it silently evicted the first session's model regardless, since llama-swap has no push notification of its own. `detectCustomModelSwapDisplacements()` (`custom-model-routes.ts`) is a separate periodic sweep (`server.ts`, `CUSTOM_MODEL_SWAP_CHECK_INTERVAL_MS` = 20s) that compares each live custom-model session's own `modelId` against what `/running` actually reports loaded, broadcasting a `custom-model:swapped-out` SSE event — shown as a global toast, never tied to the displaced session's own tab, since the whole point is telling the user before they type into it — the first time a mismatch appears, via a caller-owned de-dupe `Set` cleared once that session's own model is loaded and ready again so a later, genuinely new displacement notifies again rather than staying silently un-notified forever after the first one. ⚠️ **Context length is read from the REAL launch command, never `/props`** — `/props?model=`'s `default_generation_settings.n_ctx` was confirmed live to report a `--fit-ctx`-launched backend's theoretical/trained maximum rather than the real runtime-configured size (a measured 154112-vs-16384 discrepancy, caught only because the unfixed value still overflowed), so discovery parses the actual configured size straight out of `/running`'s own `cmd` field instead (`parseCtxFromCmd`: `--fit-ctx ` first, then plain llama.cpp `-c`/`--ctx-size`), falling back to `/props` only when `cmd` states no recognizable flag at all. ⚠️ **Claude alone gets a context-window FLOOR check, on top of the ceiling `contextLengthVar` already fixes** — `exceedsSafeContextFloor()` (gated on the registry declaring `contextLengthVar`, so a no-op for every other CLI by construction) compares a model's discovered context against `CLAUDE_MIN_SAFE_CONTEXT_TOKENS` (40000): confirmed live, twice, that Claude Code's own system prompt and tool schemas cost roughly 36.4K tokens on the very first message, before any conversation history exists to compact, so a smaller real context fails outright regardless of what `CLAUDE_CODE_MAX_CONTEXT_TOKENS` says (that var only controls when HISTORY gets compacted, and there is none yet on message one). Below the floor, the apply returns `{requiresContextWarning, modelId, contextLength, minSafeContextTokens}` instead of launching, shown as an in-app dialog naming the actual fix: give the model an explicit larger `-c`/`--ctx-size` in llama-swap's config instead of relying on `--fit-ctx` auto-fit, which optimizes for the biggest MODEL that fits rather than the biggest CONTEXT. ⚠️ **A fresh, isolated `CLAUDE_CONFIG_DIR` looks like a brand-new Claude Code profile and replays its ENTIRE first-run sequence on every launch** — the theme picker, the security-notes screen, the per-project "trust this folder?" dialog, and (running with a bypass-permissions flag) a one-time warning about it, confirmed live, none of which a real, already-onboarded profile shows again. `skipFirstRunPrompts` (claude's entry only, requires `apiKeyTrustFile` since it reuses the same file) pre-seeds that same "already been through this" state: `hasCompletedOnboarding` and this session's own `projects[workingDir].hasTrustDialogAccepted` merge into the same `.claude.json` the API-key trust file already writes to, and `skipDangerousModePermissionPrompt` merges into `settings.json` (a different file, same corrupt-tolerant merge). ⚠️ **The loading banner shows the REAL backend log line, not a guess, and has no countdown or auto-timeout at all.** `getLatestLlamaSwapLogLine()` holds one `GET /api/events` SSE connection open per endpoint (confirmed live to stay open indefinitely — read past 220KB over 8s with no `done`; idle-closed after 30s via `pruneIdleLlamaSwapLogTails`, same 20s sweep as the swap-displacement check above), parsing `logData` frames and keeping only `source: "upstream"` (the real `llama-server` process's own stdout) lines, never `source: "proxy"` (llama-swap's own request-access log). ⚠️ `GET /logs` — the endpoint this feature's own first cut targeted, since the name suggested it — was confirmed live to carry ONLY the proxy log and never a single backend line, even seconds after a real, verified model swap; caught and corrected by a live check before merge, not after. The banner itself dropped its size-scaled expected-time estimate and matching auto-timeout (a guess dressed up as a fact that could kill a genuinely slow load partway through on slower hardware) for a generic hardware/model-size disclaimer plus a user-driven **Cancel** button (`_showCenterStatus`'s `onCancel` option, a real button distinct from the plain "×" close glyph an `'error'`-type banner gets) that ends the wait and closes the session on the user's own call rather than a guessed deadline. +**Everything below landed after the initial backend + picker cut, each confirmed live against a real llama-swap deployment.** ⚠️ **llama-swap runs one model at a time, and switching can disrupt ANOTHER live session** — before applying, both apply routes call llama-swap's own `GET /running` (feature-detected via `getLlamaSwapStatus()`, `custom-model-routes.ts`; a plain llama.cpp/OpenAI-compatible server has no such endpoint and is simply never checked). If a different model is loaded and ready AND another live session's own selection is using it, the apply returns `{requiresConfirmation, currentlyLoadedModel, affectedSessions}` instead of switching silently; retrying with `confirmedSwap: true` skips the check, and switching with nothing else affected proceeds immediately. ⚠️ **The swap question and the context-floor question below have SEPARATE flags** (`confirmedSwap`, `confirmedContext`), because the context check runs first and while both shared one `confirmed` a user who clicked past a too-small context silently consented to evicting another session's model too; the legacy `confirmed` still means both, since it shipped in the HTTP-API-only cut. llama-swap also has no dedicated "switch model" endpoint — the only thing that actually starts a swap is a real inference request naming the model (confirmed live: applying a selection alone never reached llama-swap's own logs, since nothing had asked it to load anything) — so both routes also fire `triggerLlamaSwapLoad()`, a fire-and-forget `POST /v1/chat/completions` with `max_tokens: 1`, whenever the target model isn't already loaded and ready. ⚠️ **That launch-time check cannot catch a swap caused by a DIFFERENT session's LATER, ordinary use** — confirmed live: a second Codex session picking a different model launched with no warning at all (nothing conflicted at that exact instant), yet it silently evicted the first session's model regardless, since llama-swap has no push notification of its own. `detectCustomModelSwapDisplacements()` (`custom-model-routes.ts`) is a separate periodic sweep (`server.ts`, `CUSTOM_MODEL_SWAP_CHECK_INTERVAL_MS` = 20s) that compares each live custom-model session's own `modelId` against what `/running` actually reports loaded, broadcasting a `custom-model:swapped-out` SSE event — shown as a global toast, never tied to the displaced session's own tab, since the whole point is telling the user before they type into it — the first time a mismatch appears, via a caller-owned de-dupe `Set` cleared once that session's own model is loaded and ready again so a later, genuinely new displacement notifies again rather than staying silently un-notified forever after the first one. ⚠️ **Context length is read from the REAL launch command, never `/props`** — `/props?model=`'s `default_generation_settings.n_ctx` was confirmed live to report a `--fit-ctx`-launched backend's theoretical/trained maximum rather than the real runtime-configured size (a measured 154112-vs-16384 discrepancy, caught only because the unfixed value still overflowed), so discovery parses the actual configured size straight out of `/running`'s own `cmd` field instead (`parseCtxFromCmd`: `--fit-ctx ` first, then plain llama.cpp `-c`/`--ctx-size`), falling back to `/props` only when `cmd` states no recognizable flag at all. ⚠️ **Claude alone gets a context-window FLOOR check, on top of the ceiling `contextLengthVar` already fixes** — `exceedsSafeContextFloor()` (gated on the registry declaring `contextLengthVar`, so a no-op for every other CLI by construction) compares a model's discovered context against `CLAUDE_MIN_SAFE_CONTEXT_TOKENS` (40000): confirmed live, twice, that Claude Code's own system prompt and tool schemas cost roughly 36.4K tokens on the very first message, before any conversation history exists to compact, so a smaller real context fails outright regardless of what `CLAUDE_CODE_MAX_CONTEXT_TOKENS` says (that var only controls when HISTORY gets compacted, and there is none yet on message one). Below the floor, the apply returns `{requiresContextWarning, modelId, contextLength, minSafeContextTokens}` instead of launching, shown as an in-app dialog naming the actual fix: give the model an explicit larger `-c`/`--ctx-size` in llama-swap's config instead of relying on `--fit-ctx` auto-fit, which optimizes for the biggest MODEL that fits rather than the biggest CONTEXT. ⚠️ **A fresh, isolated `CLAUDE_CONFIG_DIR` looks like a brand-new Claude Code profile and replays its ENTIRE first-run sequence on every launch** — the theme picker, the security-notes screen, the per-project "trust this folder?" dialog, and (running with a bypass-permissions flag) a one-time warning about it, confirmed live, none of which a real, already-onboarded profile shows again. `skipFirstRunPrompts` (claude's entry only, requires `apiKeyTrustFile` since it reuses the same file) pre-seeds that same "already been through this" state: `hasCompletedOnboarding` and this session's own `projects[workingDir].hasTrustDialogAccepted` merge into the same `.claude.json` the API-key trust file already writes to, and `skipDangerousModePermissionPrompt` merges into `settings.json` (a different file, same corrupt-tolerant merge). ⚠️ **The loading banner shows the REAL backend log line, not a guess, and has no countdown or auto-timeout at all.** `getLatestLlamaSwapLogLine()` holds one `GET /api/events` SSE connection open per endpoint (confirmed live to stay open indefinitely — read past 220KB over 8s with no `done`; idle-closed after 30s via `pruneIdleLlamaSwapLogTails`, same 20s sweep as the swap-displacement check above), parsing `logData` frames and keeping only `source: "upstream"` (the real `llama-server` process's own stdout) lines, never `source: "proxy"` (llama-swap's own request-access log). ⚠️ `GET /logs` — the endpoint this feature's own first cut targeted, since the name suggested it — was confirmed live to carry ONLY the proxy log and never a single backend line, even seconds after a real, verified model swap; caught and corrected by a live check before merge, not after. The banner itself dropped its size-scaled expected-time estimate and matching auto-timeout (a guess dressed up as a fact that could kill a genuinely slow load partway through on slower hardware) for a generic hardware/model-size disclaimer plus a user-driven **Cancel** button (`_showCenterStatus`'s `onCancel` option, a real button distinct from the plain "×" close glyph an `'error'`-type banner gets) that ends the wait and closes the session on the user's own call rather than a guessed deadline. **Run launch synchronization**: the Run entrypoint holds an in-flight lock and disables `#runBtn` for the whole launch (≥500ms), so a double click cannot create duplicate sessions with the same `w-` name. `_ensureCreatedSessionVisible()` runs before `selectSession()`, and `_onSessionCreated()` stays an idempotent upsert, so POST-first and SSE-first ordering both produce exactly one rendered tab. ⚠️ **Closing has the mirror-image race and one owner**: `closeSession()` reads `wasActive` BEFORE its `await` and announces the delete via `_closingSessions`, while `_onSessionDeleted` skips the active-session handoff for an id in that set. Both used to read `activeSessionId` after the fact, so the `session_deleted` broadcast for your own delete could null it first and closing the tab you were on landed on the welcome screen instead of the next session, on the same build, depending on timing. The fallback also picks the first order entry that is still in `sessions` (a dead id can linger in `sessionOrder`, same reason Alt+N indexes a live-filtered list). A delete from ANOTHER client still shows the welcome screen, which is the honest answer when what you were looking at was taken away. Tests: `test/session-close-fallback.test.ts`. → [architecture-invariants#run-launch-synchronization](docs/architecture-invariants.md#run-launch-synchronization) diff --git a/docs/api-reference.md b/docs/api-reference.md index 7aab6409..c85b4ff7 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -651,9 +651,17 @@ confirmed? } | { clear: true }` applies (or clears) the session's for a remote (SSH) or Docker session — both restart their agent differently under the hood, and applying to one would report success while changing nothing. Two more responses replace the normal - `{customModel, restarted}` shape, neither an error — both require - retrying the same call with `confirmed: true` to proceed anyway, and - neither restarts or creates anything on the first ask: + `{customModel, restarted}` shape, neither an error, and neither restarts + or creates anything on the first ask. ⚠️ **Each is answered by its OWN + flag on the retry, and answering one is not consent to the other**: they + are questions about different people, and while they shared a single flag + a caller who confirmed the context warning silently agreed to evict + another session's model as well. Send `confirmedContext: true` to proceed + past the context warning, `confirmedSwap: true` past the swap conflict, + and both when both were asked (they accumulate, so the second retry still + carries the first answer). The original `confirmed: true` still means + BOTH and is still accepted, because it shipped in this feature's + HTTP-API-only cut; new callers should send the specific one: - `{requiresConfirmation: true, currentlyLoadedModel, affectedSessions}` — llama.cpp/llama-swap only runs one model at a time, and switching would unload a model another **live session's own selection** is actively @@ -669,12 +677,14 @@ minSafeContextTokens}` — Claude Code's own fixed per-turn overhead `contextLengthVar` (claude only today), so it never fires for another harness. - `POST /api/v1/quick-start`'s `customModel: { endpointId, modelId, -confirmed? }` field (alongside its normal `caseName`/`mode`/etc. body) +confirmed?, confirmedContext?, confirmedSwap? }` field (alongside its +normal `caseName`/`mode`/etc. body) computes the same injection **before** the session exists and launches directly on the endpoint — no restart, because there was never a native-backend boot to restart away from. Runs the identical checks as the dedicated route above (`requiresConfirmation`/`requiresContextWarning`, - same shapes, same `confirmed: true` retry), and is refused the same way + same shapes, same per-question `confirmedContext`/`confirmedSwap` retry), + and is refused the same way for a remote or Docker case. This is what the Run-menu picker uses for opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP; Claude still uses the dedicated restart route above (its `--resume`-based restart is far less diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index b12565f1..9de5be90 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -308,7 +308,7 @@ Invariants: Copy goes through `_copyText()` (Clipboard API, then hidden-textarea + `execCommand`), not raw `navigator.clipboard`, because `install.sh`'s LAN option serves plain HTTP where `navigator.clipboard` is undefined; the fallback steals focus, so the terminal is refocused afterwards. Related: xterm registers its own `copy` listener on the terminal element gated on `hasSelection()`, which is why right-click → Copy has always worked. Selection itself is unavailable on touch devices by design (`user-select: none` on the terminal subtree), and in `shell`/`opencode`/`antigravity` tabs the TUI owns the mouse, so selecting there needs Shift+drag. Tests: `test/terminal-copy-selection.test.ts` (gate + wiring invariants), `test/terminal-copy-shortcut.test.ts` (browser, real key presses). -**The main terminal's four copy paths clean the selection first** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` in terminal-ui.js is the half that reads the live terminal). Those four are the `Ctrl+C` chord, right-click, the phone selection button and Auto Copy. ⚠️ Three routes still copy the RAW padded rows, all of them predating the clean: the browser's own Edit → Copy, which xterm's own `copy` listener on the terminal element serves with `selectionText` directly; a `copy-selection` shortcut the user disabled in App Settings, where nothing calls `preventDefault()` and that native listener runs; and the subagent/teammate windows, which build their own `Terminal` in panels-ui.js with no copy wiring at all. xterm hands back whole screen ROWS and its own trim drops only cells that were never written to, so the real spaces a full-screen TUI paints across the unused part of a row count as content and reach the clipboard. Measured against Claude Code in a 282-column pane, single lines arrived carrying 138 trailing spaces on top of the two-space transcript indent. The clean drops each line's trailing run. Two rules keep it honest: +**The main terminal's four copy paths clean the selection first** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` in terminal-ui.js is the half that reads the live terminal). Those four are the `Ctrl+C` chord, right-click, the phone selection button and Auto Copy. ⚠️ Three routes still copy the RAW padded rows, all of them predating the clean: the browser's own Edit → Copy, which xterm's own `copy` listener on the terminal element serves with `selectionText` directly; a `copy-selection` shortcut the user disabled in App Settings, where nothing calls `preventDefault()` and that native listener runs; and the subagent/teammate windows, which build their own `Terminal` in panels-ui.js with no copy wiring at all. xterm hands back whole screen ROWS and its own trim drops only cells that were never written to, so the real spaces a full-screen TUI paints across the unused part of a row count as content and reach the clipboard. Measured against Claude Code in a 282-column pane, single lines arrived carrying 138 trailing spaces on top of the two-space transcript indent. The clean drops each line's trailing run. Three rules keep it honest: 1. **Trailing padding only. A shared LEADING indent is deliberately NOT stripped**, and that is a decision rather than an omission: it was built, measured and dropped before #451 merged. It looks like the mirror image of the trailing trim and is not, because no native terminal does it and the transform cannot tell a TUI's margin from content that is genuinely indented. Measured over 401,445 three-row windows across 1,010 tracked files in this repo it fired on **73%** of them (92% inside a YAML workflow, 76% over `git log` output, 48% in a TypeScript source), and no width threshold separates the two because they are the same widths: a live Claude Code pane's own margins measure 2 and 5 columns while the most common non-TUI shared run is 4, sitting between them. The failure modes are what settle it. A wrong trailing trim costs nothing; a wrong dedent silently deletes information that was on screen, with no signal and nothing in the clipboard to hint at it, and it is wrong on `git log` bodies, on indented code read out of `cat` (semantic in Python), on `git diff` context rows where the leading space is the marker, and on stack traces. ⚠️ It also could not be made self-consistent cheaply: whether the first row joined the measurement depended on the mousedown COLUMN, which the user never sees, so one block of three rows produced three different clipboard results, and the flag read `getSelectionPosition().start`, which is the mousedown anchor xterm never normalises, so dragging UP through a block read it off the bottom row (the PR's test stub hardcoded a downward drag, so its suite could not express the case). If it is ever revisited, the one qualification that measured clean is **painted trailing padding** (a full-screen TUI writes real spaces across every row, while a shell pane leaves those cells never-written for xterm to trim): zero false positives over all 401,445 windows, no new plumbing. It still mangles a `git log` body sitting inside an agent's own gutter, which is why it was not taken. 2. **A COLUMN selection is returned untouched.** Alt+drag makes one (xterm's `shouldColumnSelect` keys on `altKey` alone, and neither `Terminal` Codeman builds passes the one option, `macOptionClickForcesSelection`, that would disable it), and a rectangle's rows lining up is the whole point of the gesture. xterm exposes the mode nowhere public, so the check reads `terminal._core._selectionService._activeSelectionMode` (`SelectionMode.COLUMN` is 3) and cleans normally if a future xterm renames it. diff --git a/docs/custom-model-endpoints.md b/docs/custom-model-endpoints.md index b4aa4d52..fecc8f8a 100644 --- a/docs/custom-model-endpoints.md +++ b/docs/custom-model-endpoints.md @@ -227,7 +227,7 @@ already pointed at the endpoint. No restart, because there was never a native-backend launch to restart away from. Runs the same llama-swap conflict check as the restart route (below) — a `409`-shaped `{requiresConfirmation, currentlyLoadedModel, affectedSessions}` response -with no session created, resolved by retrying with `confirmed: true` — and +with no session created, resolved by retrying with `confirmedSwap: true` — and is refused the same way for a remote or Docker case. This is what the Run-menu picker uses for opencode, Codex, Gemini, Pi, Grok, DeepSeek and OMP; Claude still uses the restart route below (see "The Run-menu picker" above @@ -350,7 +350,8 @@ which can take anywhere from a few seconds to well over a minute: session's own selection** is using it, the apply returns `{requiresConfirmation: true, currentlyLoadedModel, affectedSessions}` instead of silently switching — nothing is applied or created yet. - Retrying with `confirmed: true` skips the check. Switching with nothing + Retrying with `confirmedSwap: true` skips the check (the legacy `confirmed: true` + still means both questions). Switching with nothing else affected proceeds immediately; this is a warning about disrupting another session, never a gate on the switch itself. - **Actually starting the load.** llama-swap has no "switch model" admin @@ -413,7 +414,7 @@ contextLength, minSafeContextTokens}` instead of applying — nothing is restarted or created yet. A context length that was never discovered at all skips the check entirely (nothing to compare, so it fails open rather than warning on every model an endpoint hasn't reported a size for). -Retrying with `confirmed: true` launches anyway. +Retrying with `confirmedContext: true` launches anyway (the legacy `confirmed: true` still means both questions). The Run-menu picker shows this as an in-app modal (`#customModelContextWarningModal`, matching the llama-swap conflict diff --git a/src/session.ts b/src/session.ts index 0ff1ee88..fdad22f8 100644 --- a/src/session.ts +++ b/src/session.ts @@ -3758,7 +3758,18 @@ export class Session extends EventEmitter { ? null : this._mux.capturePaneText?.(this._muxSession.muxName), sendEnter: () => this._mux?.sendInput(this.id, '\r'), - glyph: () => getCli(this.mode)?.capabilities.workDetect?.promptGlyph ?? '❯', + // ⚠ NO fallback glyph here, unlike the screen-reading probe elsewhere in this file. + // Only claude and codex declare a promptGlyph; the other eight modes would fall back + // to claude's `❯`, which is ALSO starship's default shell prompt (and pure's, and + // spaceship's, and p10k lean's). On a shell session the line `❯ npm run build` sits + // on screen for as long as the command runs, promptStillInComposer() reads that as + // "still unsubmitted", and the verifier presses Enter into the running program's + // stdin on its 2s..60s schedule. Mostly a stray blank line; not harmless against a + // y/N prompt, `read -p`, an installer or a pager, where it takes the default. + // promptStillInComposer() returns undefined for an empty glyph, so this makes the + // verifier inert for every CLI that does not declare one, which is what the Claude + // Code 2.1.277 defect it exists for actually calls for. + glyph: () => getCli(this.mode)?.capabilities.workDetect?.promptGlyph ?? '', log: (m) => console.log(`[Session ${this.id.slice(0, 8)}] ${m}`), }); this._submitVerifier.arm(text); diff --git a/test/custom-model-one-shot-launch.test.ts b/test/custom-model-one-shot-launch.test.ts index c4ba8598..4ccb1c1b 100644 --- a/test/custom-model-one-shot-launch.test.ts +++ b/test/custom-model-one-shot-launch.test.ts @@ -154,7 +154,7 @@ describe('_quickStartWithCustomModelConfirm', () => { expect(app._lastCustomModelLaunchResult).toEqual(data.data); }); - it('confirming re-sends with confirmed:true and returns the second response', async () => { + it('confirming re-sends with confirmedSwap and returns the second response', async () => { const { win, app } = bootApp(); app._confirmModelSwap = async () => true; let calls = 0; @@ -170,7 +170,10 @@ describe('_quickStartWithCustomModelConfirm', () => { }, }; } - expect(body.customModel.confirmed).toBe(true); + // the SWAP question's own flag, never the blanket `confirmed`: answering this one + // must not also silence the context-floor warning. + expect(body.customModel.confirmedSwap).toBe(true); + expect(body.customModel.confirmed).toBeUndefined(); return { success: true, data: { sessionId: 's1', modelSwapInProgress: true } }; }); const data = await app._quickStartWithCustomModelConfirm({ diff --git a/test/custom-model-run-menu-ui.test.ts b/test/custom-model-run-menu-ui.test.ts index 80d0f067..693a2ae7 100644 --- a/test/custom-model-run-menu-ui.test.ts +++ b/test/custom-model-run-menu-ui.test.ts @@ -574,7 +574,7 @@ describe('Custom Model Endpoint Profiles: llama-swap model-swap confirmation and expect(confirmMessage).toContain('qwen3'); expect(applyBodies).toEqual([ { endpointId: 'llama-box', modelId: 'qwen3' }, - { endpointId: 'llama-box', modelId: 'qwen3', confirmed: true }, + { endpointId: 'llama-box', modelId: 'qwen3', confirmedSwap: true }, ]); }); @@ -1041,7 +1041,7 @@ describe("Custom Model Endpoint Profiles: requiresContextWarning (this CLI's own expect(confirmArgs).toEqual(['qwen3', 16384, 40000]); expect(applyBodies).toEqual([ { endpointId: 'llama-box', modelId: 'qwen3' }, - { endpointId: 'llama-box', modelId: 'qwen3', confirmed: true }, + { endpointId: 'llama-box', modelId: 'qwen3', confirmedContext: true }, ]); }); diff --git a/test/routes/session-custom-model.test.ts b/test/routes/session-custom-model.test.ts index 6dd25c6f..1c29618d 100644 --- a/test/routes/session-custom-model.test.ts +++ b/test/routes/session-custom-model.test.ts @@ -39,6 +39,17 @@ async function setup(ctxOptions?: Parameters[1]) } describe('POST /api/sessions/:id/custom-model', () => { + /** Shared by the conflict-check block and the context-floor block below, which needs + * both conditions true at once. Scoped to the outer describe on purpose: while it + * lived inside the conflict-check block, a sibling calling it threw a ReferenceError + * during setup, so those tests reported as failing rather than as not written. */ + function mockRunning(running: Array<{ model: string; state: string }>) { + fetchMock.mockImplementation(async (url: URL) => { + if (url.pathname === '/running') return new Response(JSON.stringify({ running }), { status: 200 }); + throw new Error(`unexpected request in this test: ${url.href}`); + }); + } + beforeEach(async () => { await writeCustomModelHosts(getDataDir(), []); fetchMock.mockReset(); @@ -229,13 +240,6 @@ describe('POST /api/sessions/:id/custom-model', () => { }); describe('llama-swap conflict check (llama.cpp runs one model at a time)', () => { - function mockRunning(running: Array<{ model: string; state: string }>) { - fetchMock.mockImplementation(async (url: URL) => { - if (url.pathname === '/running') return new Response(JSON.stringify({ running }), { status: 200 }); - throw new Error(`unexpected request in this test: ${url.href}`); - }); - } - it('applies straight away when the requested model is already loaded', async () => { const { app, ctx } = await setup(); ctx.sessions.get('test-session-1')!.mode = 'claude';