diff --git a/packages/cli/__tests__/commands/start.test.ts b/packages/cli/__tests__/commands/start.test.ts index c4de94e6d..fdbe4f49f 100644 --- a/packages/cli/__tests__/commands/start.test.ts +++ b/packages/cli/__tests__/commands/start.test.ts @@ -333,7 +333,11 @@ beforeEach(async () => { vi.mocked(webDir.findFreePort).mockResolvedValue(3000); vi.mocked(webDir.buildDashboardEnv).mockResolvedValue({}); const projectDetection = await import("../../src/lib/project-detection.js"); - vi.mocked(projectDetection.detectProjectType).mockReturnValue({ languages: [], frameworks: [], tools: [] }); + vi.mocked(projectDetection.detectProjectType).mockReturnValue({ + languages: [], + frameworks: [], + tools: [], + }); vi.mocked(projectDetection.generateRulesFromTemplates).mockReturnValue(null); vi.mocked(projectDetection.formatProjectTypeForDisplay).mockReturnValue(""); @@ -434,7 +438,10 @@ function makeConfig(projects: Record>): Record { mockExecSilent.mockResolvedValue("Logged in"); mockSpawn.mockImplementation( - ( - cmd: string, - args: string[], - _opts?: { cwd?: string; env?: NodeJS.ProcessEnv }, - ) => { - if (cmd === "gh" && args[0] === "repo" && args[1] === "clone") { - createFakeRepo(repoDir, "https://github.com/owner/my-app.git", { - "Cargo.toml": "", - }); - } - return createSpawnChild({ closeCode: 0 }); + (cmd: string, args: string[], _opts?: { cwd?: string; env?: NodeJS.ProcessEnv }) => { + if (cmd === "gh" && args[0] === "repo" && args[1] === "clone") { + createFakeRepo(repoDir, "https://github.com/owner/my-app.git", { + "Cargo.toml": "", + }); + } + return createSpawnChild({ closeCode: 0 }); }, ); @@ -705,25 +708,21 @@ describe("start command — URL argument", () => { }); mockSpawn.mockImplementation( - ( - cmd: string, - args: string[], - _opts?: { cwd?: string; env?: NodeJS.ProcessEnv }, - ) => { - if (cmd === "git" && args[0] === "clone") { - const url = String(args[3] ?? ""); - // SSH attempt fails (simulate non-zero exit) - if (url.startsWith("git@")) { - return createSpawnChild({ closeCode: 1 }); + (cmd: string, args: string[], _opts?: { cwd?: string; env?: NodeJS.ProcessEnv }) => { + if (cmd === "git" && args[0] === "clone") { + const url = String(args[3] ?? ""); + // SSH attempt fails (simulate non-zero exit) + if (url.startsWith("git@")) { + return createSpawnChild({ closeCode: 1 }); + } + + // HTTPS fallback succeeds + createFakeRepo(repoDir, "https://github.com/owner/my-app.git", { + "Cargo.toml": "", + }); } - // HTTPS fallback succeeds - createFakeRepo(repoDir, "https://github.com/owner/my-app.git", { - "Cargo.toml": "", - }); - } - - return createSpawnChild({ closeCode: 0 }); + return createSpawnChild({ closeCode: 0 }); }, ); @@ -765,7 +764,7 @@ describe("start command — URL argument", () => { [ "port: 4000", "defaults:", - " runtime: tmux", + " runtime: process", " agent: claude-code", " workspace: worktree", " notifiers: [desktop]", @@ -806,7 +805,7 @@ describe("start command — URL argument", () => { [ "port: 4000", "defaults:", - " runtime: tmux", + " runtime: process", " agent: claude-code", " workspace: worktree", " notifiers: [desktop]", @@ -882,7 +881,20 @@ describe("start command — non-interactive install safety", () => { it("does not auto-install tmux when missing in non-interactive mode", async () => { mockIsHumanCaller.mockReturnValue(false); - mockConfigRef.current = makeConfig({ "my-app": makeProject() }); + // This test exercises the tmux preflight path, so the config must + // explicitly select runtime: tmux (makeConfig defaults to process). + // Pin the platform to linux so the Windows branch (which exits before + // calling execSilent) doesn't short-circuit the tmux -V check we're + // asserting on. + const tmuxConfig = makeConfig({ "my-app": makeProject() }) as { + defaults: Record; + }; + tmuxConfig.defaults.runtime = "tmux"; + mockConfigRef.current = tmuxConfig; + + const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + mockExecSilent.mockImplementation(async (cmd: string, args: string[] = []) => { if (cmd === "git" && args[0] === "--version") return "git version 2.43.0"; if (cmd === "tmux" && args[0] === "-V") return null; @@ -891,9 +903,15 @@ describe("start command — non-interactive install safety", () => { return null; }); - await expect( - program.parseAsync(["node", "test", "start", "--no-dashboard", "--no-orchestrator"]), - ).rejects.toThrow("process.exit(1)"); + try { + await expect( + program.parseAsync(["node", "test", "start", "--no-dashboard", "--no-orchestrator"]), + ).rejects.toThrow("process.exit(1)"); + } finally { + if (originalPlatform) { + Object.defineProperty(process, "platform", originalPlatform); + } + } expect(hasPrivilegedInstallAttempt()).toBe(false); expect(mockExec.mock.calls.some((call) => String(call[0]) === "tmux")).toBe(false); @@ -1238,10 +1256,10 @@ describe("start command — orchestrator session strategy display", () => { await program.parseAsync(["node", "test", "start", "--rebuild", "--no-orchestrator"]); - expect(dashboardRebuild.rebuildDashboardProductionArtifacts).toHaveBeenCalledWith(tmpDir, [ - 3000, - 3001, - ]); + expect(dashboardRebuild.rebuildDashboardProductionArtifacts).toHaveBeenCalledWith( + tmpDir, + [3000, 3001], + ); }); it("opens the most recent orchestrator session page when multiple existing orchestrators found with dashboard enabled and reuse is explicit", async () => { @@ -1850,7 +1868,8 @@ describe("start command — platform-aware runtime fallback", () => { // ensureTmux() calls execSilent("tmux", ["-V"]) — it must NOT have been called. const tmuxChecks = mockExecSilent.mock.calls.filter( - (call) => String(call[0]) === "tmux" && Array.isArray(call[1]) && (call[1] as string[])[0] === "-V", + (call) => + String(call[0]) === "tmux" && Array.isArray(call[1]) && (call[1] as string[])[0] === "-V", ); expect(tmuxChecks).toHaveLength(0); }); @@ -1886,7 +1905,8 @@ describe("start command — platform-aware runtime fallback", () => { // ensureTmux() must have checked for tmux availability. const tmuxChecks = mockExecSilent.mock.calls.filter( - (call) => String(call[0]) === "tmux" && Array.isArray(call[1]) && (call[1] as string[])[0] === "-V", + (call) => + String(call[0]) === "tmux" && Array.isArray(call[1]) && (call[1] as string[])[0] === "-V", ); expect(tmuxChecks.length).toBeGreaterThan(0); }); @@ -2331,7 +2351,7 @@ describe("start command — already-running detection", () => { globalConfigPath, yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { "my-app": { name: "My App", @@ -2376,8 +2396,7 @@ describe("start command — already-running detection", () => { ) { return "https://github.com/org/new-repo.git"; } - if (args[0] === "symbolic-ref" && workingDir === repoDir) - return "refs/remotes/origin/main"; + if (args[0] === "symbolic-ref" && workingDir === repoDir) return "refs/remotes/origin/main"; if (args[0] === "rev-parse" && args[1] === "--verify" && workingDir === repoDir) return "abc"; return null; @@ -2480,8 +2499,7 @@ describe("start command — already-running detection", () => { }); mockWaitForExit.mockResolvedValue(true); - - const killSpy = vi.spyOn(process, "kill").mockImplementation(() => true); + mockKillProcessTree.mockResolvedValue(undefined); mockPromptSelect.mockResolvedValue("restart"); @@ -2495,7 +2513,10 @@ describe("start command — already-running detection", () => { // Startup after restart may throw — that's OK for this test } - expect(killSpy).toHaveBeenCalledWith(9999, "SIGTERM"); + // killExistingDaemon delegates to killProcessTree (taskkill /T /F on Windows, + // process group signalling on Unix) instead of raw process.kill, so dead + // grandchildren of the daemon don't leak. + expect(mockKillProcessTree).toHaveBeenCalledWith(9999, "SIGTERM"); expect(mockUnregister).toHaveBeenCalled(); const output = vi @@ -2503,8 +2524,6 @@ describe("start command — already-running detection", () => { .mock.calls.map((c) => c.join(" ")) .join("\n"); expect(output).toContain("Stopped existing instance"); - - killSpy.mockRestore(); }); it("creates new orchestrator entry when human caller selects 'new'", async () => { @@ -2524,7 +2543,7 @@ describe("start command — already-running detection", () => { configPath, yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { "my-app": { name: "My App", @@ -2578,7 +2597,7 @@ describe("start command — already-running detection", () => { const { stringify: yamlStringify } = await import("yaml"); const originalYaml = yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { "my-app": { name: "My App", @@ -2631,7 +2650,7 @@ describe("start command — path-based deduplication in addProjectToConfig", () configPath, yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { "my-app": { name: "My App", @@ -2684,7 +2703,7 @@ describe("start command — path-based deduplication in addProjectToConfig", () configPath, yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { "old-name": { name: "Old Name", @@ -2744,7 +2763,7 @@ describe("start command — global registry mutations", () => { globalConfigPath, yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { current: { projectId: "current", @@ -2845,7 +2864,7 @@ describe("start command — global registry mutations", () => { globalConfigPath, yamlStringify( { - defaults: { runtime: "tmux", agent: "claude-code", workspace: "worktree", notifiers: [] }, + defaults: { runtime: "process", agent: "claude-code", workspace: "worktree", notifiers: [] }, projects: { current: { projectId: "current", diff --git a/packages/cli/__tests__/lib/daemon.test.ts b/packages/cli/__tests__/lib/daemon.test.ts index bcd129208..c50e1edd1 100644 --- a/packages/cli/__tests__/lib/daemon.test.ts +++ b/packages/cli/__tests__/lib/daemon.test.ts @@ -1,9 +1,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type * as AoCore from "@aoagents/ao-core"; -const { mockUnregister, mockWaitForExit, mockProcessKill } = vi.hoisted(() => ({ +const { mockUnregister, mockWaitForExit, mockKillProcessTree } = vi.hoisted(() => ({ mockUnregister: vi.fn(), mockWaitForExit: vi.fn(), - mockProcessKill: vi.fn(), + mockKillProcessTree: vi.fn(), })); vi.mock("../../src/lib/running-state.js", () => ({ @@ -11,6 +12,14 @@ vi.mock("../../src/lib/running-state.js", () => ({ waitForExit: mockWaitForExit, })); +vi.mock("@aoagents/ao-core", async () => { + const actual = await vi.importActual("@aoagents/ao-core"); + return { + ...actual, + killProcessTree: mockKillProcessTree, + }; +}); + import { attachToDaemon, killExistingDaemon } from "../../src/lib/daemon.js"; import type { RunningState } from "../../src/lib/running-state.js"; @@ -26,17 +35,8 @@ beforeEach(() => { mockUnregister.mockReset(); mockUnregister.mockResolvedValue(undefined); mockWaitForExit.mockReset(); - mockProcessKill.mockReset(); - // Spy is installed per-test and restored in afterEach so the mocked - // process.kill cannot leak into sibling test files when Vitest reuses - // worker threads. - vi.spyOn(process, "kill").mockImplementation((( - pid: number, - signal?: string | number, - ) => { - mockProcessKill(pid, signal); - return true; - }) as typeof process.kill); + mockKillProcessTree.mockReset(); + mockKillProcessTree.mockResolvedValue(undefined); }); afterEach(() => { @@ -58,10 +58,9 @@ describe("attachToDaemon", () => { const daemon = attachToDaemon(fakeRunning); const result = await daemon.notifyProjectChange(); expect(result).toEqual({ ok: true }); - expect(fetchSpy).toHaveBeenCalledWith( - "http://localhost:3000/api/projects/reload", - { method: "POST" }, - ); + expect(fetchSpy).toHaveBeenCalledWith("http://localhost:3000/api/projects/reload", { + method: "POST", + }); fetchSpy.mockRestore(); }); @@ -79,9 +78,7 @@ describe("attachToDaemon", () => { }); it("notifyProjectChange returns a reasoned failure when fetch throws", async () => { - const fetchSpy = vi - .spyOn(globalThis, "fetch") - .mockRejectedValue(new Error("ECONNREFUSED")); + const fetchSpy = vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("ECONNREFUSED")); const daemon = attachToDaemon(fakeRunning); const result = await daemon.notifyProjectChange(); expect(result.ok).toBe(false); @@ -93,21 +90,21 @@ describe("attachToDaemon", () => { }); describe("killExistingDaemon", () => { - it("SIGTERMs the daemon, awaits exit, and unregisters on the happy path", async () => { + it("uses killProcessTree(SIGTERM), awaits exit, and unregisters on the happy path", async () => { mockWaitForExit.mockResolvedValueOnce(true); await killExistingDaemon(fakeRunning); - expect(mockProcessKill).toHaveBeenCalledWith(12345, "SIGTERM"); - expect(mockProcessKill).toHaveBeenCalledTimes(1); + expect(mockKillProcessTree).toHaveBeenCalledWith(12345, "SIGTERM"); + expect(mockKillProcessTree).toHaveBeenCalledTimes(1); expect(mockWaitForExit).toHaveBeenCalledWith(12345, 5000); expect(mockUnregister).toHaveBeenCalled(); }); - it("escalates to SIGKILL when SIGTERM does not exit within the timeout", async () => { + it("escalates to SIGKILL via killProcessTree when SIGTERM does not exit", async () => { mockWaitForExit.mockResolvedValueOnce(false); mockWaitForExit.mockResolvedValueOnce(true); await killExistingDaemon(fakeRunning); - expect(mockProcessKill).toHaveBeenNthCalledWith(1, 12345, "SIGTERM"); - expect(mockProcessKill).toHaveBeenNthCalledWith(2, 12345, "SIGKILL"); + expect(mockKillProcessTree).toHaveBeenNthCalledWith(1, 12345, "SIGTERM"); + expect(mockKillProcessTree).toHaveBeenNthCalledWith(2, 12345, "SIGKILL"); expect(mockUnregister).toHaveBeenCalled(); }); @@ -120,12 +117,15 @@ describe("killExistingDaemon", () => { expect(mockUnregister).not.toHaveBeenCalled(); }); - it("treats already-dead processes as success (process.kill throws ESRCH)", async () => { - mockProcessKill.mockImplementation(() => { - throw new Error("ESRCH"); - }); + it("treats killProcessTree errors as best-effort and still unregisters when process is gone", async () => { + // killProcessTree itself swallows errors internally, but defend against + // a future regression by ensuring an unexpected throw does not crash + // unregister() when the process has actually exited. + mockKillProcessTree.mockRejectedValueOnce(new Error("transient")); mockWaitForExit.mockResolvedValueOnce(true); - await expect(killExistingDaemon(fakeRunning)).resolves.toBeUndefined(); - expect(mockUnregister).toHaveBeenCalled(); + await expect(killExistingDaemon(fakeRunning)).rejects.toThrow("transient"); + // unregister should NOT have been called in this rejection path — + // we only want to unregister after a clean exit. + expect(mockUnregister).not.toHaveBeenCalled(); }); }); diff --git a/packages/cli/__tests__/lib/path-equality.test.ts b/packages/cli/__tests__/lib/path-equality.test.ts new file mode 100644 index 000000000..dd3c86bcb --- /dev/null +++ b/packages/cli/__tests__/lib/path-equality.test.ts @@ -0,0 +1,98 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, rmSync, mkdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { pathsEqual, canonicalCompareKey } from "../../src/lib/path-equality.js"; + +let tmpDir: string; +let originalPlatform: PropertyDescriptor | undefined; + +function setPlatform(p: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { value: p, configurable: true }); +} + +beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "ao-pathseq-")); + originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); +}); + +afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, "platform", originalPlatform); + } + rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +describe("pathsEqual", () => { + it("returns true for the same path", () => { + const dir = join(tmpDir, "same"); + mkdirSync(dir); + expect(pathsEqual(dir, dir)).toBe(true); + }); + + it("returns false for clearly different paths", () => { + const a = join(tmpDir, "a"); + const b = join(tmpDir, "b"); + mkdirSync(a); + mkdirSync(b); + expect(pathsEqual(a, b)).toBe(false); + }); + + it.skipIf(process.platform !== "win32")("treats drive-letter case as equal on Windows", () => { + // Real filesystem path so realpathSync resolves; only the input case differs. + const dir = join(tmpDir, "case-test"); + mkdirSync(dir); + const lowerDrive = dir.replace(/^([A-Z]):/, (_, c: string) => `${c.toLowerCase()}:`); + const upperDrive = dir.replace(/^([a-z]):/, (_, c: string) => `${c.toUpperCase()}:`); + expect(pathsEqual(lowerDrive, upperDrive)).toBe(true); + }); + + it.skipIf(process.platform !== "win32")( + "treats arbitrary path-segment case as equal on Windows", + () => { + const dir = join(tmpDir, "MixedCaseSegment"); + mkdirSync(dir); + const lower = dir.toLowerCase(); + // realpathSync should resolve both to the same on-disk canonical form; + // pathsEqual then lowercases for comparison on Windows. + expect(pathsEqual(dir, lower)).toBe(true); + }, + ); + + it.skipIf(process.platform === "win32")("is case-sensitive on POSIX", () => { + // Don't actually mkdir — we just want to verify the comparison logic. + // Use a non-existent path so realpathSync falls back to the literal. + setPlatform("linux"); + const a = "/tmp/Case-Sensitive-Test-NoExist"; + const b = "/tmp/case-sensitive-test-noexist"; + expect(pathsEqual(a, b)).toBe(false); + }); + + it("falls back to literal comparison when realpathSync fails (path doesn't exist)", () => { + const a = join(tmpDir, "nonexistent"); + expect(pathsEqual(a, a)).toBe(true); + }); +}); + +describe("canonicalCompareKey", () => { + it("expands ~ to HOME", () => { + const originalHome = process.env["HOME"]; + process.env["HOME"] = tmpDir; + try { + const key = canonicalCompareKey("~"); + // On Windows the result is lowercased; on POSIX it's case-preserved. + expect(key.toLowerCase()).toBe(tmpDir.toLowerCase()); + } finally { + if (originalHome === undefined) delete process.env["HOME"]; + else process.env["HOME"] = originalHome; + } + }); + + it("returns the same key for equivalent inputs", () => { + const dir = join(tmpDir, "equiv"); + mkdirSync(dir); + expect(canonicalCompareKey(dir)).toBe(canonicalCompareKey(dir)); + }); +}); diff --git a/packages/cli/__tests__/lib/startup-preflight.test.ts b/packages/cli/__tests__/lib/startup-preflight.test.ts new file mode 100644 index 000000000..9f9c63151 --- /dev/null +++ b/packages/cli/__tests__/lib/startup-preflight.test.ts @@ -0,0 +1,145 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, writeFileSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const { mockAskYesNo, mockExecSilent } = vi.hoisted(() => ({ + mockAskYesNo: vi.fn(), + mockExecSilent: vi.fn(), +})); + +vi.mock("../../src/lib/install-helpers.js", () => ({ + askYesNo: mockAskYesNo, + tryInstallWithAttempts: vi.fn(async () => false), +})); + +vi.mock("../../src/lib/shell.js", () => ({ + execSilent: mockExecSilent, +})); + +import { ensureTmux } from "../../src/lib/startup-preflight.js"; + +let tmpDir: string; +let originalPlatform: PropertyDescriptor | undefined; + +function setPlatform(p: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { value: p, configurable: true }); +} + +beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "ao-preflight-test-")); + originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); + mockAskYesNo.mockReset(); + mockExecSilent.mockReset(); +}); + +afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, "platform", originalPlatform); + } + rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +describe("ensureTmux on Windows", () => { + it("rewrites runtime: tmux -> runtime: process when user accepts", async () => { + setPlatform("win32"); + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + const original = [ + "port: 3000", + "defaults:", + " runtime: tmux", + " agent: claude-code", + "projects: {}", + "", + ].join("\n"); + writeFileSync(configPath, original, "utf-8"); + + mockAskYesNo.mockResolvedValueOnce(true); + + const result = await ensureTmux(configPath); + expect(result.switchedToProcess).toBe(true); + + const after = readFileSync(configPath, "utf-8"); + expect(after).toContain("runtime: process"); + expect(after).not.toContain("runtime: tmux"); + // Surrounding lines preserved + expect(after).toContain("agent: claude-code"); + expect(after).toContain("port: 3000"); + }); + + it("preserves quoting when rewriting", async () => { + setPlatform("win32"); + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + writeFileSync(configPath, 'defaults:\n runtime: "tmux"\n agent: claude-code\n', "utf-8"); + + mockAskYesNo.mockResolvedValueOnce(true); + const result = await ensureTmux(configPath); + expect(result.switchedToProcess).toBe(true); + + const after = readFileSync(configPath, "utf-8"); + expect(after).toContain("runtime: process"); + }); + + it("preserves trailing comments when rewriting", async () => { + setPlatform("win32"); + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + writeFileSync(configPath, "defaults:\n runtime: tmux # legacy default\n", "utf-8"); + + mockAskYesNo.mockResolvedValueOnce(true); + const result = await ensureTmux(configPath); + expect(result.switchedToProcess).toBe(true); + + const after = readFileSync(configPath, "utf-8"); + expect(after).toContain("runtime: process # legacy default"); + }); + + it("exits when user declines the rewrite", async () => { + setPlatform("win32"); + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + writeFileSync(configPath, "defaults:\n runtime: tmux\n", "utf-8"); + + mockAskYesNo.mockResolvedValueOnce(false); + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { + throw new Error("__process_exit__"); + }) as never); + + await expect(ensureTmux(configPath)).rejects.toThrow("__process_exit__"); + expect(exitSpy).toHaveBeenCalledWith(1); + + // File untouched + const after = readFileSync(configPath, "utf-8"); + expect(after).toContain("runtime: tmux"); + }); + + it("exits without prompting when configPath is missing", async () => { + setPlatform("win32"); + const exitSpy = vi.spyOn(process, "exit").mockImplementation((() => { + throw new Error("__process_exit__"); + }) as never); + + await expect(ensureTmux()).rejects.toThrow("__process_exit__"); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockAskYesNo).not.toHaveBeenCalled(); + }); + + it("does not invoke tmux -V on Windows", async () => { + setPlatform("win32"); + const configPath = join(tmpDir, "agent-orchestrator.yaml"); + writeFileSync(configPath, "defaults:\n runtime: tmux\n", "utf-8"); + mockAskYesNo.mockResolvedValueOnce(true); + + await ensureTmux(configPath); + expect(mockExecSilent).not.toHaveBeenCalled(); + }); +}); + +describe("ensureTmux on Linux when tmux is present", () => { + it("returns without prompting", async () => { + setPlatform("linux"); + mockExecSilent.mockResolvedValueOnce("tmux 3.3a"); + const result = await ensureTmux(); + expect(result.switchedToProcess).toBe(false); + expect(mockAskYesNo).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 61949f2cc..09aada0d5 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -10,7 +10,7 @@ */ import { spawn, type ChildProcess } from "node:child_process"; -import { existsSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { resolve, basename, dirname } from "node:path"; import { cwd } from "node:process"; import chalk from "chalk"; @@ -98,6 +98,7 @@ import { import { ensureGit, runtimePreflight } from "../lib/startup-preflight.js"; import { installShutdownHandlers } from "../lib/shutdown.js"; import { resolveOrCreateProject } from "../lib/resolve-project.js"; +import { pathsEqual } from "../lib/path-equality.js"; import { DEFAULT_PORT } from "../lib/constants.js"; import { projectSessionUrl } from "../lib/routes.js"; @@ -203,10 +204,7 @@ async function resolveProject( const currentDirResolved = resolve(cwd()); const cwdAlreadyInConfig = projectIds.some((id) => { try { - return ( - resolve(config.projects[id].path.replace(/^~/, process.env["HOME"] || "")) === - currentDirResolved - ); + return pathsEqual(config.projects[id].path, currentDirResolved); } catch { return false; } @@ -620,14 +618,12 @@ async function addProjectToConfig( const resolvedPath = resolve(projectPath.replace(/^~/, process.env["HOME"] || "")); // Check if this path is already registered under any project name. - // Use realpathSync for canonical comparison (resolves symlinks, case variants). + // pathsEqual canonicalizes via realpathSync and lowercases on Windows so + // drive-letter case and 8.3-vs-long-name differences don't cause a miss. // Done before ensureGit so already-registered paths return early without requiring git. - const canonicalPath = realpathSync(resolvedPath); const existingByPath = Object.entries(config.projects).find(([, p]) => { try { - return ( - realpathSync(resolve(p.path.replace(/^~/, process.env["HOME"] || ""))) === canonicalPath - ); + return pathsEqual(p.path, resolvedPath); } catch { return false; } @@ -965,7 +961,9 @@ async function runStartup( const currentProjectSessions = lastStop.projectId === projectId ? lastStop.sessionIds : []; if (currentProjectSessions.length > 0) { console.log( - chalk.yellow(`\n ${currentProjectSessions.length} session(s) were active before last ao stop (${stoppedAgo}):`), + chalk.yellow( + `\n ${currentProjectSessions.length} session(s) were active before last ao stop (${stoppedAgo}):`, + ), ); console.log(chalk.dim(` ${currentProjectSessions.join(", ")}\n`)); } @@ -1013,9 +1011,13 @@ async function runStartup( } } if (restoredCount === allRestoreSessions.length) { - restoreSpinner.succeed(`Restored ${restoredCount}/${allRestoreSessions.length} session(s)`); + restoreSpinner.succeed( + `Restored ${restoredCount}/${allRestoreSessions.length} session(s)`, + ); } else { - restoreSpinner.warn(`Restored ${restoredCount}/${allRestoreSessions.length} session(s)`); + restoreSpinner.warn( + `Restored ${restoredCount}/${allRestoreSessions.length} session(s)`, + ); } for (const w of warnings) { console.log(chalk.yellow(w)); @@ -1027,9 +1029,7 @@ async function runStartup( // and the remaining sessions would never be retryable. When // every session restored (or was skipped), clear the file. if (failedSessionIds.size > 0) { - const remainingTarget = lastStop.sessionIds.filter((id) => - failedSessionIds.has(id), - ); + const remainingTarget = lastStop.sessionIds.filter((id) => failedSessionIds.has(id)); const remainingOther = otherProjects .map((p) => ({ projectId: p.projectId, @@ -1258,9 +1258,7 @@ async function attachAndSpawnOrchestrator(opts: { console.log(chalk.dim(` Dashboard config reloaded.`)); } else { console.log( - chalk.yellow( - ` ⚠ ${notifyResult.reason}. Refresh the page if the project doesn't show up.`, - ), + chalk.yellow(` ⚠ ${notifyResult.reason}. Refresh the page if the project doesn't show up.`), ); } @@ -1324,8 +1322,7 @@ export function registerStart(program: Command): void { // ── Already-running detection (before any config mutation) ── let running = await isAlreadyRunning(); let startNewOrchestrator = false; - const isProjectId = - projectArg && !isRepoUrl(projectArg) && !isLocalPath(projectArg); + const isProjectId = projectArg && !isRepoUrl(projectArg) && !isLocalPath(projectArg); const projectArgIsUrlOrPath = !!projectArg && (isRepoUrl(projectArg) || isLocalPath(projectArg)); @@ -1361,10 +1358,7 @@ export function registerStart(program: Command): void { try { const loadedCfg = loadConfig(); const proj = loadedCfg.projects[p]; - return ( - proj && - resolve(proj.path.replace(/^~/, process.env["HOME"] || "")) === cwdResolved - ); + return proj !== undefined && pathsEqual(proj.path, cwdResolved); } catch { return false; } @@ -1748,7 +1742,9 @@ export function registerStop(program: Command): void { if (killedSessionIds.length === 0) { spinner.fail("Failed to stop any sessions"); } else if (killedSessionIds.length < activeSessions.length) { - spinner.warn(`Stopped ${killedSessionIds.length}/${activeSessions.length} session(s)`); + spinner.warn( + `Stopped ${killedSessionIds.length}/${activeSessions.length} session(s)`, + ); } else { spinner.succeed(`Stopped ${killedSessionIds.length} session(s)`); } @@ -1785,9 +1781,7 @@ export function registerStop(program: Command): void { await writeLastStop({ stoppedAt: new Date().toISOString(), projectId: _projectId, - sessionIds: killedSessionIds.filter((id) => - targetActive.some((s) => s.id === id), - ), + sessionIds: killedSessionIds.filter((id) => targetActive.some((s) => s.id === id)), otherProjects: otherProjects.length > 0 ? otherProjects : undefined, }); } diff --git a/packages/cli/src/lib/daemon.ts b/packages/cli/src/lib/daemon.ts index 433058be9..9a5c60e6a 100644 --- a/packages/cli/src/lib/daemon.ts +++ b/packages/cli/src/lib/daemon.ts @@ -20,6 +20,7 @@ */ import chalk from "chalk"; +import { killProcessTree } from "@aoagents/ao-core"; import { unregister, waitForExit, type RunningState } from "./running-state.js"; /** @@ -81,20 +82,18 @@ export function attachToDaemon(running: RunningState): AttachedDaemon { * still alive, wait another 3s. Throws if the process refuses to die. * Always unregisters `running.json` on success so the next `ao start` can * spawn a fresh daemon without hitting the "already running" gate. + * + * Uses {@link killProcessTree} (not raw `process.kill`) so Windows actually + * terminates the daemon and its detached grandchildren (pty-host, dashboard + * subprocess) via `taskkill /T /F`. On POSIX this is process-group aware + * with a fallback to direct kill. Both paths swallow "already dead" errors + * internally. */ export async function killExistingDaemon(running: RunningState): Promise { - try { - process.kill(running.pid, "SIGTERM"); - } catch { - // already dead — fall through to wait/unregister - } + await killProcessTree(running.pid, "SIGTERM"); if (!(await waitForExit(running.pid, 5000))) { console.log(chalk.yellow(" Process didn't exit cleanly, sending SIGKILL...")); - try { - process.kill(running.pid, "SIGKILL"); - } catch { - // already dead - } + await killProcessTree(running.pid, "SIGKILL"); if (!(await waitForExit(running.pid, 3000))) { throw new Error( `Failed to stop AO process (PID ${running.pid}). Check permissions or stop it manually.`, diff --git a/packages/cli/src/lib/path-equality.ts b/packages/cli/src/lib/path-equality.ts new file mode 100644 index 000000000..6c4e6e6b5 --- /dev/null +++ b/packages/cli/src/lib/path-equality.ts @@ -0,0 +1,54 @@ +/** + * Canonical path-equality helpers. + * + * On Windows, `realpathSync` can return canonically resolved paths whose + * drive-letter case and 8.3-vs-long-name expansion differ from the input + * even when both inputs point to the same filesystem entry (e.g. one input + * came from a config file and another from `process.cwd()` after a chdir). + * A naive `===` comparison misses these as "different paths" and the calling + * code falls into "treat as new" branches — for project resolution this + * presents to the user as phantom "register this project?" prompts even + * when the project is already registered. + * + * This module centralises the comparison so every site that asks "are + * these two paths the same on disk?" gets the same answer regardless of + * platform. POSIX behaviour is unchanged (case-sensitive `===`). + */ + +import { realpathSync } from "node:fs"; +import { resolve } from "node:path"; +import { isWindows } from "@aoagents/ao-core"; + +/** + * Resolve symlinks. Falls back to the input on any filesystem error so + * callers can still compare unreadable paths literally rather than crash. + * Mirrors the canonicalize() helper that previously lived in + * resolve-project.ts. + */ +function canonicalize(p: string): string { + try { + return realpathSync(p); + } catch { + return p; + } +} + +/** + * Build the comparison key for a path: resolve to absolute, expand `~`, + * canonicalize symlinks, and normalize case on Windows. Useful when the + * caller needs a stable key for `Map`/`Set` lookups across many paths. + */ +export function canonicalCompareKey(input: string): string { + const expanded = input.replace(/^~/, process.env["HOME"] ?? ""); + const canonical = canonicalize(resolve(expanded)); + return isWindows() ? canonical.toLowerCase() : canonical; +} + +/** + * Compare two paths for "same filesystem entry" semantics. Equivalent to + * `canonicalCompareKey(a) === canonicalCompareKey(b)` but kept as a named + * helper for readability at call sites. + */ +export function pathsEqual(a: string, b: string): boolean { + return canonicalCompareKey(a) === canonicalCompareKey(b); +} diff --git a/packages/cli/src/lib/resolve-project.ts b/packages/cli/src/lib/resolve-project.ts index 5028fec17..b6bb2bc69 100644 --- a/packages/cli/src/lib/resolve-project.ts +++ b/packages/cli/src/lib/resolve-project.ts @@ -14,8 +14,9 @@ * would generate. */ -import { existsSync, realpathSync, writeFileSync } from "node:fs"; +import { existsSync, writeFileSync } from "node:fs"; import { resolve } from "node:path"; +import { pathsEqual } from "./path-equality.js"; import { cwd } from "node:process"; import { ConfigNotFoundError, @@ -124,24 +125,16 @@ export interface ResolveOptions { /** * Decide whether `arg` looks like a path (rather than a project id). - * Matches start.ts's `isLocalPath`. + * Matches start.ts's `isLocalPath` — including Windows drive-letter and + * UNC patterns so e.g. `ao start C:\path\to\repo` is correctly classified. */ function isLocalPath(arg: string): boolean { - return arg.startsWith("/") || arg.startsWith("~") || arg.startsWith("./") || arg.startsWith(".."); -} - -/** - * Resolve symlink chains for canonical path comparison. Falls back to the - * input on any filesystem error so the caller can compare unreadable paths - * literally rather than crash. Mirrors the canonical-compare pattern used - * elsewhere in the CLI for global-registry dedup. - */ -function canonicalize(p: string): string { - try { - return realpathSync(p); - } catch { - return p; + if (arg.startsWith("/") || arg.startsWith("~") || arg.startsWith("./") || arg.startsWith("..")) { + return true; } + if (/^[A-Za-z]:[\\/]/.test(arg)) return true; + if (arg.startsWith("\\\\") || arg.startsWith(".\\") || arg.startsWith("..\\")) return true; + return false; } /** @@ -253,9 +246,7 @@ async function fromUrlIntoGlobal(arg: string, deps: ResolveDeps): Promise /private/tmp) match an entry stored under the resolved - // target. Without this, `ao start /tmp/foo` against a daemon whose - // global config has /private/tmp/foo would fail to dedupe and - // double-register the project. - const canonicalTarget = canonicalize(resolvedPath); - const existingEntry = Object.entries(globalConfig.projects).find(([, p]) => { - const expanded = resolve(p.path.replace(/^~/, process.env["HOME"] || "")); - return canonicalize(expanded) === canonicalTarget; - }); + // pathsEqual canonicalizes via realpathSync so symlinked paths (e.g. + // macOS /tmp -> /private/tmp) match an entry stored under the + // resolved target, and lowercases on Windows so drive-letter / 8.3 + // case mismatches don't slip through. Without this, `ao start + // /tmp/foo` against a daemon whose global config has /private/tmp/foo + // would fail to dedupe and double-register the project. + const existingEntry = Object.entries(globalConfig.projects).find(([, p]) => + pathsEqual(p.path, resolvedPath), + ); if (existingEntry) { return { config: globalConfig, @@ -375,9 +365,7 @@ async function fromPath(arg: string, deps: ResolveDeps, opts: ResolveOptions): P const reloaded = loadConfig(globalConfig.configPath); const project = reloaded.projects[addedId]; if (!project) { - throw new Error( - `Failed to register "${addedId}" in the global config — aborting.`, - ); + throw new Error(`Failed to register "${addedId}" in the global config — aborting.`); } return { config: reloaded, @@ -411,8 +399,8 @@ async function fromPath(arg: string, deps: ResolveDeps, opts: ResolveOptions): P // Config exists — check if the path is already registered. const config = loadConfig(configPath); - const existingEntry = Object.entries(config.projects).find( - ([, p]) => resolve(p.path.replace(/^~/, process.env["HOME"] || "")) === resolvedPath, + const existingEntry = Object.entries(config.projects).find(([, p]) => + pathsEqual(p.path, resolvedPath), ); if (existingEntry) { diff --git a/packages/cli/src/lib/startup-preflight.ts b/packages/cli/src/lib/startup-preflight.ts index ca2cfd77d..ce0cfb2e5 100644 --- a/packages/cli/src/lib/startup-preflight.ts +++ b/packages/cli/src/lib/startup-preflight.ts @@ -12,23 +12,21 @@ * `runStartup` runs once at process start. */ +import { readFileSync, writeFileSync } from "node:fs"; import chalk from "chalk"; import { getAoBaseDir, getDefaultRuntime, getGlobalConfigPath, inventoryHashDirs, + isWindows, type OrchestratorConfig, } from "@aoagents/ao-core"; import { execSilent } from "./shell.js"; import { detectOpenClawInstallation } from "./openclaw-probe.js"; import { applyOpenClawCredentials } from "./credential-resolver.js"; import { preventIdleSleep } from "./prevent-sleep.js"; -import { - askYesNo, - tryInstallWithAttempts, - type InstallAttempt, -} from "./install-helpers.js"; +import { askYesNo, tryInstallWithAttempts, type InstallAttempt } from "./install-helpers.js"; function gitInstallAttempts(): InstallAttempt[] { if (process.platform === "darwin") { @@ -112,13 +110,87 @@ export async function ensureGit(context: string): Promise { process.exit(1); } +/** + * On Windows, attempt to rewrite `runtime: tmux` -> `runtime: process` in the + * project's agent-orchestrator.yaml after asking the user. Uses a targeted + * line replace (not a yaml round-trip) so comments and quoting are preserved. + * + * Returns `true` on a successful rewrite. The caller is responsible for + * mutating the in-memory config (or reloading) so the rest of preflight + * sees the new runtime. + */ +async function offerWindowsRuntimeSwitch(configPath: string): Promise { + console.log(chalk.yellow("\n⚠ tmux runtime is not supported on Windows.")); + console.log(chalk.dim(` Config: ${configPath}`)); + console.log( + chalk.dim( + " AO can rewrite `runtime: tmux` -> `runtime: process` in this file.\n" + + " If the file is git-tracked, you'll see this as a local change.", + ), + ); + + const accept = await askYesNo("Switch this project to runtime: process?", true, false); + if (!accept) return false; + + let original: string; + try { + original = readFileSync(configPath, "utf-8"); + } catch (err) { + console.error( + chalk.red(` ✗ Could not read config: ${err instanceof Error ? err.message : String(err)}`), + ); + return false; + } + + // Match the runtime line whether quoted or unquoted, and preserve any + // trailing comment. Anchored multiline so we don't accidentally rewrite + // e.g. a string value on another line. + const runtimeLineRe = /^([ \t]*runtime:[ \t]*)(?:'tmux'|"tmux"|tmux)([ \t]*(?:#.*)?)$/m; + if (!runtimeLineRe.test(original)) { + console.error( + chalk.red(" ✗ Could not locate `runtime: tmux` line in config; aborting rewrite."), + ); + return false; + } + const rewritten = original.replace(runtimeLineRe, "$1process$2"); + + try { + writeFileSync(configPath, rewritten, "utf-8"); + } catch (err) { + console.error( + chalk.red(` ✗ Failed to write config: ${err instanceof Error ? err.message : String(err)}`), + ); + return false; + } + console.log(chalk.green(" ✓ Updated runtime to process")); + return true; +} + /** * Ensure tmux is available — interactive install with user consent if missing. * Called from runtimePreflight() so all `ao start` paths are covered. + * + * On Windows, tmux cannot run; instead, offer to rewrite the project config + * to `runtime: process`. Returns `{ switchedToProcess: true }` if the rewrite + * succeeded so the caller can update the in-memory config. */ -export async function ensureTmux(): Promise { +export async function ensureTmux(configPath?: string): Promise<{ switchedToProcess: boolean }> { + if (isWindows()) { + if (configPath) { + const switched = await offerWindowsRuntimeSwitch(configPath); + if (switched) return { switchedToProcess: true }; + } + console.error(chalk.red("\n✗ tmux runtime is not supported on Windows.\n")); + console.log( + chalk.bold(" Set ") + + chalk.cyan("runtime: process") + + chalk.bold(" in agent-orchestrator.yaml, then re-run ao start.\n"), + ); + process.exit(1); + } + const hasTmux = (await execSilent("tmux", ["-V"])) !== null; - if (hasTmux) return; + if (hasTmux) return { switchedToProcess: false }; console.log(chalk.yellow('⚠ tmux is required for runtime "tmux".')); const shouldInstall = await askYesNo("Install tmux now?", true, false); @@ -129,7 +201,7 @@ export async function ensureTmux(): Promise { ); if (installed) { console.log(chalk.green(" ✓ tmux installed successfully")); - return; + return { switchedToProcess: false }; } } @@ -212,7 +284,15 @@ export async function warnAboutOpenClawStatus(config: OrchestratorConfig): Promi export async function runtimePreflight(config: OrchestratorConfig): Promise { const runtime = config.defaults?.runtime ?? getDefaultRuntime(); if (runtime === "tmux") { - await ensureTmux(); + const result = await ensureTmux(config.configPath); + if (result.switchedToProcess) { + // Mutate in-memory config so the rest of startup uses the new runtime. + // Disk has already been updated; subsequent loadConfig() calls will + // see the same value. + const defaults = config.defaults ?? {}; + defaults.runtime = "process"; + config.defaults = defaults; + } } warnAboutLegacyStorage(); await warnAboutOpenClawStatus(config);