mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 20:49:41 +02:00
Custom Model Endpoint Profiles (#393) let a session point its CLI at a custom OpenAI-compatible endpoint by injecting env vars or a config file and restarting the CLI in place. Review of the apply path found four things, two of them destructive. This lands all four plus the smaller items from the same review. 1. Clearing a selection did not clear it. The injected vars reach the CLI via `tmux setenv`, which persists at the tmux-session level and is inherited by `respawn-pane` (measured: `setenv FOO bar` survived two successive `respawn-pane -k`), so deleting the keys from the session's envOverrides relaunched the CLI still pointed at the old endpoint, and for the configDir kinds at a HOME/CODEX_HOME/GROK_HOME that had just been deleted. `Session.setCustomModel()` now reports the removed keys, queues them (`_pendingEnvUnsets`), and `RespawnPaneOptions.unsetEnvKeys` carries them into `applyEnvOverrides()`, which `setenv -u`s them before re-applying the live overrides, on the same path that already unsets the legacy CLAUDE_CODE_EFFORT_LEVEL. Verified on a private tmux socket that `setenv -u HOME` hands the next respawn the global HOME back. 2. Applying a model to a local claude session killed the pane. The relaunch was `claude --session-id <id>` and Claude refuses an id that already has a transcript, and unlike the dead-pane respawn this one kills a working pane first. `restartCli()` now pins the live conversation id as the resume id for that respawn when the CLI's launch declares a `fallback` chain, which renders the same `--resume <id> || --session-id <id>` shape the docker and remote pane commands use. Gated on the registry shape, not the CLI id: an entry whose resume id is minted by the CLI itself never declares that chain. 3. pi, omp and grok wrote their config file and then launched without the `--model` that selects it, so the file was ignored. The registry entry now declares `customModelInjection.launchModel` (`custom/{modelId}` for pi and omp, grok's `[model.codeman-custom]` block name), the builder renders it, and `_withCustomModelLaunchModel()` applies it onto the respawn options through `legacyConfigField`, leaving the stored <Mode>Config untouched so a clear falls back to the user's own model. A model id the CLI's `model` token pattern cannot carry is refused with a 400 rather than silently dropped by the argv engine. 4. Remote (SSH) and Docker sessions reported `restarted: true` and changed nothing: their `restartCli()` reattaches the durable tmux rather than relaunching the agent, and the env lands on the local pane. Both are refused with a 400 until those paths are plumbed. Smaller items from the same review: - The selection survives a Codeman restart as the disk-only `__customModel` bookkeeping (endpoint, model, injected key NAMES, config dir, launch model; never the values, which carry the API key). Recovery re-derives the values from the endpoint store through the same apply path the route uses and keeps the bookkeeping even when the endpoint is gone, so a later clear still has keys to unset. - Discovery goes through `webviewFetch()`, so the RESOLVED address is judged by the same egress guard the web-tab proxy uses, and `baseUrl` reuses `webviewUrlSchema` (http(s) only, no embedded credentials, link-local and cloud-metadata addresses refused). undici's `fetch failed` wrapper is unwrapped so the user sees the ECONNREFUSED underneath. - `custom-model-hosts.json` is written 0600 via tmp+rename, the per-session config dir 0700/0600 (pi and omp embed the key literally), and that dir is removed with the session. - `PR.md` is gone from the repo root and the design doc moved to `docs/custom-model-endpoints-plan.md` with the LAN address and the personal name scrubbed; every reference follows. The guide's `authStyle` text matches the shipped schema (`bearer | api-key`, default `bearer`) and says that `customModelEndpointsEnabled` is read by nothing until the picker lands. - `config/tsconfig.scripts.json` typechecks `scripts/test-local-llm-harnesses.ts` (four real type errors fixed). It is not yet wired into `npm run typecheck` because that line differs on master; adding `&& tsc -p config/tsconfig.scripts.json` there is the one-line follow-up. Tests: `test/session-custom-model-restart.test.ts` drives a real Session and fails on the unfixed code for items 1 to 3; the route suite covers item 4 and the pattern refusal; `test/tmux-manager.test.ts` pins that the unsets run before the overrides and that a shell-metachar key never reaches tmux. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
374 lines
10 KiB
TypeScript
374 lines
10 KiB
TypeScript
/**
|
|
* @fileoverview Tests for SessionManager
|
|
*
|
|
* Tests session lifecycle management including creation,
|
|
* event forwarding, and state persistence.
|
|
*/
|
|
|
|
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
|
import { EventEmitter } from 'node:events';
|
|
|
|
// Mock state that can be accessed by both mocks and tests
|
|
const mockState = vi.hoisted(() => ({
|
|
store: null as any,
|
|
sessions: new Map<string, any>(),
|
|
}));
|
|
|
|
// Mock the state-store
|
|
vi.mock('../src/state-store.js', () => {
|
|
class MockStateStore {
|
|
state: any = {
|
|
sessions: {},
|
|
config: { maxConcurrentSessions: 5 },
|
|
};
|
|
getConfig = vi.fn(() => this.state.config);
|
|
getSessions = vi.fn(() => this.state.sessions);
|
|
getSession = vi.fn((id: string) => this.state.sessions[id]);
|
|
setSession = vi.fn((id: string, state: any) => {
|
|
this.state.sessions[id] = state;
|
|
});
|
|
removeSession = vi.fn((id: string) => {
|
|
delete this.state.sessions[id];
|
|
});
|
|
}
|
|
|
|
const instance = new MockStateStore();
|
|
mockState.store = instance;
|
|
|
|
return {
|
|
getStore: vi.fn(() => instance),
|
|
StateStore: MockStateStore,
|
|
};
|
|
});
|
|
|
|
// Mock the Session class
|
|
vi.mock('../src/session.js', () => {
|
|
const { EventEmitter } = require('node:events');
|
|
|
|
class MockSession extends EventEmitter {
|
|
id: string;
|
|
workingDir: string;
|
|
_started = false;
|
|
_stopped = false;
|
|
|
|
constructor(options: { workingDir: string }) {
|
|
super();
|
|
this.id = `session-${Math.random().toString(36).substr(2, 9)}`;
|
|
this.workingDir = options.workingDir;
|
|
mockState.sessions.set(this.id, this);
|
|
}
|
|
|
|
async start() {
|
|
this._started = true;
|
|
return this;
|
|
}
|
|
|
|
async stop() {
|
|
this._stopped = true;
|
|
this.emit('exit');
|
|
}
|
|
|
|
toState() {
|
|
return {
|
|
id: this.id,
|
|
workingDir: this.workingDir,
|
|
status: this._stopped ? 'stopped' : this._started ? 'running' : 'pending',
|
|
pid: this._started && !this._stopped ? 12345 : null,
|
|
};
|
|
}
|
|
|
|
getCustomModelForPersist() {
|
|
return undefined;
|
|
}
|
|
getEnvOverridesForPersist() {
|
|
return undefined;
|
|
}
|
|
|
|
getOutput() {
|
|
return 'mock output';
|
|
}
|
|
|
|
getError() {
|
|
return null;
|
|
}
|
|
|
|
isIdle() {
|
|
return true;
|
|
}
|
|
|
|
isBusy() {
|
|
return false;
|
|
}
|
|
|
|
async sendInput(input: string) {
|
|
// Mock implementation
|
|
}
|
|
}
|
|
|
|
return {
|
|
Session: MockSession,
|
|
};
|
|
});
|
|
|
|
// Import after mocking
|
|
import { SessionManager, getSessionManager } from '../src/session-manager.js';
|
|
|
|
describe('SessionManager', () => {
|
|
let manager: SessionManager;
|
|
|
|
beforeEach(() => {
|
|
// Reset mock state
|
|
mockState.sessions.clear();
|
|
if (mockState.store) {
|
|
mockState.store.state = {
|
|
sessions: {},
|
|
config: { maxConcurrentSessions: 5 },
|
|
};
|
|
}
|
|
|
|
// Reset mock functions
|
|
vi.clearAllMocks();
|
|
|
|
// Create fresh manager
|
|
manager = new SessionManager();
|
|
});
|
|
|
|
describe('createSession', () => {
|
|
it('should create and start a new session', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(session).toBeDefined();
|
|
expect(session.workingDir).toBe('/tmp/test');
|
|
expect(manager.hasSession(session.id)).toBe(true);
|
|
});
|
|
|
|
it('should emit sessionStarted event', async () => {
|
|
const handler = vi.fn();
|
|
manager.on('sessionStarted', handler);
|
|
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(handler).toHaveBeenCalledWith(session);
|
|
});
|
|
|
|
it('should persist session to store', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(mockState.store.setSession).toHaveBeenCalledWith(session.id, expect.any(Object));
|
|
});
|
|
|
|
it('should throw when max sessions reached', async () => {
|
|
mockState.store.state.config.maxConcurrentSessions = 1;
|
|
await manager.createSession('/tmp/test1');
|
|
|
|
await expect(manager.createSession('/tmp/test2')).rejects.toThrow(/Maximum concurrent sessions/);
|
|
});
|
|
|
|
it('should forward session output events', async () => {
|
|
const handler = vi.fn();
|
|
manager.on('sessionOutput', handler);
|
|
|
|
const session = await manager.createSession('/tmp/test');
|
|
session.emit('output', 'test output');
|
|
|
|
expect(handler).toHaveBeenCalledWith(session.id, 'test output');
|
|
});
|
|
|
|
it('should forward session error events', async () => {
|
|
const handler = vi.fn();
|
|
manager.on('sessionError', handler);
|
|
|
|
const session = await manager.createSession('/tmp/test');
|
|
session.emit('error', 'test error');
|
|
|
|
expect(handler).toHaveBeenCalledWith(session.id, 'test error');
|
|
});
|
|
|
|
it('should forward session completion events', async () => {
|
|
const handler = vi.fn();
|
|
manager.on('sessionCompletion', handler);
|
|
|
|
const session = await manager.createSession('/tmp/test');
|
|
session.emit('completion', 'DONE');
|
|
|
|
expect(handler).toHaveBeenCalledWith(session.id, 'DONE');
|
|
});
|
|
|
|
it('should forward session exit events', async () => {
|
|
const handler = vi.fn();
|
|
manager.on('sessionStopped', handler);
|
|
|
|
const session = await manager.createSession('/tmp/test');
|
|
session.emit('exit');
|
|
|
|
expect(handler).toHaveBeenCalledWith(session.id);
|
|
});
|
|
});
|
|
|
|
describe('stopSession', () => {
|
|
it('should stop a session by ID', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
await manager.stopSession(session.id);
|
|
|
|
expect((session as any)._stopped).toBe(true);
|
|
});
|
|
|
|
it('should handle non-existent session gracefully', async () => {
|
|
await expect(manager.stopSession('non-existent')).resolves.not.toThrow();
|
|
expect(manager.getSessionCount()).toBe(0);
|
|
});
|
|
|
|
it('should update stored session to stopped', async () => {
|
|
// Set up a session in store
|
|
mockState.store.state.sessions['stored-session'] = {
|
|
id: 'stored-session',
|
|
status: 'running',
|
|
pid: 12345,
|
|
};
|
|
|
|
await manager.stopSession('stored-session');
|
|
|
|
expect(mockState.store.setSession).toHaveBeenCalledWith(
|
|
'stored-session',
|
|
expect.objectContaining({
|
|
status: 'stopped',
|
|
pid: null,
|
|
})
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('stopAllSessions', () => {
|
|
it('should stop all active sessions', async () => {
|
|
const session1 = await manager.createSession('/tmp/test1');
|
|
const session2 = await manager.createSession('/tmp/test2');
|
|
|
|
await manager.stopAllSessions();
|
|
|
|
expect((session1 as any)._stopped).toBe(true);
|
|
expect((session2 as any)._stopped).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe('getSession', () => {
|
|
it('should return undefined for non-existent session', () => {
|
|
expect(manager.getSession('non-existent')).toBeUndefined();
|
|
});
|
|
|
|
it('should return the correct session', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(manager.getSession(session.id)).toBe(session);
|
|
});
|
|
});
|
|
|
|
describe('getAllSessions', () => {
|
|
it('should return empty array when no sessions', () => {
|
|
expect(manager.getAllSessions()).toEqual([]);
|
|
});
|
|
|
|
it('should return all sessions', async () => {
|
|
const session1 = await manager.createSession('/tmp/test1');
|
|
const session2 = await manager.createSession('/tmp/test2');
|
|
|
|
const sessions = manager.getAllSessions();
|
|
|
|
expect(sessions).toHaveLength(2);
|
|
expect(sessions).toContain(session1);
|
|
expect(sessions).toContain(session2);
|
|
});
|
|
});
|
|
|
|
describe('getIdleSessions', () => {
|
|
it('should return sessions that are idle', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
const idle = manager.getIdleSessions();
|
|
|
|
expect(idle).toContain(session);
|
|
});
|
|
});
|
|
|
|
describe('getBusySessions', () => {
|
|
it('should return empty when all sessions are idle', async () => {
|
|
await manager.createSession('/tmp/test');
|
|
|
|
const busy = manager.getBusySessions();
|
|
|
|
expect(busy).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
describe('getSessionCount', () => {
|
|
it('should return 0 when no sessions', () => {
|
|
expect(manager.getSessionCount()).toBe(0);
|
|
});
|
|
|
|
it('should return correct count', async () => {
|
|
await manager.createSession('/tmp/test1');
|
|
await manager.createSession('/tmp/test2');
|
|
|
|
expect(manager.getSessionCount()).toBe(2);
|
|
});
|
|
});
|
|
|
|
describe('hasSession', () => {
|
|
it('should return false for non-existent session', () => {
|
|
expect(manager.hasSession('non-existent')).toBe(false);
|
|
});
|
|
|
|
it('should return true for existing session', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(manager.hasSession(session.id)).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe('sendToSession', () => {
|
|
it('should throw for non-existent session', async () => {
|
|
await expect(manager.sendToSession('non-existent', 'test')).rejects.toThrow(/Session non-existent not found/);
|
|
});
|
|
|
|
it('should send input to session', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
const sendSpy = vi.spyOn(session as any, 'sendInput');
|
|
|
|
await manager.sendToSession(session.id, 'test input');
|
|
|
|
expect(sendSpy).toHaveBeenCalledWith('test input');
|
|
});
|
|
});
|
|
|
|
describe('getSessionOutput', () => {
|
|
it('should return null for non-existent session', () => {
|
|
expect(manager.getSessionOutput('non-existent')).toBeNull();
|
|
});
|
|
|
|
it('should return session output', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(manager.getSessionOutput(session.id)).toBe('mock output');
|
|
});
|
|
});
|
|
|
|
describe('getSessionError', () => {
|
|
it('should return null for non-existent session', () => {
|
|
expect(manager.getSessionError('non-existent')).toBeNull();
|
|
});
|
|
|
|
it('should return session error', async () => {
|
|
const session = await manager.createSession('/tmp/test');
|
|
|
|
expect(manager.getSessionError(session.id)).toBeNull();
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('getSessionManager singleton', () => {
|
|
it('should return a SessionManager instance', () => {
|
|
const manager = getSessionManager();
|
|
expect(manager).toBeInstanceOf(SessionManager);
|
|
});
|
|
});
|