fix: keep a home value that is not a path off disk, and re-apply from the own home (release review)

getCustomModelForPersist() trusted homeEnvVar to name a path, but the registry
types it as any env name, so a clis.json override could write a session's own
credential to state.json. Only an absolute path is written now.

A re-apply resolved the CLI's real home from what the session's map held, which
under a selection is the injected directory, and recovery resolved none at all.
An entry the own home lacks (settings.yaml) was then linked from the server's
home on an endpoint switch and on recovery. The route now reads the new
Session.ownEnvValue(), and recovery reads the clamped record through the same
bounded guard, under the bulk cap since it walks every saved session. Also
corrects the comment that said only a removed directory gets its links again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-11-06 02:39:11 +01:00
1 parent 10d43b993e
commit db5222c4fa
9 files changed
+149 -27

No files matched your search

+34 -5
View File
@@ -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' });
@@ -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<unknown>;
};
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' });
+2
View File
@@ -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;
+22 -1
View File
@@ -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')!;
+26 -1
View File
@@ -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(