diff --git a/src/session.ts b/src/session.ts index 7b1f1ff5..11327f5b 100644 --- a/src/session.ts +++ b/src/session.ts @@ -29,7 +29,7 @@ */ import { EventEmitter } from 'node:events'; -import { basename } from 'node:path'; +import { basename, isAbsolute } from 'node:path'; import { realpath } from 'node:fs/promises'; import { execSync, execFileSync } from 'node:child_process'; import { v4 as uuidv4 } from 'uuid'; @@ -1374,6 +1374,17 @@ export class Session extends EventEmitter { return value && value.trim() ? value.trim() : undefined; } + /** + * What the SESSION itself set for `key`, same single-key rule. `envOverrideValue()` answers + * with whatever the map holds, which under a selection is the injected value: a re-apply + * resolving the CLI's real home must read the session's own home, or none. + */ + ownEnvValue(key: string): string | undefined { + const selection = this._customModel; + const value = selection?.envKeys.includes(key) ? selection.ownEnv?.[key] : this._envOverrides?.[key]; + return value?.trim() || undefined; + } + /** Owning username in multi-user mode, else undefined. */ get owner(): string | undefined { return this._owner; @@ -2003,7 +2014,9 @@ export class Session extends EventEmitter { * The one value it does carry is the session's own home directory (`ownEnv`, narrowed to * `homeEnvVar`), a path like `configDir` beside it: the selection overwrote the tmux * `setenv` that held it, so without this a clear after a restart would lose it. Any other - * entry of `ownEnv` may be a credential and stays in memory. + * entry of `ownEnv` may be a credential and stays in memory, and so does a `homeEnvVar` + * value that is not an absolute path: the registry types that name as any env var, so a + * `clis.json` override can point it at a credential. */ getCustomModelForPersist(): CustomModelBookkeeping | undefined { if (!this._customModel) return undefined; @@ -2013,7 +2026,7 @@ export class Session extends EventEmitter { return { ...selection, envKeys: [...selection.envKeys], - ...(homeEnvVar && ownHome ? { ownEnv: { [homeEnvVar]: ownHome } } : {}), + ...(homeEnvVar && ownHome && isAbsolute(ownHome) ? { ownEnv: { [homeEnvVar]: ownHome } } : {}), }; } diff --git a/src/types/session.ts b/src/types/session.ts index 26e7a8c4..6fea1fbf 100644 --- a/src/types/session.ts +++ b/src/types/session.ts @@ -711,8 +711,9 @@ export interface CustomModelBookkeeping extends CustomModelSelection { /** * What the session itself had set under keys the selection replaced, put back when the * selection goes. In memory this can hold a credential (a session's own `GEMINI_API_KEY`), - * so `Session.getCustomModelForPersist()` writes `homeEnvVar`'s entry and nothing else: - * tmux keeps only the injected value, so state.json is the one copy left after a restart. + * so `Session.getCustomModelForPersist()` writes `homeEnvVar`'s entry, when it is an absolute + * path, and nothing else: tmux keeps only the injected value, so state.json is the one copy + * left after a restart. */ ownEnv?: Record; } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 0ef420a1..3f1d96a3 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -202,7 +202,7 @@ import { } from '../../docker-hosts.js'; import { resolveCaseRepository, resolveManagedWorktree } from '../../git-worktrees.js'; import { LRUMap } from '../../utils/lru-map.js'; -import { describeUnknownPath, probePathKind } from '../../utils/index.js'; +import { describeUnknownPath, probePathKind, type PathProbeOptions } from '../../utils/index.js'; import { findLatestOmpSessionId } from '../../utils/omp-session-resolver.js'; import { scanOmpSessionsHistory } from '../../omp-transcript.js'; import { scanCodexSessionsHistory, codexThreadBySessionId } from '../../codex-transcript.js'; @@ -244,18 +244,20 @@ function landedInGroup(layout: TabLayout | undefined, sessionId: string): boolea * Probe the session's own home with `probePathKind(..., { pastCap: true })` first: on * `'unknown'` (a dead mount that never answered) hand the apply a lookup that hides that * one env var, so linking falls through to the server's own environment and the unresponsive - * path is never touched. Any other entry's lookup passes through untouched. + * path is never touched. Any other entry's lookup passes through untouched. Boot recovery + * passes `probeOptions` of `{}`: it walks every saved session, so it keeps the bulk cap. */ export async function guardSessionConfigDirHome( entry: Pick, - baseLookup: (envVar: string) => string | undefined + baseLookup: (envVar: string) => string | undefined, + probeOptions: PathProbeOptions = { pastCap: true } ): Promise<(envVar: string) => string | undefined> { const recipe = entry.capabilities.customModelInjection; if (recipe?.kind !== 'configDir' || !recipe.linkFromRealHome?.length) return baseLookup; const dirEnvVar = recipe.dirEnvVar; const supplied = baseLookup(dirEnvVar)?.trim(); if (!supplied || !isAbsolute(supplied)) return baseLookup; - if ((await probePathKind(supplied, { pastCap: true })) === 'unknown') { + if ((await probePathKind(supplied, probeOptions)) === 'unknown') { console.warn( `[custom-model] ${dirEnvVar} (${supplied}) did not answer; linking from the server's own home instead` ); @@ -1675,7 +1677,7 @@ export function registerSessionRoutes( session.id, contextLength, session.workingDir, - await guardSessionConfigDirHome(entry, (envVar) => session.envOverrideValue(envVar)) + await guardSessionConfigDirHome(entry, (envVar) => session.ownEnvValue(envVar)) ); if (!applied) { return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${session.mode} has no known custom-model mechanism`); diff --git a/src/web/server.ts b/src/web/server.ts index 67001f87..408c682c 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -136,6 +136,7 @@ import { fetchWorkspaceOnce } from '../git-fetch.js'; import { GitFetchScheduler } from '../git-fetch-scheduler.js'; import { probePathKind } from '../utils/bounded-path-probe.js'; import { defaultDockerWorkspaces } from './routes/git-status-routes.js'; +import { guardSessionConfigDirHome } from './routes/session-routes.js'; import { composerIsEmpty, plainFrame } from '../session-submit-verifier.js'; import { detectUsageLimitPause } from '../usage-limit-patterns.js'; import { EditHistorySync } from './edit-history-sync.js'; @@ -1520,7 +1521,8 @@ export class WebServer extends EventEmitter { */ private async _rebuildCustomModelEnv( session: Session, - saved: CustomModelBookkeeping + saved: CustomModelBookkeeping, + ownEnv: Record = {} ): Promise { const entry = getCli(session.mode); if (!entry) return undefined; @@ -1532,17 +1534,21 @@ export class WebServer extends EventEmitter { return undefined; } try { - // Boot recovery omits both `workingDir` (no reason to re-answer a trust dialog that - // already fired once) and the session-env real-home lookup: the first apply already - // linked this directory from the session's own home, and `linkRealHomeEntry()` keeps - // what is there. Only a directory someone removed gets its links again, from the - // SERVER's `DSH_HOME` or the recipe's `realHomeFallback` (~/.dsh). + // Boot recovery omits `workingDir` (no reason to re-answer a trust dialog that already + // fired once). The real-home lookup reads the record's own values, since the session's + // overrides hold what the previous run injected. `linkRealHomeEntry()` keeps what is + // there, so this only decides an entry that is missing, which an own home skips when it + // does not hold one. With no own home, or one that does not answer (probed under the + // bulk cap, this walks every saved session), a missing entry links from the SERVER's + // `DSH_HOME` or the recipe's `realHomeFallback` (~/.dsh). return applyCustomModelInjection( entry, endpoint, saved.modelId, session.id, - resolveCustomModelContextLength(endpoint, saved.modelId) + resolveCustomModelContextLength(endpoint, saved.modelId), + undefined, + await guardSessionConfigDirHome(entry, (envVar) => ownEnv[envVar], {}) ); } catch (err) { console.warn('[WebServer] Failed to rebuild custom-model env on recovery:', err); @@ -1565,7 +1571,7 @@ export class WebServer extends EventEmitter { */ private async _restoreCustomModel(session: Session, saved: CustomModelBookkeeping): Promise { const ownEnv = (await clampEnvOverridesForOwner(session.owner, saved.ownEnv)) ?? {}; - const applied = await this._rebuildCustomModelEnv(session, saved); + const applied = await this._rebuildCustomModelEnv(session, saved, ownEnv); if (!applied) { // Endpoint gone or CLI unregistered: restore the bookkeeping exactly as saved, which // is what a later clear needs. @@ -1596,8 +1602,9 @@ export class WebServer extends EventEmitter { // __attachmentHistory keeps the private (externalPath-bearing) history on disk, // separate from the sanitized public attachmentHistory in toState(). const attachmentHistory = session.getAttachmentHistoryForPersist(); - // __customModel keeps the selection's bookkeeping (injected env KEYS, config dir, - // launch model; never the values) so recovery can restore and later clear it. + // __customModel keeps the selection's bookkeeping (injected env KEYS, config dir, launch + // model, the session's own home path; never an injected value or any other own value) + // so recovery can restore and later clear it. const customModel = session.getCustomModelForPersist(); const state = { ...base, diff --git a/test/custom-model-recovery.test.ts b/test/custom-model-recovery.test.ts index 0fc64910..636f6496 100644 --- a/test/custom-model-recovery.test.ts +++ b/test/custom-model-recovery.test.ts @@ -7,17 +7,18 @@ * home: restoring the saved record unchanged left `DSH_HOME` on a generated config dir * that clearing could never delete. * - * Real-home resolution takes no session lookup on this path: the first apply's links are - * still in the generated directory, and one someone removed links from the server's - * `DSH_HOME` or the recipe's `realHomeFallback` (~/.dsh), see `_rebuildCustomModelEnv`'s - * comment. A session's own home comes back on clear from `ownEnv`, the last block here. + * Real-home resolution reads the record's `ownEnv` on this path, never the session's + * overrides: the first apply's links are still in the generated directory, and an entry + * that is missing links from the own home, or with none from the server's `DSH_HOME` or + * the recipe's `realHomeFallback` (~/.dsh), see `_rebuildCustomModelEnv`'s comment. The + * own home itself comes back on clear from `ownEnv`, the last block here. * * `_restoreCustomModel` reads no `this` state beyond the rebuild helper (it re-derives * everything from the endpoint store and the Session it is handed), so it is driven off * the prototype with a minimal receiver: `restoreMuxSessions()` itself cannot be * reached under vitest, the same constraint recorded in test/session-model-recovery.test.ts. */ -import { mkdirSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, readlinkSync, rmSync, writeFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -137,6 +138,34 @@ describe('_restoreCustomModel: bookkeeping rebuilt from the current recipe', () expect(session.envOverrideValue('DSH_HOME')).toBe(OWN_HOME); }); + it('never fills an entry the own home lacks from the server home', async () => { + // The first apply skipped `settings.yaml`, since the own home has none. Recovery finds + // that gap and, with no lookup, used to fill it with the server's settings. + const serverHome = join(homedir(), 'codeman-recovery-server-dsh'); + const ownHome = join(homedir(), 'codeman-recovery-own-dsh'); + mkdirSync(join(ownHome, 'profiles'), { recursive: true }); + mkdirSync(serverHome, { recursive: true }); + writeFileSync(join(serverHome, 'settings.yaml'), 'server: true\n'); + vi.stubEnv('DSH_HOME', serverHome); + await writeCustomModelHosts(getDataDir(), [endpoint()]); + const session = new Session({ workingDir: '/tmp', mode: 'deepseek' }); + configs.push(session.id); + + await restoreCustomModel(session, { + endpointId: 'ep', + modelId: 'qwen3', + envKeys: ['DSH_HOME', 'CODEMAN_CUSTOM_MODEL_API_KEY'], + homeEnvVar: 'DSH_HOME', + ownEnv: { DSH_HOME: ownHome }, + }); + + const configDir = customModelConfigDir(session.id); + expect(readlinkSync(join(configDir, 'profiles'))).toBe(join(ownHome, 'profiles')); + expect(existsSync(join(configDir, 'settings.yaml'))).toBe(false); + rmSync(ownHome, { recursive: true, force: true }); + rmSync(serverHome, { recursive: true, force: true }); + }); + it('keeps it when the endpoint is gone', async () => { await writeCustomModelHosts(getDataDir(), []); const session = new Session({ workingDir: '/tmp', mode: 'deepseek' }); diff --git a/test/custom-model-session-home-guard.test.ts b/test/custom-model-session-home-guard.test.ts index 08f36fc7..18460bbd 100644 --- a/test/custom-model-session-home-guard.test.ts +++ b/test/custom-model-session-home-guard.test.ts @@ -14,8 +14,14 @@ vi.mock('../src/utils/bounded-path-probe.js', async (importOriginal) => { }); import { getCli } from '../src/config/cli-registry/registry.js'; +import { getDataDir } from '../src/config/instance.js'; +import { writeCustomModelHosts } from '../src/custom-model-hosts.js'; +import { customModelConfigDir, removeConfigDir } from '../src/custom-model-injection-apply.js'; +import { Session } from '../src/session.js'; import { probePathKind } from '../src/utils/bounded-path-probe.js'; import { guardSessionConfigDirHome } from '../src/web/routes/session-routes.js'; +import { WebServer } from '../src/web/server.js'; +import type { CustomModelBookkeeping } from '../src/types/session.js'; const probe = vi.mocked(probePathKind); const deepseek = () => getCli('deepseek')!; @@ -52,6 +58,22 @@ describe('guardSessionConfigDirHome', () => { expect(warn).not.toHaveBeenCalled(); }); + it('boot recovery probes the own home under the stall cap: it is a bulk pass, not one request', async () => { + probe.mockResolvedValue('unknown'); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + await writeCustomModelHosts(getDataDir(), [{ id: 'ep', label: 'Ep', baseUrl: 'http://box:8080' }]); + const session = new Session({ workingDir: '/tmp', mode: 'deepseek' }); + const saved: CustomModelBookkeeping = { endpointId: 'ep', modelId: 'q', envKeys: [] }; + const proto = WebServer.prototype as unknown as { + _rebuildCustomModelEnv: (session: Session, saved: CustomModelBookkeeping, ownEnv: object) => Promise; + }; + + await proto._rebuildCustomModelEnv(session, saved, { DSH_HOME: '/mnt/dead/dsh' }); + + expect(probe).toHaveBeenCalledWith('/mnt/dead/dsh', {}); + removeConfigDir(customModelConfigDir(session.id)); + }); + it('never probes a relative home: the apply refuses to join one', async () => { const base = env({ DSH_HOME: 'relative/dsh' }); diff --git a/test/mocks/mock-session.ts b/test/mocks/mock-session.ts index cd04cebf..fa59db6c 100644 --- a/test/mocks/mock-session.ts +++ b/test/mocks/mock-session.ts @@ -464,6 +464,8 @@ export class MockSession extends EventEmitter { /** 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); + /** Mirrors Session.ownEnvValue, same default. */ + ownEnvValue = 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; diff --git a/test/routes/session-custom-model.test.ts b/test/routes/session-custom-model.test.ts index d28277f0..964c1ae5 100644 --- a/test/routes/session-custom-model.test.ts +++ b/test/routes/session-custom-model.test.ts @@ -9,7 +9,7 @@ import { createRouteTestHarness } from './_route-test-utils.js'; import { createMockSession } from '../mocks/index.js'; import { getDataDir } from '../../src/config/instance.js'; import { writeCustomModelHosts, type CustomModelHost } from '../../src/custom-model-hosts.js'; -import { existsSync, readFileSync, rmSync, statSync } from 'node:fs'; +import { existsSync, readFileSync, readlinkSync, rmSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { webviewFetch } from '../../src/web/webview-egress.js'; @@ -97,6 +97,27 @@ describe('POST /api/sessions/:id/custom-model', () => { expect(trustFile.customApiKeyResponses.approved).toEqual(['k']); }); + it("links a switch from the session's own home, never the injected one the map now holds", async () => { + const { app, ctx } = await setup(); + const session = ctx.sessions.get('test-session-1')!; + session.mode = 'deepseek'; + const isolatedDir = join(getDataDir(), 'custom-model-configs', 'test-session-1'); + const ownHome = join(getDataDir(), 'own-dsh-home'); + session.envOverrideValue.mockImplementation(() => isolatedDir); + session.ownEnvValue.mockImplementation((key: string) => (key === 'DSH_HOME' ? ownHome : undefined)); + + const res = await app.inject({ + method: 'POST', + url: '/api/sessions/test-session-1/custom-model', + payload: { endpointId: 'ep1', modelId: 'qwen3' }, + }); + + expect(res.statusCode).toBe(200); + expect(readlinkSync(join(isolatedDir, 'sessions'))).toBe(join(ownHome, 'sessions')); + rmSync(isolatedDir, { recursive: true, force: true }); + rmSync(ownHome, { recursive: true, force: true }); + }); + it('clears back to the native default', async () => { const { app, ctx } = await setup(); const session = ctx.sessions.get('test-session-1')!; diff --git a/test/session-custom-model-restart.test.ts b/test/session-custom-model-restart.test.ts index 581b6dbb..9828cbb6 100644 --- a/test/session-custom-model-restart.test.ts +++ b/test/session-custom-model-restart.test.ts @@ -1,7 +1,7 @@ /** * @fileoverview Custom Model Endpoint Profiles, the Session half of applying and * clearing a selection against a live pane (`Session.setCustomModel()` + - * `Session.restartCli()`), pinned against the three ways the first cut broke a + * `Session.restartCli()`), pinned against the four ways the first cut broke a * working session: * * 1. Clearing did not clear. The injected vars reach the CLI via `tmux setenv`, @@ -347,6 +347,31 @@ describe('a value the session set itself comes back with the selection gone', () expect(session.envOverrideValue('GEMINI_API_KEY')).toBe('own-secret'); }); + it('keeps a home variable holding anything but an absolute path off disk', () => { + // The registry types `configDirVar`/`dirEnvVar` as plain env names, so a clis.json + // override can name a credential as the home variable. Only a path may reach disk. + const { session } = liveSession('gemini', { envOverrides: { GEMINI_API_KEY: 'own-secret' } }); + session.setCustomModel( + { endpointId: 'ep', modelId: 'q', envKeys: ['GEMINI_API_KEY'], homeEnvVar: 'GEMINI_API_KEY' }, + { GEMINI_API_KEY: 'endpoint-key' } + ); + + expect(JSON.stringify(session.getCustomModelForPersist())).not.toContain('own-secret'); + session.setCustomModel(undefined); + expect(session.envOverrideValue('GEMINI_API_KEY')).toBe('own-secret'); + }); + + it('reads the own value while a selection holds the key, never the injected one', () => { + const { session } = liveSession('deepseek', { envOverrides: { DSH_HOME: ' /home/u/my-dsh ' } }); + expect(session.ownEnvValue('DSH_HOME')).toBe('/home/u/my-dsh'); + + session.setCustomModel(DSH_SELECTION, DSH_ENV); + expect(session.ownEnvValue('DSH_HOME')).toBe('/home/u/my-dsh'); + // An injected key the session never set has no own value, whatever the map holds now. + expect(session.envOverrideValue('CODEMAN_CUSTOM_MODEL_API_KEY')).toBe('k'); + expect(session.ownEnvValue('CODEMAN_CUSTOM_MODEL_API_KEY')).toBeUndefined(); + }); + it('persists nothing extra for a recipe with no home variable', () => { const { session } = liveSession('gemini', { envOverrides: { GEMINI_API_KEY: 'own-secret' } }); session.setCustomModel(