mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 02:49:33 +01:00
fix(custom-model): keep the launch model out of the stored config
Quick-start wrote a selection's launchModel into the session's stored <Mode>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 <noreply@anthropic.com>
This commit is contained in:
1 parent
fc25bf5f83
commit
a651616907
6 files changed
+95
-32
No files matched your search
@@ -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.<name>]` 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 <id>` refuses an id that already has a transcript (`Session ID ... is already in use`), and without the `--resume <id> || --session-id <id>` shape the docker/remote pane commands already use, applying a model killed the pane and lost the session. ⚠️ The pin looks in `<isolated>/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/<id>` for pi/omp, grok's `[model.codeman-custom]` block name), and codex gets one too (`--model <bare modelId>`, 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 <id>` refuses an id that already has a transcript (`Session ID ... is already in use`), and without the `--resume <id> || --session-id <id>` shape the docker/remote pane commands already use, applying a model killed the pane and lost the session. ⚠️ The pin looks in `<isolated>/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/<id>` for pi/omp, grok's `[model.codeman-custom]` block name), and codex gets one too (`--model <bare modelId>`, 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 `<Mode>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`.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
+8
-8
@@ -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
|
||||
* `<Mode>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 `<Mode>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',
|
||||
});
|
||||
|
||||
|
||||
@@ -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<string, string> | 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/<id>`, grok's
|
||||
// `[model.<name>]` 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<string, unknown>;
|
||||
qsSessionOptionsBag[qsCustomModelField] = {
|
||||
...((qsSessionOptionsBag[qsCustomModelField] as Record<string, unknown>) ?? {}),
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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<string, unknown> = {}) {
|
||||
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(
|
||||
|
||||
Reference in new issue
Block a user