From a6516169076efa59a0cc1c3594fcd46f665f4bfe Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 6 Nov 2026 12:41:32 +0100 Subject: [PATCH] fix(custom-model): keep the launch model out of the stored config Quick-start wrote a selection's launchModel into the session's stored Config, which is what a clear falls back to, so a clear relaunched the CLI with the endpoint's model id on its native backend. Only API callers can reach that, since no page sends a clear. The first launch now takes the forced model from the session's selection, through the same _withCustomModelLaunchModel() the respawn path uses, so the stored config keeps the model chosen at create. That also carries the model onto a new tmux session for a selection applied to a running session, which the create path used to lose. Co-Authored-By: Claude Opus 5.5 --- docs/architecture-invariants.md | 2 +- docs/custom-model-endpoints.md | 5 +++ src/session.ts | 16 +++---- src/web/routes/session-routes.ts | 29 +++--------- test/routes/quick-start-custom-model.test.ts | 28 ++++++++++++ test/session-custom-model-restart.test.ts | 47 ++++++++++++++++++++ 6 files changed, 95 insertions(+), 32 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 4c682b3e..5d67547d 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -662,7 +662,7 @@ Adding the header changed the preamble, so `CODEMAN_PREAMBLE` was bumped (1.22.0 ⚠️ 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. It also puts back what the session had set itself under an injected key (`CustomModelBookkeeping.ownEnv`, read after the undo so a switch carries it across): an own `DSH_HOME` or `CLAUDE_CONFIG_DIR` used to be deleted with the injected value, which moved the session to the server's tree or the default Claude account. ⚠️ `ownEnv` can hold a credential in memory, so `getCustomModelForPersist()` writes only its `homeEnvVar` entry, and only as an absolute path (the registry types that name as any env var, so a `clis.json` override can point it at a credential), which it must write because the selection overwrote the tmux `setenv` that was the only other copy. `_restoreCustomModel()` always states `ownEnv` (empty for an older record) after `clampEnvOverridesForOwner()`: after a restart `_envOverrides` holds what the previous run INJECTED, and reading that as the session's own would pin it to a directory the clear deletes. The re-apply's real-home lookup has the same rule: the route reads `Session.ownEnvValue()` and recovery that clamped record (probed under the bulk cap, since it walks every saved session), never `envOverrideValue()`, so an entry the own home lacks is skipped rather than filled from the server's home. Quick-start hands the `Session` constructor the caller's overrides alone so `setCustomModel()` can see them. ⚠️ 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 pin looks in `/projects`, so `linkSharedProjectsDir()` links it from the session's own `CLAUDE_CONFIG_DIR` when it has one (`guardSessionConfigDirHome()` covers `configDirVar` too): the server's tree would hide the conversation on apply and leave the endpoint's transcript behind on clear. 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), and codex gets one too (`--model `, its pattern changed to `model-plain-colon`, which admits qwen3-style colon ids and requires a letter or digit first, for the native App Settings Codex model too): 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, `homeEnvVar` and the session's own home path; never an injected value, which carries the API key and is re-derived from the endpoint store on recovery, and never any other own value), 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. +⚠️ **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. It also puts back what the session had set itself under an injected key (`CustomModelBookkeeping.ownEnv`, read after the undo so a switch carries it across): an own `DSH_HOME` or `CLAUDE_CONFIG_DIR` used to be deleted with the injected value, which moved the session to the server's tree or the default Claude account. ⚠️ `ownEnv` can hold a credential in memory, so `getCustomModelForPersist()` writes only its `homeEnvVar` entry, and only as an absolute path (the registry types that name as any env var, so a `clis.json` override can point it at a credential), which it must write because the selection overwrote the tmux `setenv` that was the only other copy. `_restoreCustomModel()` always states `ownEnv` (empty for an older record) after `clampEnvOverridesForOwner()`: after a restart `_envOverrides` holds what the previous run INJECTED, and reading that as the session's own would pin it to a directory the clear deletes. The re-apply's real-home lookup has the same rule: the route reads `Session.ownEnvValue()` and recovery that clamped record (probed under the bulk cap, since it walks every saved session), never `envOverrideValue()`, so an entry the own home lacks is skipped rather than filled from the server's home. Quick-start hands the `Session` constructor the caller's overrides alone so `setCustomModel()` can see them. ⚠️ 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 pin looks in `/projects`, so `linkSharedProjectsDir()` links it from the session's own `CLAUDE_CONFIG_DIR` when it has one (`guardSessionConfigDirHome()` covers `configDirVar` too): the server's tree would hide the conversation on apply and leave the endpoint's transcript behind on clear. 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), and codex gets one too (`--model `, its pattern changed to `model-plain-colon`, which admits qwen3-style colon ids and requires a letter or digit first, for the native App Settings Codex model too): that is the registry's `customModelInjection.launchModel` template, applied onto the create and respawn options through `legacyConfigField` by `_withCustomModelLaunchModel()`, never by id and never onto the stored `Config` (that config is what a clear falls back to; the quick-start route used to write the launch model there, so a clear relaunched the endpoint's id on the native backend), 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, `homeEnvVar` and the session's own home path; never an injected value, which carries the API key and is re-derived from the endpoint store on recovery, and never any other own value), 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`. diff --git a/docs/custom-model-endpoints.md b/docs/custom-model-endpoints.md index dbc38ae6..78001ed4 100644 --- a/docs/custom-model-endpoints.md +++ b/docs/custom-model-endpoints.md @@ -543,6 +543,11 @@ selection goes, on a clear or a switch to another endpoint. That is what keeps a session on its own `DSH_HOME`, `CODEX_HOME` or `GROK_HOME`, and on its own account through `CLAUDE_CONFIG_DIR` (the per-client-account case). +The launch model goes the same way: a clear relaunches on the model the session +was created with, or with none if it was created without one. A session an +older build launched straight onto an endpoint saved that endpoint's model in +its config, so its clear still relaunches on that id. + ⚠️ Across a Codeman restart only the home directory comes back, and only as an absolute path. The selection overwrote the tmux `setenv` that held it, so its record in `state.json` keeps that one path (`ownEnv`), and a non-granted diff --git a/src/session.ts b/src/session.ts index 7a29f62d..4b84fb5d 100644 --- a/src/session.ts +++ b/src/session.ts @@ -2990,13 +2990,13 @@ export class Session extends EventEmitter { * launch param. Where that param * lives is registry DATA — the entry's `legacyConfigField` (`piConfig`, `grokConfig`, * ...) or the top-level `model` for an entry that declares none — so this stays a - * generic reader rather than a branch per CLI. Applied on the OPTIONS only: the stored - * `Config` keeps whatever model the user chose at create, which is exactly what a - * later clear must fall back to. + * generic reader rather than a branch per CLI. Applied on the OPTIONS only, on both the + * respawn and the create path: the stored `Config` keeps whatever model the user + * chose at create, which is exactly what a later clear must fall back to. */ - private _withCustomModelLaunchModel( - options: import('./mux-interface.js').RespawnPaneOptions - ): import('./mux-interface.js').RespawnPaneOptions { + private _withCustomModelLaunchModel< + T extends import('./mux-interface.js').RespawnPaneOptions | import('./mux-interface.js').CreateSessionOptions, + >(options: T): T { const launchModel = this._customModel?.launchModel; if (!launchModel) return options; const entry = getCli(this.mode); @@ -3348,7 +3348,7 @@ export class Session extends EventEmitter { const { isRestored, respawnedResumeId, respawnedDeadPane } = await this._setupOrAttachMuxSession({ // Single source of truth shared with reattachRemote() (COD-108). respawnPaneOptions: this._buildRespawnPaneOptions(), - createSessionOptions: { + createSessionOptions: this._withCustomModelLaunchModel({ sessionId: this.id, workingDir: this.workingDir, mode: this.mode, @@ -3378,7 +3378,7 @@ export class Session extends EventEmitter { docker: this._docker, adopt: this._adopt, owner: this._owner, - }, + }), spawnErrLabel: 'mux attachment', }); diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 6ece509c..b7ba3f41 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -4750,7 +4750,6 @@ export function registerSessionRoutes( // would hide them, and giving setCustomModel() that set would put // CLAUDE_CODE_EFFORT_LEVEL back after the constructor stripped it. let qsCustomModelAppliedEnvOverrides: Record | undefined; - let qsCustomModelLaunchModel: string | undefined; let qsCustomModelSessionId: string | undefined; let qsCustomModelSwapInProgress = false; let qsCustomModelBookkeeping: @@ -4866,7 +4865,6 @@ export function registerSessionRoutes( return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${mode} has no known custom-model mechanism`); } qsCustomModelAppliedEnvOverrides = cmApplied.envOverrides; - qsCustomModelLaunchModel = cmApplied.launchModel; qsCustomModelBookkeeping = { endpointId: cmEndpoint.id, modelId: customModel.modelId, @@ -4916,30 +4914,15 @@ export function registerSessionRoutes( tmuxHistoryLimit: qsTerminalHistoryConfig.tmuxHistoryLimit, parentSessionId: qsParentSessionId, }; - // Force the custom-model selection's launchModel (pi/omp `custom/`, grok's - // `[model.]` block name) onto whichever config field the registry says the - // CLI's `model` launch param lives in — mirrors Session._withCustomModelLaunchModel, - // which the restart-in-place path already uses, rather than a hardcoded per-CLI - // branch here that a CLI landing its injection recipe later would silently miss. - if (qsCustomModelLaunchModel !== undefined) { - const qsCustomModelField = getCli(mode)?.launch.legacyConfigField; - if (qsCustomModelField) { - const qsSessionOptionsBag = qsSessionOptions as unknown as Record; - qsSessionOptionsBag[qsCustomModelField] = { - ...((qsSessionOptionsBag[qsCustomModelField] as Record) ?? {}), - model: qsCustomModelLaunchModel, - }; - } else { - qsSessionOptions.model = qsCustomModelLaunchModel; - } - } const session = new Session(qsSessionOptions); // Records the selection for session.customModel/getCustomModelForPersist() and future - // clear/switch calls, and merges the injected env, which the launch below reads along - // with the launch-model config set above (piConfig/grokConfig/ompConfig.model). Nothing - // has started yet, so this is never a restart: setCustomModel() is synchronous state, - // no tmux IO of its own (see its own doc comment in session.ts). + // clear/switch calls, and merges the injected env. The launch below reads both: the + // selection's launchModel is forced onto the launch options, never onto the stored + // config above, because that config is what a later clear falls back to + // (Session._withCustomModelLaunchModel). Nothing has started yet, so this is never a + // restart: setCustomModel() is synchronous state, no tmux IO of its own (see its own + // doc comment in session.ts). if (qsCustomModelBookkeeping) { session.setCustomModel(qsCustomModelBookkeeping, qsCustomModelAppliedEnvOverrides); } diff --git a/test/routes/quick-start-custom-model.test.ts b/test/routes/quick-start-custom-model.test.ts index c8a3764a..09518df9 100644 --- a/test/routes/quick-start-custom-model.test.ts +++ b/test/routes/quick-start-custom-model.test.ts @@ -204,6 +204,34 @@ describe('POST /api/quick-start: customModel (one-shot custom-model launch)', () expect(restartSpy).not.toHaveBeenCalled(); }); + it("leaves the caller's own model in the stored config, which is what a later clear falls back to", async () => { + const res = await quickStart({ + caseName: 'cm-pi-own-model', + mode: 'pi', + piConfig: { model: 'anthropic/claude-x' }, + customModel: { endpointId: 'ep1', modelId: 'qwen3' }, + }); + + expect(res.statusCode).toBe(200); + const { sessionId } = res.json(); + const session = ctx.sessions.get(sessionId) as unknown as Session; + expect(session.getCustomModelForPersist()?.launchModel).toBe('custom/qwen3'); + expect(session.toState().piConfig?.model).toBe('anthropic/claude-x'); + }); + + it('stores no model at all for a caller that chose none', async () => { + const res = await quickStart({ + caseName: 'cm-codex-no-model', + mode: 'codex', + customModel: { endpointId: 'ep1', modelId: 'qwen3' }, + }); + + expect(res.statusCode).toBe(200); + const session = ctx.sessions.get(res.json().sessionId) as unknown as Session; + expect(session.getCustomModelForPersist()?.launchModel).toBe('qwen3'); + expect(session.toState().codexConfig?.model).toBeUndefined(); + }); + it('404s for an unknown endpoint id', async () => { const res = await quickStart({ caseName: 'cm-ghost', diff --git a/test/session-custom-model-restart.test.ts b/test/session-custom-model-restart.test.ts index 5939aba1..7fa5d9a8 100644 --- a/test/session-custom-model-restart.test.ts +++ b/test/session-custom-model-restart.test.ts @@ -195,6 +195,53 @@ describe('restartCli() must not kill a working pane', () => { }); describe('launchModel reaches the CLI through its own launch param', () => { + /** A session the route just built: no pane yet, so its launch is `createSession`. */ + function freshSession(mode: SessionMode, extra: Record = {}) { + mkdirSync(workingDir, { recursive: true }); + const mux = new TmuxManager(); + const create = vi.spyOn(mux, 'createSession'); + vi.spyOn(mux, 'muxSessionExists').mockReturnValue(true); + const respawn = vi.spyOn(mux, 'respawnPane').mockResolvedValue(4242); + const session = new Session({ workingDir, mode, mux, useMux: true, ...extra }); + sessions.push(session); + return { session, create, respawn }; + } + + it('the first launch carries it too, and a clear relaunches on the model the session was created with', async () => { + const { session, create, respawn } = freshSession('pi', { piConfig: { model: 'anthropic/claude-x' } }); + session.setCustomModel( + { endpointId: 'ep', modelId: 'qwen3', envKeys: ['HOME'], configDir: '/tmp/cfg', launchModel: 'custom/qwen3' }, + { HOME: '/tmp/cfg' } + ); + await session.startInteractive(); + expect(create.mock.calls[0][0].piConfig?.model).toBe('custom/qwen3'); + expect(session.toState().piConfig?.model).toBe('anthropic/claude-x'); + + session.setCustomModel(undefined); + expect(await session.restartCli()).toBe(true); + expect(respawn.mock.calls[0][0].piConfig?.model).toBe('anthropic/claude-x'); + }); + + it('a clear on a session created with no model relaunches with none', async () => { + const { session, create, respawn } = freshSession('codex'); + session.setCustomModel( + { endpointId: 'ep', modelId: 'qwen3', envKeys: ['CODEX_HOME'], configDir: '/tmp/cfg', launchModel: 'qwen3' }, + { CODEX_HOME: '/tmp/cfg' } + ); + await session.startInteractive(); + expect(create.mock.calls[0][0].codexConfig?.model).toBe('qwen3'); + + session.setCustomModel(undefined); + expect(await session.restartCli()).toBe(true); + expect(respawn.mock.calls[0][0].codexConfig?.model).toBeUndefined(); + }); + + it('leaves the launch untouched without a selection', async () => { + const { session, create } = freshSession('pi', { piConfig: { model: 'anthropic/claude-x', thinking: 'low' } }); + await session.startInteractive(); + expect(create.mock.calls[0][0].piConfig).toEqual({ model: 'anthropic/claude-x', thinking: 'low' }); + }); + it('pi: the selection forces piConfig.model on the respawn options, leaving the stored config alone', async () => { const { session, respawn } = liveSession('pi', { piConfig: { model: 'anthropic/claude-x', thinking: 'low' } }); session.setCustomModel(