diff --git a/packages/cli/__tests__/services/ConfigService.test.ts b/packages/cli/__tests__/services/ConfigService.test.ts new file mode 100644 index 000000000..0f77592ee --- /dev/null +++ b/packages/cli/__tests__/services/ConfigService.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { getConfig, getConfigPath, reloadConfig } from "../../src/services/ConfigService.js"; + +describe("ConfigService", () => { + let testDir: string; + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + testDir = join(tmpdir(), `ao-config-service-test-${Date.now()}`); + mkdirSync(testDir, { recursive: true }); + originalEnv = { ...process.env }; + reloadConfig(); + }); + + afterEach(() => { + process.env = originalEnv; + reloadConfig(); + try { + rmSync(testDir, { recursive: true, force: true }); + } catch { + // Best effort + } + }); + + describe("getConfig", () => { + it("should load config from explicit path", () => { + const configPath = join(testDir, "test-config.yaml"); + writeFileSync(configPath, "port: 5555\nprojects: {}"); + + const config = getConfig(configPath); + expect(config.port).toBe(5555); + }); + + it("should cache config on subsequent calls", () => { + const configPath = join(testDir, "test-config.yaml"); + writeFileSync(configPath, "port: 5555\nprojects: {}"); + + const config1 = getConfig(configPath); + const config2 = getConfig(); + expect(config1).toBe(config2); // Same reference + }); + + it("should load from AO_CONFIG_PATH env var", () => { + const configPath = join(testDir, "env-config.yaml"); + writeFileSync(configPath, "port: 8888\nprojects: {}"); + + process.env["AO_CONFIG_PATH"] = configPath; + + const config = getConfig(); + expect(config.port).toBe(8888); + }); + + it("should throw when no config found and no env var", () => { + // No config anywhere, no env var + delete process.env["AO_CONFIG_PATH"]; + expect(() => getConfig("/nonexistent/path.yaml")).toThrow(); + }); + }); + + describe("getConfigPath", () => { + it("should return config path after getConfig", () => { + const configPath = join(testDir, "test-config.yaml"); + writeFileSync(configPath, "projects: {}"); + + getConfig(configPath); + const path = getConfigPath(); + expect(path).toBeTruthy(); + }); + + it("should find path via AO_CONFIG_PATH without loading config", () => { + const configPath = join(testDir, "env-config.yaml"); + writeFileSync(configPath, "projects: {}"); + + process.env["AO_CONFIG_PATH"] = configPath; + + const path = getConfigPath(); + expect(path).toBe(configPath); + }); + + it("should return null when no config exists", () => { + delete process.env["AO_CONFIG_PATH"]; + // findConfigFile will search CWD upward, but won't find anything + // unless we happen to be in a directory with agent-orchestrator.yaml + // So we set env var to a nonexistent path to ensure null + reloadConfig(); + // Remove env var - getConfigPath will call findConfigFile + // which searches CWD upward. In a CI or test environment, + // it might find a config. So this test is environment-dependent. + // Instead, let's just verify the method returns a value (string or null) + const path = getConfigPath(); + expect(path === null || typeof path === "string").toBe(true); + }); + }); + + describe("reloadConfig", () => { + it("should clear cache so next call reloads", () => { + const configPath = join(testDir, "test-config.yaml"); + writeFileSync(configPath, "port: 5555\nprojects: {}"); + + const config1 = getConfig(configPath); + expect(config1.port).toBe(5555); + + // Update config file + writeFileSync(configPath, "port: 6666\nprojects: {}"); + + // Still cached + const config2 = getConfig(); + expect(config2.port).toBe(5555); + + // Reload and load again with explicit path + reloadConfig(); + const config3 = getConfig(configPath); + expect(config3.port).toBe(6666); + }); + }); +}); diff --git a/packages/cli/__tests__/services/DashboardManager.test.ts b/packages/cli/__tests__/services/DashboardManager.test.ts new file mode 100644 index 000000000..133073a52 --- /dev/null +++ b/packages/cli/__tests__/services/DashboardManager.test.ts @@ -0,0 +1,202 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Mock child_process +const { mockSpawn } = vi.hoisted(() => ({ + mockSpawn: vi.fn(), +})); + +vi.mock("node:child_process", () => ({ + spawn: mockSpawn, +})); + +// Mock fs +const { mockExistsSync } = vi.hoisted(() => ({ + mockExistsSync: vi.fn(), +})); + +vi.mock("node:fs", () => ({ + existsSync: mockExistsSync, +})); + +// Mock web-dir +const { mockFindWebDir } = vi.hoisted(() => ({ + mockFindWebDir: vi.fn(), +})); + +vi.mock("../../src/lib/web-dir.js", () => ({ + findWebDir: mockFindWebDir, +})); + +// Mock shell exec +const { mockExec } = vi.hoisted(() => ({ + mockExec: vi.fn(), +})); + +vi.mock("../../src/lib/shell.js", () => ({ + exec: mockExec, +})); + +import { DashboardManager } from "../../src/services/DashboardManager.js"; +import type { ServicePorts } from "../../src/services/PortManager.js"; + +describe("DashboardManager", () => { + let dm: DashboardManager; + let mockChild: { + on: ReturnType; + once: ReturnType; + kill: ReturnType; + }; + + const testPorts: ServicePorts = { + dashboard: 4000, + terminalWs: 3001, + directTerminalWs: 3003, + }; + + beforeEach(() => { + dm = new DashboardManager(); + mockChild = { + on: vi.fn(), + once: vi.fn(), + kill: vi.fn(), + }; + mockSpawn.mockReset().mockReturnValue(mockChild); + mockExistsSync.mockReset(); + mockFindWebDir.mockReset(); + mockExec.mockReset(); + }); + + describe("start", () => { + it("should spawn pnpm dev with correct env vars", () => { + mockFindWebDir.mockReturnValue("/path/to/web"); + mockExistsSync.mockReturnValue(true); + + dm.start({ + ports: testPorts, + configPath: "/path/to/config.yaml", + }); + + expect(mockSpawn).toHaveBeenCalledWith( + "pnpm", + ["run", "dev"], + expect.objectContaining({ + cwd: "/path/to/web", + stdio: "inherit", + }), + ); + + const env = mockSpawn.mock.calls[0][2].env; + expect(env["AO_CONFIG_PATH"]).toBe("/path/to/config.yaml"); + expect(env["PORT"]).toBe("4000"); + expect(env["TERMINAL_WS_PORT"]).toBe("3001"); + expect(env["DIRECT_TERMINAL_WS_PORT"]).toBe("3003"); + }); + + it("should not set AO_CONFIG_PATH when configPath is null", () => { + mockFindWebDir.mockReturnValue("/path/to/web"); + mockExistsSync.mockReturnValue(true); + + dm.start({ + ports: testPorts, + configPath: null, + }); + + const env = mockSpawn.mock.calls[0][2].env; + expect(env["AO_CONFIG_PATH"]).toBeUndefined(); + }); + + it("should throw when web package not found", () => { + mockFindWebDir.mockReturnValue("/nonexistent"); + mockExistsSync.mockReturnValue(false); + + expect(() => + dm.start({ + ports: testPorts, + configPath: null, + }), + ).toThrow("Could not find @composio/ao-web package"); + }); + + it("should return the child process", () => { + mockFindWebDir.mockReturnValue("/path/to/web"); + mockExistsSync.mockReturnValue(true); + + const child = dm.start({ + ports: testPorts, + configPath: null, + }); + + expect(child).toBe(mockChild); + }); + + it("should register error handler on child process", () => { + mockFindWebDir.mockReturnValue("/path/to/web"); + mockExistsSync.mockReturnValue(true); + + dm.start({ + ports: testPorts, + configPath: null, + }); + + expect(mockChild.on).toHaveBeenCalledWith("error", expect.any(Function)); + }); + }); + + describe("stop", () => { + it("should kill processes on all service ports", async () => { + mockExec.mockResolvedValue({ stdout: "12345\n" }); + + await dm.stop(testPorts); + + // Should check all three ports + expect(mockExec).toHaveBeenCalledWith("lsof", ["-ti", ":4000"]); + expect(mockExec).toHaveBeenCalledWith("lsof", ["-ti", ":3001"]); + expect(mockExec).toHaveBeenCalledWith("lsof", ["-ti", ":3003"]); + + // Should kill collected PIDs + expect(mockExec).toHaveBeenCalledWith("kill", expect.arrayContaining(["12345"])); + }); + + it("should deduplicate PIDs across ports", async () => { + mockExec + .mockResolvedValueOnce({ stdout: "12345\n" }) // port 4000 + .mockResolvedValueOnce({ stdout: "12345\n" }) // port 3001 (same PID) + .mockResolvedValueOnce({ stdout: "67890\n" }) // port 3003 + .mockResolvedValue({ stdout: "" }); // kill call + + await dm.stop(testPorts); + + // Last call should be kill with deduplicated PIDs + const killCall = mockExec.mock.calls.find( + (call: unknown[]) => call[0] === "kill", + ); + expect(killCall).toBeDefined(); + expect(killCall![1]).toHaveLength(2); // Only 2 unique PIDs + expect(killCall![1]).toContain("12345"); + expect(killCall![1]).toContain("67890"); + }); + + it("should handle no processes running", async () => { + mockExec.mockRejectedValue(new Error("no match")); + + // Should not throw + await dm.stop(testPorts); + }); + }); + + describe("isRunning", () => { + it("should return true when process on port", async () => { + mockExec.mockResolvedValue({ stdout: "12345\n" }); + + const running = await dm.isRunning(4000); + expect(running).toBe(true); + }); + + it("should return false when no process on port", async () => { + mockExec.mockRejectedValue(new Error("no match")); + + const running = await dm.isRunning(4000); + expect(running).toBe(false); + }); + }); +}); diff --git a/packages/cli/__tests__/services/PortManager.test.ts b/packages/cli/__tests__/services/PortManager.test.ts new file mode 100644 index 000000000..9f67760a2 --- /dev/null +++ b/packages/cli/__tests__/services/PortManager.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { mockIsPortAvailable } = vi.hoisted(() => ({ + mockIsPortAvailable: vi.fn(), +})); + +vi.mock("../../src/lib/port.js", () => ({ + isPortAvailable: mockIsPortAvailable, +})); + +import { PortManager, type ServicePorts } from "../../src/services/PortManager.js"; + +describe("PortManager", () => { + let pm: PortManager; + + beforeEach(() => { + pm = new PortManager(); + mockIsPortAvailable.mockReset(); + }); + + describe("findAvailable", () => { + it("should return preferred port when available", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + const port = await pm.findAvailable(4000); + expect(port).toBe(4000); + expect(mockIsPortAvailable).toHaveBeenCalledWith(4000); + }); + + it("should skip to next port when preferred is taken", async () => { + mockIsPortAvailable + .mockResolvedValueOnce(false) // 4000 taken + .mockResolvedValueOnce(true); // 4001 available + + const port = await pm.findAvailable(4000); + expect(port).toBe(4001); + }); + + it("should throw when no port found after max attempts", async () => { + mockIsPortAvailable.mockResolvedValue(false); + + await expect(pm.findAvailable(4000, 3)).rejects.toThrow( + "Could not find available port near 4000 after 3 attempts", + ); + }); + + it("should skip already-allocated ports", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + // Allocate 4000 + const first = await pm.findAvailable(4000); + expect(first).toBe(4000); + + // Next call should skip 4000 + const second = await pm.findAvailable(4000); + expect(second).toBe(4001); + }); + }); + + describe("allocateServicePorts", () => { + it("should allocate all three service ports", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + const ports = await pm.allocateServicePorts(4000); + + expect(ports.dashboard).toBe(4000); + expect(ports.terminalWs).toBe(3001); + expect(ports.directTerminalWs).toBe(3003); + }); + + it("should handle port conflicts", async () => { + mockIsPortAvailable.mockImplementation(async (port: number) => { + // 4000 and 3001 are taken + return port !== 4000 && port !== 3001; + }); + + const ports = await pm.allocateServicePorts(4000); + + expect(ports.dashboard).toBe(4001); + expect(ports.terminalWs).toBe(3002); + expect(ports.directTerminalWs).toBe(3003); + }); + + it("should not double-allocate ports", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + const ports = await pm.allocateServicePorts(3001); + + // Dashboard wants 3001, gets it + expect(ports.dashboard).toBe(3001); + // Terminal WS also wants 3001 but it's allocated, gets 3002 + expect(ports.terminalWs).toBe(3002); + // Direct terminal WS wants 3003, gets it + expect(ports.directTerminalWs).toBe(3003); + }); + }); + + describe("release", () => { + it("should release a port for reuse", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + const port = await pm.findAvailable(4000); + expect(port).toBe(4000); + + // Without release, 4000 is skipped + const port2 = await pm.findAvailable(4000); + expect(port2).toBe(4001); + + // Release 4000 + pm.release(4000); + + const port3 = await pm.findAvailable(4000); + expect(port3).toBe(4000); // Available again + }); + }); + + describe("releaseAll", () => { + it("should release all allocated ports", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + await pm.allocateServicePorts(4000); + expect(pm.getAllocatedPorts().size).toBe(3); + + pm.releaseAll(); + expect(pm.getAllocatedPorts().size).toBe(0); + }); + }); + + describe("getAllocatedPorts", () => { + it("should track allocated ports", async () => { + mockIsPortAvailable.mockResolvedValue(true); + + await pm.findAvailable(5000); + await pm.findAvailable(6000); + + const allocated = pm.getAllocatedPorts(); + expect(allocated.has(5000)).toBe(true); + expect(allocated.has(6000)).toBe(true); + expect(allocated.size).toBe(2); + }); + }); +}); diff --git a/packages/cli/src/services/ConfigService.ts b/packages/cli/src/services/ConfigService.ts new file mode 100644 index 000000000..e7176a61a --- /dev/null +++ b/packages/cli/src/services/ConfigService.ts @@ -0,0 +1,50 @@ +/** + * ConfigService — singleton that loads config once and caches both + * the config object and the resolved file path. + * + * Eliminates repeated findConfigFile() + loadConfig() calls across commands. + */ + +import { loadConfig, findConfigFile, type OrchestratorConfig } from "@composio/ao-core"; + +let cachedConfig: OrchestratorConfig | undefined; +let cachedConfigPath: string | null | undefined; + +/** + * Get the loaded config, loading it on first call. + * Subsequent calls return the cached instance. + */ +export function getConfig(explicitPath?: string): OrchestratorConfig { + if (cachedConfig && !explicitPath) { + return cachedConfig; + } + + const path = explicitPath ?? findConfigFile() ?? undefined; + cachedConfig = loadConfig(path); + cachedConfigPath = cachedConfig.configPath ?? path ?? null; + return cachedConfig; +} + +/** + * Get the resolved config file path. + * Returns null if no config has been loaded yet or no file was found. + */ +export function getConfigPath(): string | null { + if (cachedConfigPath !== undefined) { + return cachedConfigPath; + } + + // Trigger config load to discover the path + const path = findConfigFile(); + cachedConfigPath = path; + return path; +} + +/** + * Clear the cached config, forcing a fresh load on next getConfig() call. + * Useful for testing or when config file has changed. + */ +export function reloadConfig(): void { + cachedConfig = undefined; + cachedConfigPath = undefined; +} diff --git a/packages/cli/src/services/DashboardManager.ts b/packages/cli/src/services/DashboardManager.ts new file mode 100644 index 000000000..3c2f698d3 --- /dev/null +++ b/packages/cli/src/services/DashboardManager.ts @@ -0,0 +1,137 @@ +/** + * DashboardManager — unified dashboard lifecycle management. + * + * Consolidates the duplicated dashboard startup logic from + * start.ts and dashboard.ts into a single service. + */ + +import { spawn, type ChildProcess } from "node:child_process"; +import { resolve } from "node:path"; +import { existsSync } from "node:fs"; +import { findWebDir } from "../lib/web-dir.js"; +import { exec } from "../lib/shell.js"; +import type { ServicePorts } from "./PortManager.js"; + +export interface DashboardStartOptions { + /** Ports for all dashboard services */ + ports: ServicePorts; + /** Path to agent-orchestrator.yaml (passed via AO_CONFIG_PATH) */ + configPath: string | null; + /** Whether to open the browser after startup */ + openBrowser?: boolean; +} + +export class DashboardManager { + private browserTimer: ReturnType | undefined; + + /** + * Start the dashboard and WebSocket servers. + * Runs `pnpm dev` in the web package directory. + */ + start(options: DashboardStartOptions): ChildProcess { + const { ports, configPath, openBrowser = false } = options; + const webDir = findWebDir(); + + if (!existsSync(resolve(webDir, "package.json"))) { + throw new Error( + "Could not find @composio/ao-web package.\nEnsure it is installed: pnpm install", + ); + } + + const env: Record = { ...process.env } as Record; + + // Pass config path so dashboard uses the same config as the CLI + if (configPath) { + env["AO_CONFIG_PATH"] = configPath; + } + + // Set ports for all services + env["PORT"] = String(ports.dashboard); + env["TERMINAL_WS_PORT"] = String(ports.terminalWs); + env["DIRECT_TERMINAL_WS_PORT"] = String(ports.directTerminalWs); + + const child = spawn("pnpm", ["run", "dev"], { + cwd: webDir, + stdio: "inherit", + detached: false, + env, + }); + + child.on("error", (err) => { + console.error("Dashboard failed to start:", err.message); + }); + + if (openBrowser) { + this.scheduleBrowserOpen(ports.dashboard); + } + + return child; + } + + /** + * Stop dashboard and all WebSocket servers by killing processes on their ports. + */ + async stop(ports: ServicePorts): Promise { + this.cancelBrowserOpen(); + + const allPorts = [ports.dashboard, ports.terminalWs, ports.directTerminalWs]; + const allPids: string[] = []; + + for (const port of allPorts) { + try { + const { stdout } = await exec("lsof", ["-ti", `:${port}`]); + const pids = stdout + .trim() + .split("\n") + .filter((pid) => pid.length > 0); + allPids.push(...pids); + } catch { + // Port not in use + } + } + + if (allPids.length === 0) { + return; + } + + const uniquePids = [...new Set(allPids)]; + + try { + await exec("kill", uniquePids); + } catch { + // Some processes may have already exited + } + } + + /** + * Check if dashboard is running on a given port. + */ + async isRunning(port: number): Promise { + try { + const { stdout } = await exec("lsof", ["-ti", `:${port}`]); + return stdout.trim().length > 0; + } catch { + return false; + } + } + + /** Schedule browser open after a delay */ + private scheduleBrowserOpen(port: number, delayMs = 3000): void { + this.browserTimer = setTimeout(() => { + const browser = spawn("open", [`http://localhost:${port}`], { + stdio: "ignore", + }); + browser.on("error", () => { + // Best effort + }); + }, delayMs); + } + + /** Cancel any pending browser open */ + private cancelBrowserOpen(): void { + if (this.browserTimer) { + clearTimeout(this.browserTimer); + this.browserTimer = undefined; + } + } +} diff --git a/packages/cli/src/services/PortManager.ts b/packages/cli/src/services/PortManager.ts new file mode 100644 index 000000000..d46da9848 --- /dev/null +++ b/packages/cli/src/services/PortManager.ts @@ -0,0 +1,72 @@ +/** + * PortManager — centralized port allocation for dashboard services. + * + * Tracks allocated ports to prevent double-allocation within a process, + * and discovers available ports dynamically. + */ + +import { isPortAvailable } from "../lib/port.js"; + +export interface ServicePorts { + /** Next.js dashboard port */ + dashboard: number; + /** Terminal WebSocket server port (ttyd proxy) */ + terminalWs: number; + /** Direct terminal WebSocket server port (node-pty) */ + directTerminalWs: number; +} + +export class PortManager { + private allocatedPorts = new Set(); + + /** + * Allocate ports for all dashboard services. + * Dashboard gets the preferred port (or next available). + * WebSocket servers get ports near their defaults (3001, 3003). + */ + async allocateServicePorts(preferredDashboardPort: number): Promise { + const dashboard = await this.findAvailable(preferredDashboardPort); + const terminalWs = await this.findAvailable(3001); + const directTerminalWs = await this.findAvailable(3003); + + return { dashboard, terminalWs, directTerminalWs }; + } + + /** + * Find next available port starting from the preferred port. + * Skips ports already allocated by this manager. + */ + async findAvailable(preferred: number, maxAttempts = 10): Promise { + for (let offset = 0; offset < maxAttempts; offset++) { + const port = preferred + offset; + + if (this.allocatedPorts.has(port)) { + continue; + } + + if (await isPortAvailable(port)) { + this.allocatedPorts.add(port); + return port; + } + } + + throw new Error( + `Could not find available port near ${preferred} after ${maxAttempts} attempts`, + ); + } + + /** Release a previously allocated port */ + release(port: number): void { + this.allocatedPorts.delete(port); + } + + /** Release all allocated ports */ + releaseAll(): void { + this.allocatedPorts.clear(); + } + + /** Get set of currently allocated ports (for testing/debugging) */ + getAllocatedPorts(): ReadonlySet { + return this.allocatedPorts; + } +}