Fix Tower startup checks and shellper cleanup

packages/codev/src/agent-farm/__tests__/tower-command.test.ts

@@ -0,0 +1,76 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const mockKillScopedShellpers = vi.fn();
+
+vi.mock('../../terminal/session-manager.js', () => ({
+  SessionManager: vi.fn(function SessionManagerMock() {
+    return { killScopedShellpers: mockKillScopedShellpers };
+  }),
+}));
+
+vi.mock('../utils/config.js', () => ({
+  getConfig: vi.fn(() => ({ serversDir: '/tmp/codev-test-servers' })),
+}));
+
+vi.mock('../utils/logger.js', () => ({
+  logger: {
+    header: vi.fn(),
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    success: vi.fn(),
+    kv: vi.fn(),
+    blank: vi.fn(),
+  },
+  fatal: vi.fn((message: string) => {
+    throw new Error(message);
+  }),
+}));
+
+vi.mock('../lib/tower-client.js', () => ({
+  DEFAULT_TOWER_PORT: 4100,
+  AGENT_FARM_DIR: '/tmp/codev-test-agent-farm',
+}));
+
+vi.mock('node:child_process', async () => {
+  const actual = await vi.importActual<typeof import('node:child_process')>('node:child_process');
+  return {
+    ...actual,
+    spawn: vi.fn(),
+    execSync: vi.fn(() => {
+      throw new Error('no process on port');
+    }),
+  };
+});
+
+describe('tower command lifecycle options', () => {
+  beforeEach(() => {
+    mockKillScopedShellpers.mockReset();
+  });
+
+  it('waits for tower start readiness by default', async () => {
+    const { shouldWaitForTowerStart } = await import('../commands/tower.js');
+
+    expect(shouldWaitForTowerStart()).toBe(true);
+    expect(shouldWaitForTowerStart({ wait: undefined })).toBe(true);
+    expect(shouldWaitForTowerStart({ wait: false })).toBe(false);
+  });
+
+  it('cleans scoped shellpers on explicit stop by default when tower is already stopped', async () => {
+    mockKillScopedShellpers.mockResolvedValue(2);
+    const { towerStop } = await import('../commands/tower.js');
+
+    await towerStop({ port: 49_123 });
+
+    expect(mockKillScopedShellpers).toHaveBeenCalledTimes(1);
+  });
+
+  it('preserves scoped shellpers when requested', async () => {
+    mockKillScopedShellpers.mockResolvedValue(2);
+    const { towerStop } = await import('../commands/tower.js');
+
+    await towerStop({ port: 49_123, preserveShellpers: true });
+
+    expect(mockKillScopedShellpers).not.toHaveBeenCalled();
+  });
+});

packages/codev/src/agent-farm/cli.ts

@@ -637,9 +637,9 @@ export async function runAgentFarm(args: string[]): Promise<void> {
 
   towerCmd
     .command('start')
-    .description('Start the tower dashboard (daemonizes by default)')
+    .description('Start the tower dashboard and wait for readiness by default')
     .option('-p, --port <port>', 'Port to run on (default: 4100)')
-    .option('--wait', 'Wait for server to start before returning')
+    .option('--no-wait', 'Daemonize without waiting for readiness')
     .action(async (options) => {
       try {
         await towerStart({
@@ -656,12 +656,14 @@ export async function runAgentFarm(args: string[]): Promise<void> {
     .command('stop')
     .description('Stop the tower dashboard')
     .option('-p, --port <port>', 'Port to stop (default: 4100)')
+    .option('--preserve-shellpers', 'Stop Tower but leave scoped shellper processes running')
     .option('--force-kill-all-child-processes', 'SIGKILL tower and every child process (builders, shells, everything)')
     .action(async (options) => {
       try {
         await towerStop({
           port: options.port ? parseInt(options.port, 10) : undefined,
           forceKillAllChildProcesses: options.forceKillAllChildProcesses,
+          preserveShellpers: options.preserveShellpers,
         });
       } catch (error) {
         logger.error(error instanceof Error ? error.message : String(error));

packages/codev/src/agent-farm/commands/tower.ts

@@ -2,6 +2,7 @@
  * Tower command - launches the tower dashboard showing all instances
  */
 
+import { homedir } from 'node:os';
 import { resolve } from 'node:path';
 import { existsSync, mkdirSync, appendFileSync } from 'node:fs';
 import http from 'node:http';
@@ -11,6 +12,7 @@ import { getConfig } from '../utils/config.js';
 import { execSync } from 'node:child_process';
 import { DEFAULT_TOWER_PORT, AGENT_FARM_DIR } from '../lib/tower-client.js';
 import { isPortAvailable } from '../utils/shell.js';
+import { SessionManager } from '../../terminal/session-manager.js';
 
 // Log file location
 const LOG_FILE = resolve(AGENT_FARM_DIR, 'tower.log');
@@ -21,12 +23,17 @@ const STARTUP_CHECK_INTERVAL_MS = 200;
 
 export interface TowerStartOptions {
   port?: number;
-  wait?: boolean; // Wait for server to start before returning
+  wait?: boolean; // Defaults to true. Set false for fire-and-forget startup.
 }
 
 export interface TowerStopOptions {
   port?: number;
   forceKillAllChildProcesses?: boolean;
+  preserveShellpers?: boolean;
+}
+
+export function shouldWaitForTowerStart(options: TowerStartOptions = {}): boolean {
+  return options.wait ?? true;
 }
 
 /**
@@ -112,6 +119,7 @@ function getProcessesOnPort(port: number): number[] {
  */
 export async function towerStart(options: TowerStartOptions = {}): Promise<void> {
   const port = options.port || DEFAULT_TOWER_PORT;
+  const wait = shouldWaitForTowerStart(options);
 
   // Check if already running and responding
   if (await isServerResponding(port)) {
@@ -185,7 +193,7 @@ export async function towerStart(options: TowerStartOptions = {}): Promise<void>
 
   const dashboardUrl = `http://localhost:${port}`;
 
-  if (options.wait) {
+  if (wait) {
     // Wait for server to actually start responding
     logger.info('Waiting for server to start...');
     const started = await waitForServer(port);
@@ -210,19 +218,41 @@ export async function towerStart(options: TowerStartOptions = {}): Promise<void>
   }
 }
 
+function getShellperSocketDir(): string {
+  return process.env.SHELLPER_SOCKET_DIR || resolve(homedir(), '.codev', 'run');
+}
+
+async function cleanupScopedShellpers(): Promise<number> {
+  const config = getConfig();
+  const manager = new SessionManager({
+    socketDir: getShellperSocketDir(),
+    shellperScript: resolve(config.serversDir, '../terminal/shellper-main.js'),
+    nodeExecutable: process.execPath,
+    logger: (message) => logToFile(message),
+  });
+  return manager.killScopedShellpers();
+}
+
 /**
  * Stop the tower dashboard
  */
 export async function towerStop(options: TowerStopOptions = {}): Promise<void> {
   const port = options.port || DEFAULT_TOWER_PORT;
   const forceKill = options.forceKillAllChildProcesses || false;
+  const preserveShellpers = options.preserveShellpers || false;
 
   logger.header(forceKill ? 'Force-Killing Tower and All Child Processes' : 'Stopping Tower');
 
   const pids = getProcessesOnPort(port);
 
   if (pids.length === 0) {
     logger.info('Tower is not running');
+    if (!preserveShellpers) {
+      const killed = await cleanupScopedShellpers();
+      if (killed > 0) {
+        logger.success(`Cleaned up ${killed} scoped shellper process${killed > 1 ? 'es' : ''}`);
+      }
+    }
     return;
   }
 
@@ -289,6 +319,18 @@ export async function towerStop(options: TowerStopOptions = {}): Promise<void> {
   if (stopped > 0) {
     logger.success(`Tower stopped (${stopped} process${stopped > 1 ? 'es' : ''}: PIDs ${pids.join(', ')})`);
   }
+
+  if (preserveShellpers) {
+    logger.info('Preserving shellper processes');
+    return;
+  }
+
+  const killed = await cleanupScopedShellpers();
+  if (killed > 0) {
+    logger.success(`Cleaned up ${killed} scoped shellper process${killed > 1 ? 'es' : ''}`);
+  } else {
+    logger.info('No scoped shellper processes found');
+  }
 }
 
 export interface TowerLogOptions {

packages/codev/src/terminal/__tests__/session-manager.test.ts

@@ -559,6 +559,65 @@ describe('SessionManager', () => {
       }
     });
 
+    it('kills scoped shellpers without probing responsive sockets', async () => {
+      const liveSocketPath = path.join(socketDir, 'shellper-live-cleanup.sock');
+
+      const logs: string[] = [];
+      const manager = new SessionManager({
+        socketDir,
+        shellperScript: '/nonexistent/shellper.js',
+        nodeExecutable: process.execPath,
+        logger: (msg) => logs.push(msg),
+      });
+
+      vi.spyOn(manager as any, 'findShellperProcesses').mockResolvedValue([
+        { pid: 7777, socketPath: liveSocketPath },
+      ]);
+      vi.spyOn(manager as any, 'waitForProcessExit').mockResolvedValue(true);
+      const probeSpy = vi.spyOn(manager as any, 'probeSocket');
+
+      const killed: Array<{ pid: number; signal: string }> = [];
+      const originalKill = process.kill;
+      process.kill = ((pid: number, signal?: string | number) => {
+        killed.push({ pid, signal: String(signal || 'SIGTERM') });
+        return true;
+      }) as typeof process.kill;
+
+      try {
+        const count = await manager.killScopedShellpers();
+        expect(count).toBe(1);
+        expect(probeSpy).not.toHaveBeenCalled();
+        expect(killed).toContainEqual({ pid: -7777, signal: 'SIGTERM' });
+        expect(logs.some(m => m.includes('Killing scoped shellper process: pid=7777'))).toBe(true);
+      } finally {
+        process.kill = originalKill;
+      }
+    });
+
+    it('killScopedShellpers does not kill shellpers from another socket dir', async () => {
+      const uniqueDir = `/tmp/codev-scoped-cleanup-test-${Date.now()}-${Math.random().toString(36)}`;
+      const manager = new SessionManager({
+        socketDir: uniqueDir,
+        shellperScript: '/nonexistent/shellper.js',
+        nodeExecutable: process.execPath,
+      });
+
+      const killed: number[] = [];
+      const originalKill = process.kill;
+      process.kill = ((pid: number) => {
+        killed.push(pid);
+        return true;
+      }) as typeof process.kill;
+
+      try {
+        const count = await manager.killScopedShellpers();
+        expect(count).toBe(0);
+        expect(killed).toEqual([]);
+      } finally {
+        process.kill = originalKill;
+      }
+    });
+
     it('returns 0 when no orphans found', async () => {
       const manager = new SessionManager({
         socketDir,
@@ -795,6 +854,30 @@ describe('SessionManager', () => {
       try { process.kill(pid, 0); alive = true; } catch { /* ESRCH = dead, good */ }
       expect(alive).toBe(false);
     }, 20000);
+
+    it('surfaces shellper stderr when startup info is missing', async () => {
+      const failScript = path.join(socketDir, 'fail-before-info.js');
+      fs.writeFileSync(failScript, [
+        `process.stderr.write('node-pty failed: posix_spawnp failed\\n');`,
+        `process.exit(1);`,
+      ].join('\n'));
+
+      const manager = new SessionManager({
+        socketDir,
+        shellperScript: failScript,
+        nodeExecutable: process.execPath,
+      });
+
+      await expect(manager.createSession({
+        sessionId: 'stderr-startup-failure',
+        command: '/bin/echo',
+        args: [],
+        cwd: '/tmp',
+        env: { PATH: process.env.PATH || '/usr/bin:/bin', SECRET_VALUE: 'do-not-log' },
+        cols: 80,
+        rows: 24,
+      })).rejects.toThrow(/(Shellper exited with code 1 before writing info|Invalid shellper info JSON)[\s\S]*posix_spawnp failed/);
+    }, 15000);
   });
 
   describe('killSession', () => {