From 3c4bb91d6f68db8b38291b3c187b260ea1fb932b Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Fri, 6 Nov 2026 23:54:46 +0100 Subject: [PATCH] fix(local-model): ask the launch guard before every start of an agent on the managed endpoint Only the two session routes asked launchRefusalMessage(). The attach that finds no pane, the respawn of a dead pane (a click on the tab, a respawn start, the boot recovery), restartCli() and the reboot-restore rebuild started an agent on the endpoint unasked. Session gains setLaunchGuard(), asked right before a NEW agent process is started and never for an attach to an agent that runs; WebServer.setupSessionListeners() sets it for every session; a refusal is a LaunchRefusedError that never falls back to a direct PTY. The reboot-restore route asks before it builds and skips the entry with its own reason, left on offer. Co-Authored-By: Claude Fable 5.1 --- src/reboot-restore.ts | 2 + src/session.ts | 48 +++ src/web/public/reboot-restore-ui.js | 2 + src/web/routes/reboot-restore-routes.ts | 16 + src/web/server.ts | 11 + test/local-model-session-launch-guard.test.ts | 294 ++++++++++++++++++ test/routes/local-model-launch-guard.test.ts | 9 +- 7 files changed, 380 insertions(+), 2 deletions(-) create mode 100644 test/local-model-session-launch-guard.test.ts diff --git a/src/reboot-restore.ts b/src/reboot-restore.ts index fe7bff40..b1aec5f4 100644 --- a/src/reboot-restore.ts +++ b/src/reboot-restore.ts @@ -134,6 +134,8 @@ export interface RebootRestoreRejection { | 'workspace-forbidden' | 'already-live' | 'capacity-reached' + /** The session ran on the model Codeman runs on this Mac, and that model cannot be started on right now. */ + | 'local-model-not-ready' | 'rebuild-failed'; } diff --git a/src/session.ts b/src/session.ts index 4b84fb5d..9d17f767 100644 --- a/src/session.ts +++ b/src/session.ts @@ -510,6 +510,18 @@ export class GuardedInputBusyError extends Error { } } +/** + * The session's launch guard refused the start of a new agent process + * (`Session.setLaunchGuard()`). Its message is the sentence for the user. A typed error, so a + * start that was refused is told apart from a tmux failure and never falls back to a direct PTY. + */ +export class LaunchRefusedError extends Error { + constructor(message: string) { + super(message); + this.name = 'LaunchRefusedError'; + } +} + /** * Whether a programmatic write leaves the keep-alive turn running: the nudge itself, or a bare * Enter (which submits nothing new). Anything else carrying an Enter starts a turn of its own. @@ -880,6 +892,12 @@ export class Session extends EventEmitter { // config dir) without guessing what it once wrote. Persisted disk-only (`__customModel`). private _customModel: CustomModelBookkeeping | undefined; + // Asked right before a NEW agent process is started for this session (a fresh pane, the + // respawn of a dead one, a restart in place), never for an attach to an agent that runs. + // Resolves with the sentence that refuses the start, or undefined. Set by the web layer + // (`WebServer.setupSessionListeners()`); this file does not know who asks or why. + private _launchGuard: (() => Promise) | null = null; + // Env keys a retired custom-model selection injected that the NEXT respawn must // `tmux setenv -u`. Deleting a key from `_envOverrides` alone does nothing to the // tmux session, which keeps every `setenv` and hands it to `respawn-pane`, so the @@ -2063,6 +2081,22 @@ export class Session extends EventEmitter { } } + /** + * Sets what is asked before a new agent process is started for this session (see + * `_launchGuard`). The first user is the model Codeman runs on this Mac: an agent is not + * started on it while its runtime is down or its model changed. `null` removes it. + */ + setLaunchGuard(guard: (() => Promise) | null): void { + this._launchGuard = guard; + } + + /** Throws the guard's sentence (`LaunchRefusedError`) when a new agent process must not be started now. */ + private async _assertLaunchAllowed(): Promise { + if (!this._launchGuard) return; + const refusal = await this._launchGuard(); + if (refusal) throw new LaunchRefusedError(refusal); + } + // Custom Model Endpoint Profiles (docs/custom-model-endpoints-plan.md) — public-safe // subset only (never envKeys/configDir/launchModel, the bookkeeping for setCustomModel). get customModel(): CustomModelSelection | undefined { @@ -2635,6 +2669,8 @@ export class Session extends EventEmitter { let respawnedResumeId: string | undefined; if (this._muxSession && mux.isPaneDead(this._muxSession.muxName)) { console.log('[Session] Dead pane detected, respawning:', this._muxSession.muxName); + // A new agent process: asked first (see `_launchGuard`). A refusal leaves the pane as it is. + await this._assertLaunchAllowed(); // Confirmed dead — safe to resolve/pin now (see `_pinOmpRespawnId()`). // `options.respawnPaneOptions` was built eagerly before this dead-pane // check ran, so it still carries the pre-pin ompConfig; rebuild it. @@ -2689,6 +2725,8 @@ export class Session extends EventEmitter { this._resumeSessionId = pinned; } } + // A new agent process: asked first (see `_launchGuard`). + await this._assertLaunchAllowed(); this._muxSession = await mux.createSession(options.createSessionOptions); console.log('[Session] Created mux session:', this._muxSession.muxName); // No extra sleep — createSession() already waits for tmux readiness @@ -2808,6 +2846,9 @@ export class Session extends EventEmitter { return false; } + // A new agent process: asked first (see `_launchGuard`). A refusal rejects, and the pane + // keeps what runs in it. + await this._assertLaunchAllowed(); this._pinOmpRespawnId(); const newPid = await mux.respawnPane(await this._buildRespawnPaneOptionsWithResumePin()); if (!newPid) { @@ -3461,6 +3502,9 @@ export class Session extends EventEmitter { this._armPaneSettle(true); } } catch (err) { + // A refused launch is an answer, not a tmux failure: it must not fall through to + // the direct PTY below, which would start the very process that was refused. + if (err instanceof LaunchRefusedError) throw err; if (this._adopt) this._failAdoptedStart(err); console.error('[Session] Failed to create mux session, falling back to direct PTY:', err); this._useMux = false; @@ -3482,6 +3526,8 @@ export class Session extends EventEmitter { throw new Error(`${getModeLabel(this.mode)} sessions require tmux. Direct PTY fallback is not supported.`); } if (this._adopt) this._failAdoptedStart(new Error('tmux is not available')); + // A new agent process on this path too: asked first (see `_launchGuard`). + await this._assertLaunchAllowed(); try { // Pass --session-id to use the SAME ID as the Codeman session // This ensures subagents can be directly matched to the correct tab. @@ -4485,6 +4531,8 @@ export class Session extends EventEmitter { }, 100); } } catch (err) { + // As in startInteractive(): a refused launch never falls back to a direct PTY. + if (err instanceof LaunchRefusedError) throw err; if (this._adopt) this._failAdoptedStart(err); console.error('[Session] Failed to create mux session, falling back to direct PTY:', err); this._useMux = false; diff --git a/src/web/public/reboot-restore-ui.js b/src/web/public/reboot-restore-ui.js index 1e738bdd..58653cc9 100644 --- a/src/web/public/reboot-restore-ui.js +++ b/src/web/public/reboot-restore-ui.js @@ -40,6 +40,8 @@ function rebootSkipReason(reason) { return 'already open'; case 'capacity-reached': return 'session limit reached'; + case 'local-model-not-ready': + return 'the local model is not ready'; case 'rebuild-failed': return 'the agent would not start'; default: diff --git a/src/web/routes/reboot-restore-routes.ts b/src/web/routes/reboot-restore-routes.ts index 7945e96d..9cffe9f5 100644 --- a/src/web/routes/reboot-restore-routes.ts +++ b/src/web/routes/reboot-restore-routes.ts @@ -35,6 +35,7 @@ import { isWorkingDirAllowedForUsername, sessionCapacityMessage, } from '../route-helpers.js'; +import { getLocalModelService } from '../../local-model/install-job.js'; import { rebootRestoreRegistry } from '../reboot-restore-registry.js'; import { rejectAlreadyLive, type RebootRestoreEntry, type RebootRestoreRejection } from '../../reboot-restore.js'; import { clampEnvOverridesForOwner } from '../../session-env-clamp.js'; @@ -163,6 +164,21 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes failures.push({ sessionId: entry.sessionId, reason: 'workspace-forbidden' }); continue; } + // A session that ran on the model Codeman runs on this Mac is rebuilt only while that + // model can be started on: its own runtime is up and its digest is the one stored at + // install time. Asked before anything is built, like the two session routes; the + // session's own launch guard would refuse the start too, but only after the session + // exists. Every other endpoint, and a session without one, is answered `undefined`. + const savedModel = (entry.state as { __customModel?: { endpointId?: string; modelId?: string } }).__customModel; + if (savedModel && typeof savedModel.endpointId === 'string' && typeof savedModel.modelId === 'string') { + const refusal = await getLocalModelService().launchRefusalMessage(savedModel.endpointId, savedModel.modelId); + if (refusal) { + console.warn(`[reboot-restore] ${entry.sessionId} is not rebuilt: ${refusal}`); + // Left on offer: the model can be started (or installed again) and the click repeated. + failures.push({ sessionId: entry.sessionId, reason: 'local-model-not-ready' }); + continue; + } + } try { const saved = entry.state; const claudeModeConfig = await ctx.getClaudeModeConfig(); diff --git a/src/web/server.ts b/src/web/server.ts index d8f2ace3..0a47f4d6 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2194,6 +2194,17 @@ export class WebServer extends EventEmitter { // First keep-warm evaluation: publishes the effective opt-in for the Session Options // readout even before any cache report arrives. void this.keepWarmService.schedule(session.id); + // Before every start of a NEW agent process the session asks this (a fresh pane, the + // respawn of a dead one, a restart in place; never an attach to an agent that runs). + // The model Codeman runs on this Mac is the one endpoint with something to say: no agent + // is started on it while its runtime is down or its model is not the one that was + // installed. Every other session, and every other endpoint, is answered `undefined`. + // Set on every call, above the idempotency return: it holds no state of its own. + session.setLaunchGuard(async () => { + const selection = session.customModel; + if (!selection) return undefined; + return getLocalModelService().launchRefusalMessage(selection.endpointId, selection.modelId); + }); // Idempotent: the wiring exit handler detaches ALL listeners on every PTY exit // (removeSessionListenerRefs), so the re-attach routes (/interactive, // /interactive-respawn, /shell) call this again to restore observability diff --git a/test/local-model-session-launch-guard.test.ts b/test/local-model-session-launch-guard.test.ts new file mode 100644 index 00000000..5528b677 --- /dev/null +++ b/test/local-model-session-launch-guard.test.ts @@ -0,0 +1,294 @@ +/** + * @fileoverview No coding agent is STARTED on the model Codeman runs on this Mac while that + * model cannot be started on: the launch guard below the routes. + * + * The two routes that put a session onto a custom model endpoint ask the install service + * before they write anything (test/routes/local-model-launch-guard.test.ts). That leaves + * every other way a new agent process comes to be in a session that already carries the + * selection: the attach that finds no pane, the respawn of a dead pane (a click on the tab, a + * respawn start, the recovery at boot), a restart in place, and the rebuild after a reboot. + * They all end in `Session`, so the guard sits there: + * + * - `Session.setLaunchGuard()` is asked right before a NEW agent process is started (a fresh + * pane, the respawn of a dead one, `restartCli()`), and never for an attach to an agent + * that runs: a session whose runtime was stopped can still be looked at and closed. + * - `WebServer.setupSessionListeners()` sets it for every session, and it asks + * `LocalModelService.launchRefusalMessage()` with the session's own selection. + * - The reboot-restore route asks the same question before it builds anything, so the entry + * is refused with its own reason and stays on offer. + * + * The Session cases drive a real `Session` against the in-memory tmux layer vitest + * substitutes. The wiring cases use the REAL install service over the real state file, with + * no runtime running, which is exactly the state that must refuse. + * + * Port: N/A (WebServer in test mode binds none; the routes go through app.inject). + */ +import { mkdirSync, rmSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; + +import { dataPath } from '../src/config/instance.js'; +import type { RebootRestoreEntry } from '../src/reboot-restore.js'; +import { Session } from '../src/session.js'; +import { TmuxManager } from '../src/tmux-manager.js'; +import { httpStatusForErrorCode, type ApiErrorCode, type MuxSession, type SessionState } from '../src/types.js'; +import { rebootRestoreRegistry } from '../src/web/reboot-restore-registry.js'; +import { installRouteErrorHandler } from '../src/web/route-error-handler.js'; +import { registerRebootRestoreRoutes } from '../src/web/routes/reboot-restore-routes.js'; +import { WebServer } from '../src/web/server.js'; +import { createMockRouteContext, safeRmHomeTree } from './mocks/index.js'; + +const WORKSPACE = join(homedir(), 'codeman-cases', 'local-model-launch-guard'); +const STATE = () => dataPath('local-model.json'); +const NOT_RUNNING = 'The model on this Mac is not running. Start it first.'; +const CHANGED = 'The model on this Mac changed since it was installed. Remove it and install it again.'; +const SELECTION = { endpointId: 'local-ollama', modelId: 'qwen3.6:35b-a3b', envKeys: [] as string[] }; + +const sessions: Session[] = []; + +function paneOf(mode: 'claude' | 'opencode' = 'claude'): MuxSession { + return { + sessionId: 'placeholder', + muxName: 'codeman-cafe0002', + pid: 1, + createdAt: Date.now(), + workingDir: WORKSPACE, + mode, + attached: false, + }; +} + +/** A session that already has a pane, alive or dead as the test says. */ +function sessionWithPane(dead: boolean) { + mkdirSync(WORKSPACE, { recursive: true }); + const mux = new TmuxManager(); + const session = new Session({ workingDir: WORKSPACE, mode: 'claude', mux, useMux: true, muxSession: paneOf() }); + sessions.push(session); + vi.spyOn(mux, 'muxSessionExists').mockReturnValue(true); + vi.spyOn(mux, 'isPaneDead').mockReturnValue(dead); + const respawn = vi.spyOn(mux, 'respawnPane').mockResolvedValue(4242); + const create = vi.spyOn(mux, 'createSession'); + return { session, mux, respawn, create }; +} + +/** The state file of an installed model whose connection is the managed record. No runtime runs. */ +function writeInstalledState(): void { + writeFileSync( + STATE(), + JSON.stringify({ + schema: 1, + state: 'installed', + model: { id: 'qwen3.6:35b-a3b', label: 'Qwen3.6 35B-A3B', context: 131072, digest: 'abc123' }, + cli: { id: 'opencode', label: 'OpenCode' }, + runtime: { version: '0.40.15', port: 11435, enabled: true }, + connection: { endpointId: 'local-ollama', flagWasEnabled: false }, + }) + ); +} + +afterEach(() => { + for (const session of sessions.splice(0)) session.stop(); + rmSync(STATE(), { force: true }); + rebootRestoreRegistry.reset(); + vi.restoreAllMocks(); +}); + +afterAll(() => { + safeRmHomeTree(WORKSPACE); +}); + +describe('Session: the guard is asked before a new agent process, and only then', () => { + it('a restart in place that is refused rejects with the sentence, and the pane keeps what runs in it', async () => { + const { session, respawn } = sessionWithPane(false); + const guard = vi.fn(async () => CHANGED); + session.setLaunchGuard(guard); + await expect(session.restartCli()).rejects.toThrow(CHANGED); + expect(guard).toHaveBeenCalledTimes(1); + expect(respawn).not.toHaveBeenCalled(); + }); + + it('a guard that has nothing to say lets the restart through', async () => { + const { session, respawn } = sessionWithPane(false); + session.setLaunchGuard(async () => undefined); + expect(await session.restartCli()).toBe(true); + expect(respawn).toHaveBeenCalledTimes(1); + }); + + it('without a guard nothing changes', async () => { + const { session, respawn } = sessionWithPane(false); + expect(await session.restartCli()).toBe(true); + expect(respawn).toHaveBeenCalledTimes(1); + }); + + it('an attach to a pane whose agent runs is never asked: the session can still be looked at', async () => { + const { session, respawn, create } = sessionWithPane(false); + const guard = vi.fn(async () => NOT_RUNNING); + session.setLaunchGuard(guard); + await session.startInteractive(); + expect(guard).not.toHaveBeenCalled(); + expect(respawn).not.toHaveBeenCalled(); + expect(create).not.toHaveBeenCalled(); + }); + + it('the respawn of a dead pane is asked first, and a refusal starts nothing', async () => { + const { session, respawn, create } = sessionWithPane(true); + const guard = vi.fn(async () => NOT_RUNNING); + session.setLaunchGuard(guard); + await expect(session.startInteractive()).rejects.toThrow(NOT_RUNNING); + expect(guard).toHaveBeenCalledTimes(1); + expect(respawn).not.toHaveBeenCalled(); + // And no new pane is made in its place either. + expect(create).not.toHaveBeenCalled(); + }); + + it('a session without a pane is asked before one is created', async () => { + mkdirSync(WORKSPACE, { recursive: true }); + const mux = new TmuxManager(); + const session = new Session({ workingDir: WORKSPACE, mode: 'claude', mux, useMux: true }); + sessions.push(session); + const create = vi.spyOn(mux, 'createSession'); + session.setLaunchGuard(async () => NOT_RUNNING); + await expect(session.startInteractive()).rejects.toThrow(NOT_RUNNING); + expect(create).not.toHaveBeenCalled(); + + // The guard taken away again: the pane is created. + session.setLaunchGuard(null); + create.mockResolvedValue(paneOf()); + await session.startInteractive(); + expect(create).toHaveBeenCalledTimes(1); + }); +}); + +describe('WebServer: every session asks the install service about its own selection', () => { + interface ServerInternals { + setupSessionListeners(session: Session): Promise; + discardPartiallyBuiltSession(sessionId: string): Promise; + } + let server: WebServer; + let internals: ServerInternals; + + beforeAll(() => { + // Test mode: no port is opened and no CLI is launched. + server = new WebServer(0, false, true); + internals = server as unknown as ServerInternals; + }); + + afterAll(async () => { + await server.stop().catch(() => {}); + }); + + async function wired(selection?: typeof SELECTION) { + const made = sessionWithPane(false); + if (selection) made.session.setCustomModel(selection, {}); + await internals.setupSessionListeners(made.session); + return made; + } + + it('a session on the managed endpoint is not restarted while the runtime is down', async () => { + writeInstalledState(); + const { session, respawn } = await wired(SELECTION); + await expect(session.restartCli()).rejects.toThrow(NOT_RUNNING); + expect(respawn).not.toHaveBeenCalled(); + await internals.discardPartiallyBuiltSession(session.id).catch(() => {}); + }); + + it('nor is its dead pane respawned, which is what a click on the tab and the boot recovery do', async () => { + writeInstalledState(); + const { session, mux, respawn } = await wired(SELECTION); + vi.spyOn(mux, 'isPaneDead').mockReturnValue(true); + await expect(session.startInteractive()).rejects.toThrow(NOT_RUNNING); + expect(respawn).not.toHaveBeenCalled(); + await internals.discardPartiallyBuiltSession(session.id).catch(() => {}); + }); + + it('only the model that was installed may be started on it', async () => { + writeInstalledState(); + const { session } = await wired({ ...SELECTION, modelId: 'something-pulled-by-hand' }); + await expect(session.restartCli()).rejects.toThrow(/Only Qwen3\.6 35B-A3B is installed on this Mac|not running/); + await internals.discardPartiallyBuiltSession(session.id).catch(() => {}); + }); + + it('a session on another endpoint, and one without a selection, start as before', async () => { + writeInstalledState(); + const other = await wired({ ...SELECTION, endpointId: 'office-gpu' }); + expect(await other.session.restartCli()).toBe(true); + await internals.discardPartiallyBuiltSession(other.session.id).catch(() => {}); + const plain = await wired(); + expect(await plain.session.restartCli()).toBe(true); + await internals.discardPartiallyBuiltSession(plain.session.id).catch(() => {}); + }); + + it('a selection made AFTER the listeners were wired is asked about too', async () => { + writeInstalledState(); + const { session, respawn } = await wired(); + session.setCustomModel(SELECTION, {}); + await expect(session.restartCli()).rejects.toThrow(NOT_RUNNING); + expect(respawn).not.toHaveBeenCalled(); + await internals.discardPartiallyBuiltSession(session.id).catch(() => {}); + }); +}); + +describe('POST /api/reboot-restore/restore: a session that ran on the managed endpoint', () => { + async function harness(ctx: ReturnType): Promise { + const app = Fastify({ logger: false }); + await app.register(fastifyCookie); + registerRebootRestoreRoutes(app, ctx as never); + app.addHook('preSerialization', (req, reply, payload: unknown, done) => { + if (payload === null || typeof payload !== 'object') return done(null, payload); + const p = payload as { success?: unknown; errorCode?: unknown }; + if (p.success === false) { + if (reply.statusCode === 200 && typeof p.errorCode === 'string') { + reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode)); + } + return done(null, payload); + } + if (p.success === true) return done(null, payload); + return done(null, { success: true, data: payload }); + }); + installRouteErrorHandler(app); + await app.ready(); + return app; + } + + /** An entry whose workspace exists, so only the model question can stop it. */ + function entryOn(endpointId: string): RebootRestoreEntry { + mkdirSync(WORKSPACE, { recursive: true }); + return { + sessionId: 'feedbeef00000001', + name: 'w1-app', + workingDir: WORKSPACE, + mode: 'claude', + resumeConversationId: 'conv-1', + state: { + id: 'feedbeef00000001', + pid: null, + status: 'idle', + workingDir: WORKSPACE, + currentTaskId: null, + createdAt: 1_760_000_000_000, + mode: 'claude', + __customModel: { endpointId, modelId: 'qwen3.6:35b-a3b', envKeys: [] }, + } as unknown as SessionState, + }; + } + + it('is not rebuilt while the model cannot be started on, says why, and stays on offer', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + writeInstalledState(); + rebootRestoreRegistry.set([entryOn('local-ollama')]); + const ctx = createMockRouteContext(); + const app = await harness(ctx); + const body = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; + expect(body.restored).toEqual([]); + expect(body.skipped).toEqual([{ sessionId: 'feedbeef00000001', reason: 'local-model-not-ready' }]); + // Nothing was built: no session was added, so there is nothing to discard. + expect(ctx.addSession).not.toHaveBeenCalled(); + // The model can be started and the click repeated. + const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; + expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['feedbeef00000001']); + await app.close(); + }); +}); diff --git a/test/routes/local-model-launch-guard.test.ts b/test/routes/local-model-launch-guard.test.ts index ce77f5f0..15bbfce7 100644 --- a/test/routes/local-model-launch-guard.test.ts +++ b/test/routes/local-model-launch-guard.test.ts @@ -1,6 +1,7 @@ /** - * @fileoverview The launch guard of the endpoint the local model install manages: both ways a - * session gets onto a custom model endpoint ask the install service first. + * @fileoverview The launch guard of the endpoint the local model install manages, at the two + * routes that put a session onto a custom model endpoint: they ask the install service before + * anything is written. * * - `POST /api/sessions/:id/custom-model` (a running session switches to the endpoint), and * - `POST /api/quick-start` with `customModel` (a session is created on it) @@ -11,6 +12,10 @@ * model whose digest is no longer the one stored at install time is not launched, and how * nothing is launched against a port our runtime is not on. * + * Every OTHER way a new agent process comes to be on the endpoint (the attach that finds no + * pane, the respawn of a dead pane, a restart in place, the rebuild after a reboot) is + * covered below the routes, by `Session`'s launch guard: test/local-model-session-launch-guard.test.ts. + * * The service is replaced by a fake for most cases, and is the REAL one (over the real state * file, with no runtime running) for the cases that pin the default wiring. *