From c646613c563755809480225fee94b365b8918d64 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Sun, 1 Nov 2026 21:34:37 +0800 Subject: [PATCH] refactor(custom-model): move the real-home link into the registry recipe - linkFromRealHome/realHomeFallback on the configDir recipe; the apply consumer is generic and names no CLI - lstat guard so boot-recovery re-applies survive a dangling link; symlink is best-effort - resolve the real home from the session's own dirEnvVar override before the server's, threaded through both routes and boot recovery (Session.envOverrideValue) - deepseek links profiles/sessions/settings.yaml; boot-recovery test covers the session lookup --- docs/architecture-invariants.md | 4 +- docs/custom-model-endpoints.md | 15 ++++-- src/config/cli-registry/schema.ts | 14 +++++ src/config/cli-registry/stock.ts | 11 +++- src/config/cli-registry/types.ts | 39 +++++++++----- src/custom-model-injection-apply.ts | 76 ++++++++++++++++++++++----- src/custom-model-injection.ts | 13 ++++- src/session.ts | 11 ++++ src/web/routes/session-routes.ts | 6 ++- src/web/server.ts | 8 ++- test/custom-model-recovery.test.ts | 70 ++++++++++++++++++++++++ test/deepseek-custom-endpoint.test.ts | 32 +++++------ test/mocks/mock-session.ts | 3 ++ 13 files changed, 248 insertions(+), 54 deletions(-) create mode 100644 test/custom-model-recovery.test.ts diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index ebba7973..e85826a4 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -628,13 +628,13 @@ Adding the header changed the preamble, so `CODEMAN_PREAMBLE` was bumped (1.22.0 **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. +⚠️ The actual per-CLI redirect is `capabilities.customModelInjection` on the CLI registry (four kinds: `env` for claude/gemini/copilot, `configContentEnv` reusing opencode's existing `OPENCODE_CONFIG_CONTENT`, `configDir` for codex/pi/grok/omp/deepseek — writes an isolated per-session config file, NEVER the user's real `~/.codex`/`~/.pi`/`~/.omp`/grok config/dsh home; deepseek's redirect is a per-session `DSH_HOME` holding only the generated `cordis.patch.yml`, whose `linkFromRealHome` recipe field symlinks `profiles`/`sessions`/`settings.yaml` back out of the real dsh home (resolved session-`DSH_HOME`-first, lstat-guarded so a dangling link survives re-apply) instead of stranding them — 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**: 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()` relaunches a CLI in an existing pane, so a CLI whose launch declares a `fallback` chain (claude) gets a conversation 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. The dead-pane respawn shares this pin, see [Dead-pane respawn: the resume pin](#dead-pane-respawn-the-resume-pin). ⚠️ 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. +**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), live against llama-swap (`.../chat/completions` 404s, `.../v1/chat/completions` succeeds) — and the env-var redirect that first fixed it has since been REPLACED by a `configDir` recipe: a per-session `DSH_HOME` carrying only a generated `cordis.patch.yml` that declares a `codeman-custom` `openai-completions` provider over the installed profile, with the `/v1` fix living in `renderConfigFile`'s shared `withV1Suffix()` (every file template applies it; the `env` kind's `appendV1Suffix` flag now belongs to copilot alone) and the session's state symlinked back from the real dsh home; a real "hello world" reply through an actual `dsh` binary is still outstanding before calling this fully verified like the harnesses above; 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). diff --git a/docs/custom-model-endpoints.md b/docs/custom-model-endpoints.md index f76c21a1..74130e31 100644 --- a/docs/custom-model-endpoints.md +++ b/docs/custom-model-endpoints.md @@ -541,11 +541,16 @@ for '' not found. Defaulting to fallback metadata...` on launch — `POST /chat/completions` 404s against llama-swap while `POST /v1/chat/completions` succeeds; the harness's own error template (`DeepSeek API error (HTTP ${status})`) matches the originally - reported symptom exactly. `customModelInjection`'s new `appendV1Suffix` - (deepseek's entry only — claude/gemini must NOT get it, since claude was - already confirmed working against the raw `baseUrl`) fixes it by writing - `DEEPSEEK_BASE_URL` with `/v1` appended. Not yet re-run end-to-end with a - real `dsh` binary (no install available in this environment) — the fix + reported symptom exactly. The fix is no longer the original env redirect: + deepseek's `customModelInjection` is a `configDir` recipe — a per-session + `DSH_HOME` holding only a generated `cordis.patch.yml` that declares a + `codeman-custom` `openai-completions` provider (the base URL gets the `/v1` + suffix from the shared `renderConfigFile` step every file template goes + through), so the installed profile keeps supplying the transport; the + recipe's `linkFromRealHome` field symlinks `profiles`, `sessions` and + `settings.yaml` back out of the real dsh home so installed bundles, + transcripts and user settings all survive the redirect. Not yet re-run + end-to-end with a real `dsh` binary — the fix is source-confirmed and live-verified at the HTTP level, but a real "hello world" reply through `dsh` itself is still outstanding before calling this fully verified like the harnesses above. diff --git a/src/config/cli-registry/schema.ts b/src/config/cli-registry/schema.ts index 5c65279e..dfa7e3f3 100644 --- a/src/config/cli-registry/schema.ts +++ b/src/config/cli-registry/schema.ts @@ -286,6 +286,14 @@ const launchModelTemplate = z .regex(/^[a-zA-Z0-9._\-/:{}]+$/) .optional(); +/** One entry name under a CLI's real home, for `linkFromRealHome`: single path segment, + * leading alphanumeric, no traversal or hidden-dot escape. */ +const homeSegmentName = z + .string() + .min(1) + .max(40) + .regex(/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/); + const capabilitiesSchema = z .object({ external: z.boolean(), @@ -582,6 +590,12 @@ const capabilitiesSchema = z fileName: z.string().min(1).max(80), template: z.enum(['codex-toml', 'pi-models-json', 'omp-models-yml', 'grok-toml', 'deepseek-patch-json']), launchModel: launchModelTemplate, + // State that must survive the isolated-directory redirect, symlinked by name from + // the CLI's real home (deepseek: profiles/sessions/settings.yaml). Single path + // segments only — the consumer joins them, so a traversal here would escape the + // real home. See the field's comment in cli-registry/types.ts. + linkFromRealHome: z.array(homeSegmentName).max(8).optional(), + realHomeFallback: z.string().min(1).max(80).optional(), }) .strict(), z.object({ kind: z.literal('unsupported') }).strict(), diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index ade27249..5ce80be8 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -1553,6 +1553,13 @@ const DEEPSEEK: CliEntry = { dirEnvVar: 'DSH_HOME', fileName: 'cordis.patch.yml', template: 'deepseek-patch-json', + // dsh keeps state in its home that the redirect must NOT swallow: `sessions` is the + // session-log root (deleting it would take the user's transcripts with it on clear/ + // close), `profiles` are the installed bundles the patch composes over, and + // `settings.yaml` must keep applying. The generated home-level `cordis.patch.yml` + // deliberately replaces the user's own for the session rather than linking it. + linkFromRealHome: ['profiles', 'sessions', 'settings.yaml'], + realHomeFallback: '.dsh', }, }, overlays: { @@ -1905,8 +1912,8 @@ const COPILOT: CliEntry = { // custom provider and drops the GitHub sign-in requirement; COPILOT_PROVIDER_TYPE defaults to // "openai", which is what a Codeman custom endpoint is (llama.cpp, vLLM, Ollama). // COPILOT_MODEL names the model and COPILOT_PROVIDER_MODEL_ID / _WIRE_MODEL default to it. - // The OpenAI-style route is `/v1/chat/completions`, so the base gets the `/v1` suffix - // the same way deepseek's does. All four already match the COPILOT_ prefix allowlist, so + // The OpenAI-style route is `/v1/chat/completions`, so the base gets the `/v1` + // suffix via appendV1Suffix below. All four already match the COPILOT_ prefix allowlist, so // they are privileged env keys below: a non-granted multi-user owner must not be able to // redirect the session's endpoint or credentials through plain envOverrides. customModelInjection: { diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 1777eccc..f5ca60f7 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -856,19 +856,13 @@ export interface CliCapabilities { * reuses that file. * * `appendV1Suffix` (env kind only): the raw `endpoint.baseUrl` gets `withV1Suffix()` - * applied before being written to `baseUrlVar`, instead of being used verbatim. - * DeepSeek needs this and claude/gemini must NOT get it — a per-CLI asymmetry confirmed - * by reading each SDK's own request-building source, not assumed: DeepSeek Harness's - * bundled `@deepseek-ai/dsh-llm-deepseek` concatenates `${connection.baseURL}/chat/ - * completions` with no `/v1` insertion of its own (its real public API base, - * `https://api.deepseek.com`, expects the caller's base URL to already carry any - * needed prefix), while llama-swap/llama.cpp only ever serves the OpenAI-conventional - * `/v1/chat/completions` — confirmed live: a bare `POST /chat/completions` - * 404s, `POST /v1/chat/completions` succeeds, and the harness's own error - * message template (`DeepSeek API error (HTTP ${status})`) reproduces the exact - * `HTTP_404` this feature originally shipped with unexplained. Claude Code's own SDK, - * by contrast, was already confirmed working end-to-end against the RAW `baseUrl` with - * no suffix — appending one there would be wrong, not just redundant. + * applied before being written to `baseUrlVar`, instead of being used verbatim. Only + * copilot declares it — its OpenAI-style route is `/v1/chat/completions`, and a + * gateway saves the bare root. claude/gemini must NOT get it: Claude Code's own SDK was + * confirmed working end-to-end against the RAW `baseUrl`, so appending a suffix there + * would be wrong, not just redundant. The per-session config-file recipes do not need + * this flag at all — `renderConfigFile()` applies the suffix inside every template + * (deepseek's patch included), so no configDir entry carries a raw base URL. */ customModelInjection: | { @@ -890,6 +884,25 @@ export interface CliCapabilities { fileName: string; template: 'codex-toml' | 'pi-models-json' | 'omp-models-yml' | 'grok-toml' | 'deepseek-patch-json'; launchModel?: string; + /** + * Names of entries under this CLI's REAL home that must be symlinked into the + * isolated directory after the generated file is written — the same trick + * `linkSharedProjectsDir()` plays for claude's `projects`. Declared for CLIs whose + * home carries state that must survive the redirect (deepseek's `sessions` transcript + * root and `profiles` bundles, plus `settings.yaml`, which would otherwise stop + * applying and — for `sessions` — get deleted by `removeConfigDir` on clear/close). + * Never include `fileName` itself: the generated file owns that name in the isolated + * directory, and a link there would write THROUGH into the user's real file. + * Consumed generically by custom-model-injection-apply.ts. + */ + linkFromRealHome?: string[]; + /** + * Fallback location of the CLI's real home, relative to the user's home directory + * (deepseek's `.dsh`), used only when neither the session's own `dirEnvVar` override + * nor the server's environment sets it. Required whenever `linkFromRealHome` is + * declared, so no consumer has to hardcode a per-CLI default of its own. + */ + realHomeFallback?: string; } | { kind: 'unsupported' }; } diff --git a/src/custom-model-injection-apply.ts b/src/custom-model-injection-apply.ts index ad6d9b9f..3ca067b0 100644 --- a/src/custom-model-injection-apply.ts +++ b/src/custom-model-injection-apply.ts @@ -10,7 +10,7 @@ * cli-registry changes" requirement it was written against. */ -import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync, rmSync, symlinkSync } from 'node:fs'; +import { chmodSync, existsSync, lstatSync, mkdirSync, readFileSync, writeFileSync, rmSync, symlinkSync } from 'node:fs'; import { homedir, platform } from 'node:os'; import { join, dirname } from 'node:path'; import { dataPath } from './config/instance.js'; @@ -39,24 +39,70 @@ export function customModelConfigDir(sessionId: string): string { * covers a re-apply onto a file that already exists (`mode` only applies at * creation). */ -export function applyConfigDirInjection(baseDir: string, injection: ConfigDirInjection): Record { +export function applyConfigDirInjection( + baseDir: string, + injection: ConfigDirInjection, + /** + * The value this SESSION's own env overrides set for `injection.dirEnvVar`, when it + * set one — a user who relocated the CLI's real home (an allowlisted key like + * DSH_HOME) must have their state linked from THAT directory, not the server's. + */ + sessionHomeOverride?: string +): Record { for (const file of injection.files) { const filePath = join(baseDir, file.relPath); mkdirSync(dirname(filePath), { recursive: true, mode: 0o700 }); writeFileSync(filePath, file.content, { encoding: 'utf8', mode: 0o600 }); chmodSync(filePath, 0o600); } - if (injection.dirEnvVar === 'DSH_HOME') { - // Reuse installed profile bundles without changing the user's profile or credentials. - const profiles = join(baseDir, 'profiles'); - if (!existsSync(profiles)) { - const realHome = process.env.DSH_HOME?.trim() || join(homedir(), '.dsh'); - symlinkSync(join(realHome, 'profiles'), profiles, platform() === 'win32' ? 'junction' : 'dir'); - } - } + linkRealHomeEntries(baseDir, injection, sessionHomeOverride); return { [injection.dirEnvVar]: baseDir, ...injection.extraEnv }; } +/** + * Symlinks the registry recipe's `linkFromRealHome` entries out of the CLI's real home + * into the otherwise-isolated `baseDir`, so redirecting `dirEnvVar` moves only the + * generated config file and not the state that has to survive the session (deepseek: + * the `sessions` transcript root `removeConfigDir` would otherwise delete, the `profiles` + * bundles the patch composes over, and `settings.yaml`). Declared per entry in stock.ts — + * this consumer is recipe-driven and names no CLI itself. + * + * Home resolution: the session's own override first (unless it IS `baseDir` — a re-apply + * onto a session already holding a previous injection's value must not self-link), then + * the server's environment, then the recipe's `realHomeFallback`. No home resolvable = + * no links, same best-effort posture as `linkSharedProjectsDir()`. + */ +function linkRealHomeEntries(baseDir: string, injection: ConfigDirInjection, sessionHomeOverride?: string): void { + const names = injection.linkFromRealHome; + if (!names || names.length === 0) return; + const fromSession = sessionHomeOverride?.trim(); + const realHome = + (fromSession && fromSession !== baseDir ? fromSession : undefined) || + process.env[injection.dirEnvVar]?.trim() || + (injection.realHomeFallback ? join(homedir(), injection.realHomeFallback) : undefined); + if (!realHome) return; + for (const name of names) { + linkRealHomeEntry(join(realHome, name), join(baseDir, name)); + } +} + +function linkRealHomeEntry(target: string, link: string): void { + // lstat, never existsSync: existsSync is false for a DANGLING link, and recovery + // re-applies on every restart, so the second pass would try to recreate the link and + // throw EEXIST. lstat answers for real entries, good links and dangling links alike. + try { + if (lstatSync(link)) return; // already linked (idempotent re-apply) or a real dir wrote one + } catch { + // ENOENT — nothing is here yet, the only state in which a link gets created + } + try { + symlinkSync(target, link, platform() === 'win32' ? 'junction' : 'dir'); + } catch { + // best-effort only — a platform that refuses symlinks keeps the documented + // side effect for this session rather than failing the whole apply over it + } +} + /** * Real, shared Claude config directory Codeman's own host process runs under — honors * `CLAUDE_CONFIG_DIR` the same way `claude-credentials.ts`'s `claudeCredentialsPath()` @@ -258,7 +304,13 @@ export function applyCustomModelInjection( * trust-dialog seed, and only when provided (boot recovery, which has no reason to * re-answer a dialog that already fired once, omits it rather than re-deriving it). */ - workingDir?: string + workingDir?: string, + /** + * Reads one env var from the SESSION's own env overrides (single-key on purpose: the + * map carries credentials). Only the `configDir` kind uses it, to resolve the CLI's real + * home for `linkFromRealHome` from the session's value before the server's. + */ + sessionEnvLookup?: (envVar: string) => string | undefined ): AppliedCustomModel | undefined { const injection = buildCustomModelInjection(entry, endpoint, modelId, contextLength); if (injection.kind === 'unsupported') return undefined; @@ -291,6 +343,6 @@ export function applyCustomModelInjection( }; } const configDir = customModelConfigDir(sessionId); - const envOverrides = applyConfigDirInjection(configDir, injection); + const envOverrides = applyConfigDirInjection(configDir, injection, sessionEnvLookup?.(injection.dirEnvVar)); return { envOverrides, envKeys: Object.keys(envOverrides), configDir, launchModel: injection.launchModel }; } diff --git a/src/custom-model-injection.ts b/src/custom-model-injection.ts index 5505ad72..24aface2 100644 --- a/src/custom-model-injection.ts +++ b/src/custom-model-injection.ts @@ -87,6 +87,10 @@ export interface ConfigDirInjection { * `customModelInjection.launchModel` template, never hand-built per CLI. */ launchModel?: string; + /** Carried from the registry recipe — see `cli-registry/types.ts` `linkFromRealHome`. */ + linkFromRealHome?: string[]; + /** Carried from the registry recipe — see `cli-registry/types.ts` `realHomeFallback`. */ + realHomeFallback?: string; } export interface UnsupportedInjection { @@ -143,7 +147,14 @@ export function buildCustomModelInjection( case 'configDir': { const { content, extraEnv } = renderConfigFile(cap.template, endpoint, modelId, apiKey, contextLength); return withLaunchModel( - { kind: 'configDir', dirEnvVar: cap.dirEnvVar, files: [{ relPath: cap.fileName, content }], extraEnv }, + { + kind: 'configDir', + dirEnvVar: cap.dirEnvVar, + files: [{ relPath: cap.fileName, content }], + extraEnv, + ...(cap.linkFromRealHome ? { linkFromRealHome: cap.linkFromRealHome } : {}), + ...(cap.realHomeFallback ? { realHomeFallback: cap.realHomeFallback } : {}), + }, cap.launchModel, modelId ); diff --git a/src/session.ts b/src/session.ts index a2b49565..33373973 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1330,6 +1330,17 @@ export class Session extends EventEmitter { return value && value.trim() ? value.trim() : undefined; } + /** + * One env var from this session's overrides, for internal path resolution only + * (custom-model injection linking an isolated config dir back to a path that must + * survive lives in). Single-key lookup by design, like `deepSeekHomeOverride`: the + * map can hold provider credentials and stays off the public `SessionState`. + */ + envOverrideValue(key: string): string | undefined { + const value = this._envOverrides?.[key]; + return value && value.trim() ? value.trim() : undefined; + } + /** Owning username in multi-user mode, else undefined. */ get owner(): string | undefined { return this._owner; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 0eb80b82..9afd5b18 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1618,7 +1618,8 @@ export function registerSessionRoutes( body.modelId, session.id, contextLength, - session.workingDir + session.workingDir, + (envVar) => session.envOverrideValue(envVar) ); if (!applied) { return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${session.mode} has no known custom-model mechanism`); @@ -4514,7 +4515,8 @@ export function registerSessionRoutes( customModel.modelId, qsCustomModelSessionId, cmContextLength, - workspacePath + workspacePath, + (envVar) => qsGatedEnvOverrides?.[envVar] ); if (!cmApplied) { return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${mode} has no known custom-model mechanism`); diff --git a/src/web/server.ts b/src/web/server.ts index ddfc296d..b26b864e 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1470,7 +1470,13 @@ export class WebServer extends EventEmitter { endpoint, saved.modelId, session.id, - endpoint.modelContextLengths?.[saved.modelId] + endpoint.modelContextLengths?.[saved.modelId], + // Boot recovery deliberately omits `workingDir` (no reason to re-answer a trust + // dialog that already fired once), so the session-env lookup rides in after it: + // a session with its own DSH_HOME override must link its config dir back to + // THAT home, not the server's, exactly like the routes do. + undefined, + (envVar) => session.envOverrideValue(envVar) )?.envOverrides; } catch (err) { console.warn('[WebServer] Failed to rebuild custom-model env on recovery:', err); diff --git a/test/custom-model-recovery.test.ts b/test/custom-model-recovery.test.ts new file mode 100644 index 00000000..cb9c54e1 --- /dev/null +++ b/test/custom-model-recovery.test.ts @@ -0,0 +1,70 @@ +/** + * @fileoverview Boot recovery re-derives a saved custom-model selection through + * `_rebuildCustomModelEnv`, and the `linkFromRealHome` recipe must resolve the CLI's + * real home from the SESSION's own env overrides first: the user who relocated dsh + * (allowlisted `DSH_HOME`) runs their CLI against THAT home, and recovery must link the + * regenerated config dir back to it, not to whatever the server's environment says. + * + * `_rebuildCustomModelEnv` reads no `this` state (it re-derives everything from the endpoint + * store and the Session it is handed), so it is driven straight off the prototype with an + * empty receiver — `restoreMuxSessions()` itself cannot be reached under vitest, the same + * constraint recorded in test/session-model-recovery.test.ts. + */ +import { mkdirSync, realpathSync, rmSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { getDataDir } from '../src/config/instance.js'; +import { writeCustomModelHosts, type CustomModelHost } from '../src/custom-model-hosts.js'; +import { customModelConfigDir, removeConfigDir } from '../src/custom-model-injection-apply.js'; +import { Session } from '../src/session.js'; +import { WebServer } from '../src/web/server.js'; +import type { CustomModelBookkeeping } from '../src/types/session.js'; + +type RebuildFn = (session: Session, saved: CustomModelBookkeeping) => Promise | undefined>; + +/** The real method, with an empty receiver — see the file header for why that is sound. */ +function rebuildCustomModelEnv(session: Session, saved: CustomModelBookkeeping) { + const fn = (WebServer.prototype as unknown as { _rebuildCustomModelEnv: RebuildFn })._rebuildCustomModelEnv; + expect(typeof fn, '_rebuildCustomModelEnv vanished from WebServer.prototype').toBe('function'); + return fn.call({}, session, saved); +} + +function endpoint(): CustomModelHost { + return { + id: 'ep', + label: 'Ep', + baseUrl: 'http://localhost:8080', + apiKey: 'recovery-probe-key', + models: ['qwen3'], + }; +} + +describe('_rebuildCustomModelEnv: real-home resolution for linkFromRealHome', () => { + const configs: string[] = []; + + afterEach(() => { + for (const id of configs.splice(0)) removeConfigDir(customModelConfigDir(id)); + vi.unstubAllEnvs(); + }); + + it('links an isolated deepseek home from the SESSION’s own DSH_HOME, not the server’s', async () => { + // The user who relocated dsh runs their CLI against THAT home; recovery must re-link + // the session's generated config dir back to it even though the value never persists + // into the server's environment. The server's DSH_HOME is emptied deliberately: with + // the session lookup working, the fallback home is never consulted, and with it + // broken this test gets a link into ~/.dsh rather than a pass. + const sessionHome = join(homedir(), 'codeman-recovery-dsh-home'); + mkdirSync(join(sessionHome, 'profiles'), { recursive: true }); + vi.stubEnv('DSH_HOME', ''); + await writeCustomModelHosts(getDataDir(), [endpoint()]); + const session = new Session({ workingDir: '/tmp', mode: 'deepseek', envOverrides: { DSH_HOME: sessionHome } }); + configs.push(session.id); + + const env = await rebuildCustomModelEnv(session, { endpointId: 'ep', modelId: 'qwen3', envKeys: [] }); + + expect(env?.DSH_HOME).toBe(customModelConfigDir(session.id)); + expect(realpathSync(join(env!.DSH_HOME, 'profiles'))).toBe(join(sessionHome, 'profiles')); + rmSync(sessionHome, { recursive: true, force: true }); + }); +}); diff --git a/test/deepseek-custom-endpoint.test.ts b/test/deepseek-custom-endpoint.test.ts index 7bb63408..705e263e 100644 --- a/test/deepseek-custom-endpoint.test.ts +++ b/test/deepseek-custom-endpoint.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, readFileSync, realpathSync, writeFileSync } from 'node:fs'; +import { mkdirSync, readFileSync, readlinkSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -20,6 +20,8 @@ describe('DeepSeek custom endpoint isolation', () => { const realHome = join(homedir(), '.dsh'); const profile = join(realHome, 'profiles', 'dsh-tui'); mkdirSync(profile, { recursive: true }); + mkdirSync(join(realHome, 'sessions'), { recursive: true }); + writeFileSync(join(realHome, 'settings.yaml'), 'theme: dark'); const original = join(profile, 'cordis.patch.yml'); writeFileSync(original, 'original profile'); vi.stubEnv('DSH_HOME', realHome); @@ -43,22 +45,20 @@ describe('DeepSeek custom endpoint isolation', () => { model: 'chosen-model', }); expect(realpathSync(join(applied.configDir, 'profiles'))).toBe(join(realHome, 'profiles')); + // The rest of the linkFromRealHome list, consumed generically: `sessions` is the + // transcript root removeConfigDir would otherwise delete, and the user's settings.yaml + // must keep applying through the redirect. + expect(realpathSync(join(applied.configDir, 'sessions'))).toBe(join(realHome, 'sessions')); + expect(realpathSync(join(applied.configDir, 'settings.yaml'))).toBe(join(realHome, 'settings.yaml')); + // Re-apply onto the same config dir (boot recovery re-runs this every restart) with one + // link dangling: the lstat guard must skip every existing link, broken ones included, + // which the old existsSync check missed — recreating them would raise EEXIST. + rmSync(join(realHome, 'sessions'), { recursive: true, force: true }); + const reapplied = applyCustomModelInjection(entry, endpoint, 'chosen-model', 'deepseek-probe', 131072); + expect(reapplied?.envOverrides.DSH_HOME).toBe(applied.configDir); + expect(readlinkSync(join(applied.configDir, 'sessions'))).toBe(join(realHome, 'sessions')); + expect(readFileSync(original, 'utf8')).toBe('original profile'); removeConfigDir(applied.configDir); expect(readFileSync(original, 'utf8')).toBe('original profile'); }); - - it('gives Codex its known context and compaction budget', () => { - const entry = getCli('codex'); - if (!entry) throw new Error('Missing Codex entry'); - const injection = buildCustomModelInjection( - entry, - { id: 'local', label: 'Local', baseUrl: 'http://box' }, - 'chosen-model', - 131072 - ); - if (injection.kind !== 'configDir') throw new Error('Expected isolated Codex config'); - expect(injection.files[0].content).toContain('model_context_window = 131072'); - expect(injection.files[0].content).toContain('model_auto_compact_token_limit = 98304'); - expect(injection.launchModel).toBe('chosen-model'); - }); }); diff --git a/test/mocks/mock-session.ts b/test/mocks/mock-session.ts index 3be966a2..7ce1e0c8 100644 --- a/test/mocks/mock-session.ts +++ b/test/mocks/mock-session.ts @@ -454,6 +454,9 @@ export class MockSession extends EventEmitter { } ); restartCli = vi.fn(async () => true); + /** Mirrors Session.envOverrideValue; no override map here, so the custom-model + * real-home lookup defaults to "session says nothing" (overridable per test). */ + envOverrideValue = vi.fn((_key: string): string | undefined => undefined); getCustomModelForPersist = vi.fn(() => this._mockCustomModel); /** Mirrors Session.noteReportedModel: the statusline route records the CLI's model. */ reportedModel: { model: string; source: 'statusline' | 'screen' } | null = null;