diff --git a/CLAUDE.md b/CLAUDE.md index 731ee041f..69dc6b580 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -210,6 +210,7 @@ Strong success criteria let you loop independently. Weak criteria ("make it work ### ao start - Registers in `running.json` (PID, port, projects) - Offers to restore sessions from `last-stop.json` — includes cross-project sessions via `otherProjects` field +- `ao start --restore` restores `last-stop.json` without prompting; `ao start --no-restore` skips restore - **Ctrl+C performs full graceful shutdown** (same as ao stop): kills all sessions, writes last-stop state, unregisters from running.json. 10s hard timeout guarantees exit. ### ao stop @@ -218,6 +219,10 @@ Strong success criteria let you loop independently. Weak criteria ("make it work - Always loads global config (`~/.agent-orchestrator/config.yaml`) to see all projects — local config only has the cwd project - Records `LastStopState` with `otherProjects` field for cross-project session restore +### ao update +- For package-manager installs, `ao update` pauses a running AO via `ao stop --yes`, runs the global package update, verifies `ao --version`, then restarts with `ao start --restore` (or `--no-restore` if requested) +- Failed package-manager updates must report that AO was not updated, include actionable remediation, and restart the previous installation if AO was paused + ### Dashboard sidebar - Sidebar always shows sessions from ALL projects regardless of which project page is active - `useSessionEvents` in Dashboard.tsx is called without project filter — sidebar gets unscoped sessions diff --git a/packages/cli/__tests__/commands/start.test.ts b/packages/cli/__tests__/commands/start.test.ts index b821d3637..964f0a674 100644 --- a/packages/cli/__tests__/commands/start.test.ts +++ b/packages/cli/__tests__/commands/start.test.ts @@ -1810,6 +1810,46 @@ describe("stop command", () => { expect(output).toContain("app-orchestrator-3"); }); + it("does not show the project picker for no-args ao stop across multiple projects", async () => { + mockConfigRef.current = makeConfig({ + "project-1": makeProject({ name: "Project 1", sessionPrefix: "p1" }), + "project-2": makeProject({ name: "Project 2", sessionPrefix: "p2" }), + }); + mockSessionManager.list.mockResolvedValue([ + { + id: "p1-1", + projectId: "project-1", + status: "working", + activity: "active", + metadata: {}, + lastActivityAt: new Date(), + runtimeHandle: { id: "tmux-1" }, + }, + { + id: "p2-1", + projectId: "project-2", + status: "working", + activity: "active", + metadata: {}, + lastActivityAt: new Date(), + runtimeHandle: { id: "tmux-2" }, + }, + ]); + mockSessionManager.kill.mockResolvedValue({ cleaned: true, alreadyTerminated: false }); + mockPromptConfirm.mockResolvedValue(true); + + await program.parseAsync(["node", "test", "stop"]); + + expect(mockPromptSelect).not.toHaveBeenCalledWith( + expect.stringContaining("Choose project to stop"), + expect.anything(), + expect.anything(), + ); + expect(mockPromptConfirm).toHaveBeenCalledWith("Stop AO and 2 active session(s)?", false); + expect(mockSessionManager.kill).toHaveBeenCalledWith("p1-1", { purgeOpenCode: false }); + expect(mockSessionManager.kill).toHaveBeenCalledWith("p2-1", { purgeOpenCode: false }); + }); + it("kills the most-recently-active orchestrator when multiple exist", async () => { mockConfigRef.current = makeConfig({ "my-app": makeProject() }); const now = Date.now(); diff --git a/packages/cli/__tests__/commands/update.test.ts b/packages/cli/__tests__/commands/update.test.ts index 72cb31047..47e408ad8 100644 --- a/packages/cli/__tests__/commands/update.test.ts +++ b/packages/cli/__tests__/commands/update.test.ts @@ -5,9 +5,7 @@ import { Command } from "commander"; // Mocks // --------------------------------------------------------------------------- -const { - mockRunRepoScript, -} = vi.hoisted(() => ({ +const { mockRunRepoScript } = vi.hoisted(() => ({ mockRunRepoScript: vi.fn(), })); @@ -55,8 +53,8 @@ vi.mock("../../src/lib/update-check.js", () => ({ isManualOnlyInstall: (m: string) => m === "homebrew", })); -// Stub the active-session guard's dependencies so handlers don't try to load -// real config / spawn plugins. Default: no sessions, so the guard passes. +// Stub the update lifecycle planner's dependencies so handlers don't try to +// load real config / spawn plugins. Default: no sessions, so no stop is needed. const { mockSessions } = vi.hoisted(() => ({ mockSessions: { value: [] as Array<{ id: string; status: string }> }, })); @@ -84,8 +82,7 @@ vi.mock("@aoagents/ao-core", async () => { loadConfig: (...args: unknown[]) => mockLoadConfig(...args), loadGlobalConfig: (...args: unknown[]) => mockLoadGlobalConfig(...args), getGlobalConfigPath: () => "/tmp/test-global-config.yaml", - isCanonicalGlobalConfigPath: (p: string | undefined) => - p === "/tmp/test-global-config.yaml", + isCanonicalGlobalConfigPath: (p: string | undefined) => p === "/tmp/test-global-config.yaml", isWindows: () => mockIsWindows(), }; }); @@ -98,10 +95,8 @@ vi.mock("node:fs", async () => { }; }); -// running.json is the live signal: ensureNoActiveSessions now consults -// `getRunning()` before falling back to the global registry. Default to -// "no daemon running" so the existing global-config-driven tests keep -// exercising the fallback path. Per-test overrides simulate a live daemon. +// running.json is the live signal for update stop/start orchestration. Default +// to "no daemon running"; per-test overrides simulate a live daemon. const { mockGetRunning } = vi.hoisted(() => ({ mockGetRunning: vi.fn<() => Promise>(async () => null), })); @@ -147,9 +142,28 @@ function makeNpmUpdateInfo(overrides = {}) { }; } -function createMockChild(exitCode: number | null, signal?: NodeJS.Signals) { +function createMockChild( + exitCode: number | null, + signal?: NodeJS.Signals, + output: { stdout?: string; stderr?: string } = { stdout: "0.3.0\n" }, +) { const child = new EventEmitter(); - setTimeout(() => child.emit("exit", exitCode, signal ?? null), 0); + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + Object.assign(child, { stdout, stderr }); + const emitResult = () => { + setTimeout(() => { + if (output.stdout) stdout.emit("data", Buffer.from(output.stdout)); + if (output.stderr) stderr.emit("data", Buffer.from(output.stderr)); + child.emit("exit", exitCode, signal ?? null); + }, 0); + }; + const originalOn = child.on.bind(child); + child.on = ((event: string | symbol, listener: (...args: unknown[]) => void) => { + const result = originalOn(event, listener); + if (event === "exit") emitResult(); + return result; + }) as EventEmitter["on"]; return child; } @@ -166,7 +180,9 @@ describe("update command", () => { mockRunRepoScript.mockResolvedValue(0); mockDetectInstallMethod.mockReturnValue("git"); mockCheckForUpdate.mockReset(); - mockCheckForUpdate.mockResolvedValue(makeNpmUpdateInfo({ installMethod: "git", recommendedCommand: "ao update" })); + mockCheckForUpdate.mockResolvedValue( + makeNpmUpdateInfo({ installMethod: "git", recommendedCommand: "ao update" }), + ); mockInvalidateCache.mockReset(); mockPromptConfirm.mockReset(); mockPromptConfirm.mockResolvedValue(false); @@ -234,9 +250,9 @@ describe("update command", () => { it("rejects --smoke-only on npm installs with an actionable message", async () => { mockDetectInstallMethod.mockReturnValue("npm-global"); const errSpy = vi.mocked(console.error); - await expect( - program.parseAsync(["node", "test", "update", "--smoke-only"]), - ).rejects.toThrow("process.exit(1)"); + await expect(program.parseAsync(["node", "test", "update", "--smoke-only"])).rejects.toThrow( + "process.exit(1)", + ); const messages = errSpy.mock.calls.map((c) => String(c[0])).join("\n"); expect(messages).toMatch(/--smoke-only only applies to git installs/); }); @@ -306,9 +322,9 @@ describe("update command", () => { new Error("Script not found: ao-update.sh. Expected at: /tmp/ao-update.sh"), ); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); expect(mockSpawn).not.toHaveBeenCalled(); expect(mockCheckForUpdate).not.toHaveBeenCalled(); @@ -353,7 +369,9 @@ describe("update command", () => { }); it("prints already up to date when not outdated", async () => { - mockCheckForUpdate.mockResolvedValue(makeNpmUpdateInfo({ isOutdated: false, latestVersion: "0.2.2", currentVersion: "0.2.2" })); + mockCheckForUpdate.mockResolvedValue( + makeNpmUpdateInfo({ isOutdated: false, latestVersion: "0.2.2", currentVersion: "0.2.2" }), + ); const logSpy = vi.mocked(console.log); await program.parseAsync(["node", "test", "update"]); @@ -365,9 +383,9 @@ describe("update command", () => { makeNpmUpdateInfo({ latestVersion: null, isOutdated: false }), ); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); expect(vi.mocked(console.error)).toHaveBeenCalledWith( expect.stringContaining("Could not reach npm registry"), ); @@ -375,9 +393,7 @@ describe("update command", () => { it("forces a fresh registry fetch", async () => { await program.parseAsync(["node", "test", "update"]); - expect(mockCheckForUpdate).toHaveBeenCalledWith( - expect.objectContaining({ force: true }), - ); + expect(mockCheckForUpdate).toHaveBeenCalledWith(expect.objectContaining({ force: true })); }); it("prints command and exits cleanly in non-TTY mode without prompting", async () => { @@ -400,7 +416,11 @@ describe("update command", () => { await program.parseAsync(["node", "test", "update"]); - expect(mockSpawn).toHaveBeenCalledWith("npm", expect.arrayContaining(["install"]), expect.anything()); + expect(mockSpawn).toHaveBeenCalledWith( + "npm", + expect.arrayContaining(["install"]), + expect.anything(), + ); expect(mockInvalidateCache).toHaveBeenCalled(); }); @@ -410,9 +430,9 @@ describe("update command", () => { mockPromptConfirm.mockResolvedValue(true); mockSpawn.mockReturnValue(createMockChild(1)); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); expect(mockInvalidateCache).not.toHaveBeenCalled(); }); @@ -438,9 +458,9 @@ describe("update command", () => { mockPromptConfirm.mockResolvedValue(true); mockSpawn.mockReturnValue(createMockChild(null, "SIGTERM")); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); expect(vi.mocked(console.error)).not.toHaveBeenCalledWith( expect.stringContaining("exited with code null"), @@ -448,18 +468,26 @@ describe("update command", () => { expect(mockInvalidateCache).not.toHaveBeenCalled(); }); - it("handles spawn error (e.g. npm not found)", async () => { + it("handles spawn error (e.g. npm not found) with a friendly update failure", async () => { Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); mockPromptConfirm.mockResolvedValue(true); const child = new EventEmitter(); + Object.assign(child, { stdout: new EventEmitter(), stderr: new EventEmitter() }); mockSpawn.mockReturnValue(child); setTimeout(() => child.emit("error", new Error("ENOENT: npm not found")), 0); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("ENOENT"); + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); + + const stderr = vi + .mocked(console.error) + .mock.calls.map((c) => String(c[0])) + .join("\n"); + expect(stderr).toContain("AO was not updated. You are still on version 0.2.2."); + expect(stderr).toContain("npm not found"); }); it("does nothing when user declines prompt", async () => { @@ -489,7 +517,9 @@ describe("update command", () => { await program.parseAsync(["node", "test", "update"]); - expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Could not detect install method")); + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining("Could not detect install method"), + ); expect(mockRunRepoScript).not.toHaveBeenCalled(); }); @@ -521,269 +551,143 @@ describe("update command", () => { }); // ----------------------------------------------------------------------- - // Active-session guard (Section C) + // Update lifecycle orchestration (#1972) // ----------------------------------------------------------------------- - describe("active-session guard", () => { + describe("update lifecycle orchestration", () => { beforeEach(() => { - mockDetectInstallMethod.mockReturnValue("npm-global"); - mockCheckForUpdate.mockResolvedValue(makeNpmUpdateInfo({ installMethod: "npm-global" })); + mockDetectInstallMethod.mockReturnValue("pnpm-global"); + mockResolveUpdateChannel.mockReturnValue("stable"); + mockGetUpdateCommand.mockImplementation((method: string) => { + if (method === "pnpm-global") return "pnpm add -g @aoagents/ao@latest"; + return "npm install -g @aoagents/ao@latest"; + }); + mockCheckForUpdate.mockResolvedValue( + makeNpmUpdateInfo({ + installMethod: "pnpm-global", + recommendedCommand: "pnpm add -g @aoagents/ao@latest", + }), + ); Object.defineProperty(process.stdin, "isTTY", { value: true, configurable: true }); Object.defineProperty(process.stdout, "isTTY", { value: true, configurable: true }); - // The guard now ALWAYS loads from global config. Stage a registered - // project so the early-return ("no registry → allow") doesn't fire. + mockGetRunning + .mockResolvedValueOnce({ + pid: 12345, + configPath: "/tmp/test-global-config.yaml", + port: 3000, + startedAt: new Date().toISOString(), + projects: ["my-app"], + }) + .mockResolvedValue(null); + mockLoadConfig.mockImplementation((path?: string) => ({ + projects: { "my-app": { path: "/tmp/foo" } }, + configPath: path ?? "/cwd/agent-orchestrator.yaml", + })); + mockSessions.value = [{ id: "feat-1", status: "working", projectId: "my-app" }]; + }); + + it("runs stop → package install → version verification → start/restore when sessions are active", async () => { + mockSpawn.mockImplementation((cmd: string, args: string[]) => { + if (cmd === "ao" && args[0] === "--version") { + return createMockChild(0, undefined, { stdout: "0.3.0\n" }); + } + return createMockChild(0, undefined, { stdout: "" }); + }); + + await program.parseAsync(["node", "test", "update"]); + + expect(mockSpawn).toHaveBeenCalledTimes(4); + expect(mockSpawn.mock.calls[0]).toEqual([ + "ao", + ["stop", "--yes"], + expect.objectContaining({ stdio: "inherit" }), + ]); + expect(mockSpawn.mock.calls[1][0]).toBe("pnpm"); + expect(mockSpawn.mock.calls[1][1]).toEqual(["add", "-g", "@aoagents/ao@latest"]); + expect(mockSpawn.mock.calls[2][0]).toBe("ao"); + expect(mockSpawn.mock.calls[2][1]).toEqual(["--version"]); + expect(mockSpawn.mock.calls[3]).toEqual([ + "ao", + ["start", "my-app", "--restore"], + expect.objectContaining({ stdio: "inherit" }), + ]); + + const stopOrder = mockSpawn.mock.invocationCallOrder[0]; + const installOrder = mockSpawn.mock.invocationCallOrder[1]; + const startOrder = mockSpawn.mock.invocationCallOrder[3]; + expect(stopOrder).toBeLessThan(installOrder); + expect(installOrder).toBeLessThan(startOrder); + }); + + it("does not stop/start when no daemon or active sessions exist", async () => { + mockGetRunning.mockReset(); + mockGetRunning.mockResolvedValue(null); + mockExistsSync.mockReturnValue(false); + mockSessions.value = []; + mockSpawn.mockImplementation((cmd: string, args: string[]) => { + if (cmd === "ao" && args[0] === "--version") { + return createMockChild(0, undefined, { stdout: "0.3.0\n" }); + } + return createMockChild(0, undefined, { stdout: "" }); + }); + + await program.parseAsync(["node", "test", "update"]); + + expect(mockSpawn).toHaveBeenCalledTimes(2); + expect(mockSpawn.mock.calls[0][0]).toBe("pnpm"); + expect(mockSpawn.mock.calls[1][0]).toBe("ao"); + expect(mockSpawn.mock.calls[1][1]).toEqual(["--version"]); + }); + + it("cleans up orphaned active sessions without starting a daemon that was not running", async () => { + mockGetRunning.mockReset(); + mockGetRunning.mockResolvedValue(null); mockExistsSync.mockReturnValue(true); mockLoadGlobalConfig.mockReturnValue({ projects: { "my-app": { path: "/tmp/foo" } }, }); - mockLoadConfig.mockImplementation((path?: string) => - path - ? { projects: { "my-app": { path: "/tmp/foo" } }, configPath: path } - : { projects: { "my-app": { path: "/tmp/foo" } }, configPath: "/cwd/agent-orchestrator.yaml" }, + mockSessions.value = [{ id: "orphan-1", status: "working", projectId: "my-app" }]; + mockSpawn.mockImplementation((cmd: string, args: string[]) => { + if (cmd === "ao" && args[0] === "stop") { + mockSessions.value = []; + return createMockChild(0, undefined, { stdout: "" }); + } + if (cmd === "ao" && args[0] === "--version") { + return createMockChild(0, undefined, { stdout: "0.3.0\n" }); + } + return createMockChild(0, undefined, { stdout: "" }); + }); + + await program.parseAsync(["node", "test", "update"]); + + expect(mockSpawn.mock.calls[0][0]).toBe("ao"); + expect(mockSpawn.mock.calls[0][1]).toEqual(["stop", "--yes"]); + expect(mockSpawn.mock.calls[1][0]).toBe("pnpm"); + expect(mockSpawn.mock.calls[2][0]).toBe("ao"); + expect(mockSpawn.mock.calls[2][1]).toEqual(["--version"]); + expect(mockSpawn.mock.calls.some((call) => call[0] === "ao" && call[1][0] === "start")).toBe( + false, ); }); - it("refuses to install when a session is in 'working'", async () => { - mockSessions.value = [{ id: "feat-1", status: "working" }]; - const errSpy = vi.mocked(console.error); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); - const messages = errSpy.mock.calls.map((c) => String(c[0])).join("\n"); - expect(messages).toMatch(/1 session active/); - expect(messages).toMatch(/ao stop/); - expect(mockSpawn).not.toHaveBeenCalled(); - }); - - it.each(["working", "idle", "needs_input", "stuck"])( - "refuses for status %s", - async (status) => { - mockSessions.value = [{ id: "feat-1", status }]; - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); - }, - ); - - it("does NOT refuse for terminal statuses (done, terminated, killed)", async () => { - mockSessions.value = [ - { id: "old-1", status: "done" }, - { id: "old-2", status: "terminated" }, - ]; - mockPromptConfirm.mockResolvedValue(false); // decline, no install - await program.parseAsync(["node", "test", "update"]); - // Reaches the prompt step since the guard passed. - expect(mockPromptConfirm).toHaveBeenCalled(); - }); - - // --------------------------------------------------------------------- - // Global-config layout (review #3 / scope-gap follow-up) - // --------------------------------------------------------------------- - - it("the refusal message lists active sessions from EVERY registered project, not just one (Dhruv proof)", async () => { - // Reviewer challenge: prove loadConfig(globalPath) actually enumerates - // sessions across all registered projects, not just the cwd's project. - // We register proj-a + proj-b in the global config, seed one active - // session in each, and assert BOTH ids appear in the stderr output. - mockLoadConfig.mockImplementation((path?: string) => { - // Mimic buildEffectiveConfigFromGlobalConfigPath: the global path - // returns BOTH projects; project-local would only return one. - if (!path) { - return { projects: { "proj-a": {} }, configPath: "/cwd/agent-orchestrator.yaml" }; + it("honors --no-restore when restarting after update", async () => { + mockSpawn.mockImplementation((cmd: string, args: string[]) => { + if (cmd === "ao" && args[0] === "--version") { + return createMockChild(0, undefined, { stdout: "0.3.0\n" }); } - return { - projects: { - "proj-a": { path: "/repos/a" }, - "proj-b": { path: "/repos/b" }, - }, - configPath: path, - }; + return createMockChild(0, undefined, { stdout: "" }); }); - mockLoadGlobalConfig.mockReturnValue({ - projects: { - "proj-a": { path: "/repos/a" }, - "proj-b": { path: "/repos/b" }, - }, - }); - mockExistsSync.mockReturnValue(true); - // One active session per project. sm.list() is single-call (the SM - // implementation enumerates across all projectIds), so we return both - // sessions in one shot — matching real behavior. `projectId` is - // included so it's visible to anyone reading the refusal output. - mockSessions.value = [ - { id: "proj-a-feat-1", status: "working", projectId: "proj-a" }, - { id: "proj-b-feat-2", status: "needs_input", projectId: "proj-b" }, - ]; - const errSpy = vi.mocked(console.error); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); + await program.parseAsync(["node", "test", "update", "--no-restore"]); - const stderr = errSpy.mock.calls.map((c) => String(c[0])).join("\n"); - // Refusal message reports the correct total count (2, not 1). - expect(stderr).toMatch(/2 sessions active/); - // Both project's session ids appear in the listing. - expect(stderr).toMatch(/proj-a-feat-1/); - expect(stderr).toMatch(/proj-b-feat-2/); - expect(mockSpawn).not.toHaveBeenCalled(); + expect(mockSpawn.mock.calls.at(-1)).toEqual([ + "ao", + ["start", "my-app", "--no-restore"], + expect.objectContaining({ stdio: "inherit" }), + ]); }); - it("always loads global config (never project-local), so sessions in OTHER projects fire the guard", async () => { - // Simulate running inside a project: project-local loadConfig() would - // succeed and return only THIS project's sessions. The guard must - // ignore it and still consult the global registry, otherwise active - // sessions in other projects get missed and the install would proceed. - mockLoadConfig.mockImplementation((path?: string) => { - if (!path) { - // Project-local: would return only "this-project"'s sessions. - return { projects: { "this-project": {} }, configPath: "/cwd/agent-orchestrator.yaml" }; - } - return { - projects: { - "this-project": { path: "/cwd" }, - "other-project": { path: "/other" }, - }, - configPath: path, - }; - }); - mockLoadGlobalConfig.mockReturnValue({ - projects: { - "this-project": { path: "/cwd" }, - "other-project": { path: "/other" }, - }, - }); - mockExistsSync.mockReturnValue(true); - // Active session lives in the OTHER project — only visible via global. - mockSessions.value = [ - { id: "other-1", status: "working" }, - ]; - - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); - - expect(mockLoadGlobalConfig).toHaveBeenCalled(); - // Critical: we did NOT call the project-local (no-arg) loadConfig path. - const noArgCalls = mockLoadConfig.mock.calls.filter((c) => c.length === 0); - expect(noArgCalls).toHaveLength(0); - expect(mockSpawn).not.toHaveBeenCalled(); - }); - - it("uses the global registry when running outside any project", async () => { - mockLoadConfig.mockImplementation((path?: string) => { - if (!path) throw new Error("no config found"); - return { projects: { "my-app": { path: "/tmp/foo" } }, configPath: path }; - }); - mockLoadGlobalConfig.mockReturnValue({ - projects: { "my-app": { path: "/tmp/foo" } }, - }); - mockExistsSync.mockReturnValue(true); - mockSessions.value = [{ id: "feat-1", status: "working" }]; - - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); - - expect(mockLoadGlobalConfig).toHaveBeenCalled(); - expect(mockSpawn).not.toHaveBeenCalled(); - }); - - it("returns early without building SessionManager when global registry is empty", async () => { - mockLoadConfig.mockImplementation((path?: string) => { - if (!path) throw new Error("no config found"); - return { projects: {}, configPath: path }; - }); - mockLoadGlobalConfig.mockReturnValue({ projects: {} }); - mockExistsSync.mockReturnValue(true); - - // Guard returns true (allow update) without ever calling sm.list(). - // No mockSessions configured, no spawn → confirms we never reached - // SessionManager construction. - mockPromptConfirm.mockResolvedValue(false); // decline soft-install - await program.parseAsync(["node", "test", "update"]); - expect(mockLoadGlobalConfig).toHaveBeenCalled(); - // The decline-prompt path means the guard let us through. - expect(mockPromptConfirm).toHaveBeenCalled(); - }); - - it("returns early without building SessionManager when global config file is missing", async () => { - mockLoadConfig.mockImplementation((path?: string) => { - if (!path) throw new Error("no config found"); - return { projects: {}, configPath: path }; - }); - mockExistsSync.mockReturnValue(false); // no ~/.agent-orchestrator/config.yaml - - mockPromptConfirm.mockResolvedValue(false); - await program.parseAsync(["node", "test", "update"]); - // We didn't even consult loadGlobalConfig — existsSync(globalPath) was false. - expect(mockLoadGlobalConfig).not.toHaveBeenCalled(); - expect(mockPromptConfirm).toHaveBeenCalled(); - }); - - it("refuses when sessions exist in a locally-registered project not in global config (Dhruv edge-case)", async () => { - // The bypass: user ran `ao start` from a repo with a local - // agent-orchestrator.yaml and no global registration. running.json - // says that project is being polled, sessions live on disk, but the - // global registry is empty. Before this fix, the guard hit the - // "global has no projects → allow" branch and let `ao update` - // clobber the running daemon. - // - // Fix: consult running.json BEFORE falling back to global. When - // running.json has projects, build the SessionManager from - // running.configPath (which is the local project's yaml in this case) - // and enumerate from there. - mockGetRunning.mockResolvedValue({ - pid: 12345, - configPath: "/repos/local-only/agent-orchestrator.yaml", - port: 3000, - startedAt: new Date().toISOString(), - projects: ["local-only"], - }); - // Global registry has no record of `local-only` — this is the bypass - // condition. With the old code, we'd return true here. - mockLoadGlobalConfig.mockReturnValue({ projects: {} }); - // loadConfig with the local configPath returns the local project's - // OrchestratorConfig (project-local schema is auto-wrapped). - mockLoadConfig.mockImplementation((path?: string) => { - if (path === "/repos/local-only/agent-orchestrator.yaml") { - return { - projects: { "local-only": { path: "/repos/local-only" } }, - configPath: path, - }; - } - return { projects: {}, configPath: path ?? "/cwd/agent-orchestrator.yaml" }; - }); - mockSessions.value = [ - { id: "local-feat-1", status: "working", projectId: "local-only" }, - ]; - - const errSpy = vi.mocked(console.error); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); - - const stderr = errSpy.mock.calls.map((c) => String(c[0])).join("\n"); - expect(stderr).toMatch(/1 session active/); - expect(stderr).toMatch(/local-feat-1/); - // We must have routed through running.configPath, NOT the global path. - expect(mockLoadConfig).toHaveBeenCalledWith("/repos/local-only/agent-orchestrator.yaml"); - expect(mockSpawn).not.toHaveBeenCalled(); - }); - - it("returns true (allows update) when running.json is gone and global is empty", async () => { - // No daemon running, no global projects. Genuinely safe to update. - mockGetRunning.mockResolvedValue(null); - mockExistsSync.mockReturnValue(false); - mockPromptConfirm.mockResolvedValue(false); - await program.parseAsync(["node", "test", "update"]); - expect(mockPromptConfirm).toHaveBeenCalled(); // guard passed → reached prompt - }); - - it("trusts running.json over an inconsistent global config", async () => { - // running.json says project P is being polled. Global config also - // lists P. We should use running.configPath (the live signal), and - // any active session in P fires the guard. + it("aborts before install if ao stop exits 0 but AO still appears active", async () => { mockGetRunning.mockResolvedValue({ pid: 12345, configPath: "/tmp/test-global-config.yaml", @@ -791,23 +695,43 @@ describe("update command", () => { startedAt: new Date().toISOString(), projects: ["my-app"], }); - mockLoadConfig.mockImplementation((path?: string) => ({ - projects: { "my-app": { path: "/tmp/foo" } }, - configPath: path ?? "/cwd/agent-orchestrator.yaml", - })); - mockLoadGlobalConfig.mockReturnValue({ - projects: { "my-app": { path: "/tmp/foo" } }, - }); - mockSessions.value = [{ id: "feat-1", status: "working" }]; + mockSpawn.mockReturnValue(createMockChild(0, undefined, { stdout: "" })); - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); - // Because getRunning() returned a daemon, we went straight to its - // configPath — we should NOT have fallen back to loadGlobalConfig. - expect(mockLoadGlobalConfig).not.toHaveBeenCalled(); - expect(mockSpawn).not.toHaveBeenCalled(); + expect(mockSpawn).toHaveBeenCalledTimes(1); + expect(mockSpawn.mock.calls[0][0]).toBe("ao"); + expect(mockSpawn.mock.calls[0][1]).toEqual(["stop", "--yes"]); + expect(mockSpawn.mock.calls.some((call) => call[0] === "pnpm")).toBe(false); + const stderr = vi.mocked(console.error).mock.calls.map((c) => String(c[0])).join("\n"); + expect(stderr).toContain("AO still appears to be running after `ao stop --yes`"); + }); + + it("prints a friendly pnpm diagnostic and npm fallback when pnpm fails", async () => { + mockGetRunning.mockReset(); + mockGetRunning.mockResolvedValue(null); + mockExistsSync.mockReturnValue(false); + mockSpawn.mockReturnValue( + createMockChild(1, undefined, { + stderr: "ERR_PNPM_UNEXPECTED_VIRTUAL_STORE Unexpected virtual store location\n", + }), + ); + + await expect(program.parseAsync(["node", "test", "update"])).rejects.toThrow( + "process.exit(1)", + ); + + const stderr = vi + .mocked(console.error) + .mock.calls.map((c) => String(c[0])) + .join("\n"); + expect(stderr).toContain("AO was not updated. You are still on version 0.2.2."); + expect(stderr).toContain("pnpm's global store metadata is inconsistent"); + expect(stderr).toContain("You can also try: npm install -g @aoagents/ao@latest"); + expect(stderr).toContain("ERR_PNPM_UNEXPECTED_VIRTUAL_STORE"); + expect(mockInvalidateCache).not.toHaveBeenCalled(); }); }); @@ -874,7 +798,7 @@ describe("update command", () => { }), ); mockPromptConfirm.mockResolvedValue(true); - mockSpawn.mockReturnValue(createMockChild(0)); + mockSpawn.mockReturnValue(createMockChild(0, undefined, { stdout: "0.5.0-nightly-abc\n" })); await program.parseAsync(["node", "test", "update"]); @@ -990,7 +914,7 @@ describe("update command", () => { }), ); mockPromptConfirm.mockResolvedValue(true); - mockSpawn.mockReturnValue(createMockChild(0)); + mockSpawn.mockReturnValue(createMockChild(0, undefined, { stdout: "0.5.0-nightly-abc\n" })); await program.parseAsync(["node", "test", "update"]); @@ -1035,9 +959,7 @@ describe("update command", () => { beforeEach(() => { mockDetectInstallMethod.mockReturnValue("npm-global"); mockResolveUpdateChannel.mockReturnValue("stable"); - mockCheckForUpdate.mockResolvedValue( - makeNpmUpdateInfo({ installMethod: "npm-global" }), - ); + mockCheckForUpdate.mockResolvedValue(makeNpmUpdateInfo({ installMethod: "npm-global" })); mockExistsSync.mockReturnValue(true); mockLoadGlobalConfig.mockReturnValue({ projects: { "my-app": { path: "/tmp/foo" } }, @@ -1069,7 +991,11 @@ describe("update command", () => { // would run. Asserting spawn was called proves the install actually // happens in the API-invoked path. await program.parseAsync(["node", "test", "update"]); - expect(mockSpawn).toHaveBeenCalledTimes(1); + expect(mockSpawn).toHaveBeenCalledWith( + "npm", + expect.arrayContaining(["install"]), + expect.anything(), + ); // And without a TTY, we MUST NOT have prompted — that would hang the // detached child forever. expect(mockPromptConfirm).not.toHaveBeenCalled(); @@ -1084,12 +1010,25 @@ describe("update command", () => { expect(mockSpawn).not.toHaveBeenCalled(); }); - it("still refuses on active sessions even when API-invoked (the API's own guard isn't a single point of trust)", async () => { + it("orchestrates stop/install/verify/start when active sessions exist and update is API-invoked", async () => { mockSessions.value = [{ id: "feat-1", status: "working" }]; - await expect( - program.parseAsync(["node", "test", "update"]), - ).rejects.toThrow("process.exit(1)"); - expect(mockSpawn).not.toHaveBeenCalled(); + mockExistsSync.mockReturnValue(false); + mockGetRunning + .mockResolvedValueOnce({ + pid: 12345, + configPath: "/tmp/test-global-config.yaml", + port: 3000, + startedAt: new Date().toISOString(), + projects: ["my-app"], + }) + .mockResolvedValue(null); + + await program.parseAsync(["node", "test", "update"]); + + expect(mockSpawn.mock.calls[0][0]).toBe("ao"); + expect(mockSpawn.mock.calls[0][1]).toEqual(["stop", "--yes"]); + expect(mockSpawn.mock.calls.at(-1)?.[0]).toBe("ao"); + expect(mockSpawn.mock.calls.at(-1)?.[1]).toEqual(["start", "my-app", "--restore"]); }); }); @@ -1128,17 +1067,15 @@ describe("update command", () => { it("passes shell:true and windowsHide:true on Windows so PATHEXT resolves npm.cmd", async () => { mockIsWindows.mockReturnValue(true); await program.parseAsync(["node", "test", "update"]); - expect(mockSpawn).toHaveBeenCalledTimes(1); const opts = mockSpawn.mock.calls[0][2] as Record; expect(opts.shell).toBe(true); expect(opts.windowsHide).toBe(true); - expect(opts.stdio).toBe("inherit"); + expect(opts.stdio).toEqual(["inherit", "pipe", "pipe"]); }); it("passes shell:false on macOS / Linux (no shell wrap needed)", async () => { mockIsWindows.mockReturnValue(false); await program.parseAsync(["node", "test", "update"]); - expect(mockSpawn).toHaveBeenCalledTimes(1); const opts = mockSpawn.mock.calls[0][2] as Record; expect(opts.shell).toBe(false); }); diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 27e660bd8..79850587c 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -878,7 +878,14 @@ async function runStartup( config: OrchestratorConfig, projectId: string, project: ProjectConfig, - opts?: { dashboard?: boolean; orchestrator?: boolean; rebuild?: boolean; dev?: boolean }, + opts?: { + dashboard?: boolean; + orchestrator?: boolean; + rebuild?: boolean; + dev?: boolean; + /** true = restore without prompting, false = skip restore, undefined = prompt for humans */ + restore?: boolean; + }, ): Promise { await runtimePreflight(config); @@ -1011,11 +1018,14 @@ async function runStartup( } } - // Check for sessions from last `ao stop` and offer to restore them - if (isHumanCaller()) { + // Check for sessions from last `ao stop` and restore/prompt/skip based on caller intent. + if (opts?.restore !== false && (opts?.restore === true || isHumanCaller())) { try { const lastStop = await readLastStop(); - if (lastStop && lastStop.sessionIds.length > 0) { + const totalLastStopSessions = + (lastStop?.sessionIds.length ?? 0) + + (lastStop?.otherProjects ?? []).reduce((sum, p) => sum + p.sessionIds.length, 0); + if (lastStop && totalLastStopSessions > 0) { const stoppedAgo = `stopped at ${new Date(lastStop.stoppedAt).toLocaleString()}`; const otherProjects = lastStop.otherProjects ?? []; const restoreProjectBySessionId = new Map(); @@ -1056,7 +1066,8 @@ async function runStartup( } if (allRestoreSessions.length > 0) { - const shouldRestore = await promptConfirm("Restore these sessions?", true); + const shouldRestore = + opts?.restore === true ? true : await promptConfirm("Restore these sessions?", true); if (shouldRestore) { recordActivityEvent({ projectId, @@ -1441,6 +1452,8 @@ export function registerStart(program: Command): void { .option("--dev", "Use Next.js dev server with hot reload (for dashboard UI development)") .option("--interactive", "Prompt to configure config settings") .option("--reap-orphans", "Kill orphaned AO child processes before starting") + .option("--restore", "Restore sessions from last ao stop without prompting") + .option("--no-restore", "Skip restoring sessions from last ao stop") .action( async ( projectArg?: string, @@ -1451,6 +1464,7 @@ export function registerStart(program: Command): void { dev?: boolean; interactive?: boolean; reapOrphans?: boolean; + restore?: boolean; }, ) => { recordActivityEvent({ @@ -1847,7 +1861,8 @@ export function registerStop(program: Command): void { .description("Stop orchestrator agent and dashboard") .option("--purge-session", "Delete mapped OpenCode session when stopping") .option("--all", "Stop all running AO instances") - .action(async (projectArg?: string, opts: { purgeSession?: boolean; all?: boolean } = {}) => { + .option("-y, --yes", "Confirm stopping active sessions without prompting") + .action(async (projectArg?: string, opts: { purgeSession?: boolean; all?: boolean; yes?: boolean } = {}) => { recordActivityEvent({ source: "cli", kind: "cli.stop_invoked", @@ -1896,10 +1911,30 @@ export function registerStop(program: Command): void { config = loadConfig(globalPath); } } - const { projectId: _projectId, project } = await resolveProject(config, projectArg, "stop"); + let _projectId: string; + let project: ProjectConfig; + if (projectArg) { + ({ projectId: _projectId, project } = await resolveProject(config, projectArg, "stop")); + } else { + const projectIds = Object.keys(config.projects); + if (projectIds.length === 0) { + throw new Error("No projects configured. Add a project to agent-orchestrator.yaml."); + } + const currentDir = resolve(cwd()); + const cwdProjectId = findProjectForDirectory(config.projects, currentDir); + _projectId = + running?.projects.find((id) => config.projects[id]) ?? + cwdProjectId ?? + projectIds[0]; + project = config.projects[_projectId]; + } const port = config.port ?? DEFAULT_PORT; - console.log(chalk.bold(`\nStopping orchestrator for ${chalk.cyan(project.name)}\n`)); + if (projectArg) { + console.log(chalk.bold(`\nStopping orchestrator for ${chalk.cyan(project.name)}\n`)); + } else { + console.log(chalk.bold(`\nStopping AO across all projects\n`)); + } const sm = await getSessionManager(config); try { @@ -1925,6 +1960,16 @@ export function registerStop(program: Command): void { const otherByProject = new Map(); if (activeSessions.length > 0) { + if (!projectArg && opts.yes !== true && isHumanCaller()) { + const confirmed = await promptConfirm( + `Stop AO and ${activeSessions.length} active session(s)?`, + false, + ); + if (!confirmed) { + console.log(chalk.yellow("Stop cancelled.")); + return; + } + } const spinner = ora(`Stopping ${activeSessions.length} active session(s)`).start(); const purgeOpenCode = opts?.purgeSession === true; const warnings: string[] = []; diff --git a/packages/cli/src/commands/update.ts b/packages/cli/src/commands/update.ts index 8e3d37dd9..16f13ede0 100644 --- a/packages/cli/src/commands/update.ts +++ b/packages/cli/src/commands/update.ts @@ -68,12 +68,16 @@ export function registerUpdate(program: Command): void { .option("--skip-smoke", "Skip smoke tests after rebuilding (git installs only)") .option("--smoke-only", "Run smoke tests without fetching or rebuilding (git installs only)") .option("--check", "Print version info as JSON without upgrading") + .option("--no-restore", "Restart AO after updating but do not restore stopped sessions") .action( - async (opts: { skipSmoke?: boolean; smokeOnly?: boolean; check?: boolean }) => { + async (opts: { + skipSmoke?: boolean; + smokeOnly?: boolean; + check?: boolean; + restore?: boolean; + }) => { if (opts.skipSmoke && opts.smokeOnly) { - console.error( - "`ao update` does not allow `--skip-smoke` together with `--smoke-only`.", - ); + console.error("`ao update` does not allow `--skip-smoke` together with `--smoke-only`."); process.exit(1); } @@ -113,7 +117,7 @@ export function registerUpdate(program: Command): void { case "npm-global": case "pnpm-global": case "bun-global": - await handleNpmUpdate(method); + await handleNpmUpdate(method, { restore: opts.restore !== false }); break; case "unknown": await handleUnknownUpdate(); @@ -133,21 +137,24 @@ async function handleCheck(): Promise { } // --------------------------------------------------------------------------- -// Active-session guard +// Update lifecycle planning // --------------------------------------------------------------------------- /** - * Refuse to update when the user has live sessions. - * - * Auto-stopping would lose the agent's in-flight context (and potentially - * uncommitted work). The release doc is explicit: refuse, surface the - * `ao stop` command, let the user decide. - * - * Best-effort — when no config is reachable (fresh install, broken yaml) - * we skip the guard rather than blocking the update on a missing dependency. + * Best-effort snapshot used by `ao update` to pause and resume AO around the + * package-manager install. Missing/broken config should not block an update; + * in that case we proceed without attempting a stop/start round trip. */ -async function ensureNoActiveSessions(): Promise { +interface UpdateLifecyclePlan { + runningBeforeUpdate: boolean; + primaryProjectId?: string; + activeSessions: Session[]; +} + +async function getUpdateLifecyclePlan(): Promise { let sessions: Session[]; + let primaryProjectId: string | undefined; + let runningBeforeUpdate = false; try { // Live signal first: running.json lists whichever projects the active // `ao start` daemon is currently polling. That can include local-only @@ -160,6 +167,8 @@ async function ensureNoActiveSessions(): Promise { // truth for "which sessions does the running ao instance own?" const running = await getRunning(); if (running && running.projects.length > 0) { + runningBeforeUpdate = true; + primaryProjectId = running.projects[0]; // running.configPath could be local-wrapped (a project's // agent-orchestrator.yaml) OR the canonical global path. loadConfig // dispatches based on the path shape — both cases produce a full @@ -174,15 +183,18 @@ async function ensureNoActiveSessions(): Promise { // SessionManager's enrichment will reconcile any stale-runtime // sessions to `killed`, so terminal statuses don't block the update. const globalPath = getGlobalConfigPath(); - if (!existsSync(globalPath)) return true; + if (!existsSync(globalPath)) { + return { runningBeforeUpdate, primaryProjectId, activeSessions: [] }; + } const globalConfig = loadGlobalConfig(globalPath); if (!globalConfig || Object.keys(globalConfig.projects).length === 0) { - return true; + return { runningBeforeUpdate, primaryProjectId, activeSessions: [] }; } if (!isCanonicalGlobalConfigPath(globalPath)) { - return true; + return { runningBeforeUpdate, primaryProjectId, activeSessions: [] }; } const config = loadConfig(globalPath); + primaryProjectId = Object.keys(config.projects)[0]; const sm = await getSessionManager(config); sessions = await sm.list(); } @@ -190,29 +202,126 @@ async function ensureNoActiveSessions(): Promise { // If we can't enumerate sessions, don't pretend there are zero — but // also don't block the upgrade indefinitely. Surface a soft warning. console.error( - chalk.yellow( - "⚠ Could not check for active sessions before updating. Proceeding anyway.", - ), + chalk.yellow("⚠ Could not check for active sessions before updating. Proceeding anyway."), ); - return true; + return { runningBeforeUpdate, primaryProjectId, activeSessions: [] }; } const active = sessions.filter((s) => ACTIVE_SESSION_STATUSES.has(s.status)); - if (active.length === 0) return true; + return { runningBeforeUpdate, primaryProjectId, activeSessions: active }; +} - const noun = active.length === 1 ? "session" : "sessions"; - console.error( - chalk.red( - `\n✗ ${active.length} ${noun} active. Run \`ao stop\` first, then \`ao update\`.\n`, - ), - ); - for (const s of active.slice(0, 5)) { - console.error(chalk.dim(` • ${s.id} (${s.status})`)); +async function pauseAoForUpdate(plan: UpdateLifecyclePlan): Promise { + const shouldStop = plan.runningBeforeUpdate || plan.activeSessions.length > 0; + if (!shouldStop) return false; + + if (plan.activeSessions.length > 0) { + const noun = plan.activeSessions.length === 1 ? "session" : "sessions"; + console.log( + chalk.yellow( + `\n${plan.activeSessions.length} active ${noun} will be paused and restored after the update.`, + ), + ); + for (const s of plan.activeSessions.slice(0, 5)) { + console.log(chalk.dim(` • ${s.id} (${s.status})`)); + } + if (plan.activeSessions.length > 5) { + console.log(chalk.dim(` … and ${plan.activeSessions.length - 5} more`)); + } + } else { + console.log(chalk.dim("\nAO is running; it will be restarted after the update.")); } - if (active.length > 5) { - console.error(chalk.dim(` … and ${active.length - 5} more`)); + + const stopExit = await runAoLifecycleCommand(["stop", "--yes"]); + if (stopExit !== 0) { + recordActivityEvent({ + source: "cli", + kind: "cli.update_failed", + level: "error", + summary: `ao update failed: internal ao stop exited non-zero`, + data: { exitCode: stopExit }, + }); + console.error(chalk.red(`\nAO update could not stop the running daemon (exit ${stopExit}).`)); + process.exit(stopExit); } - return false; + + const afterStop = await getUpdateLifecyclePlan(); + if (afterStop.runningBeforeUpdate || afterStop.activeSessions.length > 0) { + recordActivityEvent({ + source: "cli", + kind: "cli.update_failed", + level: "error", + summary: `ao update failed: AO still appears active after internal ao stop`, + data: { + runningAfterStop: afterStop.runningBeforeUpdate, + activeSessionCount: afterStop.activeSessions.length, + activeSessionIds: afterStop.activeSessions.map((s) => s.id).slice(0, 20), + }, + }); + console.error( + chalk.red( + "\nAO update stopped before installing because AO still appears to be running after `ao stop --yes`.", + ), + ); + if (afterStop.activeSessions.length > 0) { + console.error(chalk.dim("Still-active sessions:")); + for (const s of afterStop.activeSessions.slice(0, 5)) { + console.error(chalk.dim(` • ${s.id} (${s.status})`)); + } + if (afterStop.activeSessions.length > 5) { + console.error(chalk.dim(` … and ${afterStop.activeSessions.length - 5} more`)); + } + } + console.error(chalk.dim("Run `ao stop` and retry `ao update` after AO is fully stopped.")); + process.exit(1); + } + + return plan.runningBeforeUpdate; +} + +async function restartAoAfterUpdate( + plan: UpdateLifecyclePlan, + opts: { restore: boolean }, +): Promise { + const args = ["start"]; + if (plan.primaryProjectId) args.push(plan.primaryProjectId); + args.push(opts.restore ? "--restore" : "--no-restore"); + + console.log(chalk.dim(`\nRestarting AO: ao ${args.join(" ")}`)); + const exitCode = await runAoLifecycleCommand(args); + if (exitCode !== 0) { + recordActivityEvent({ + source: "cli", + kind: "cli.update_restart_failed", + level: "error", + summary: `ao update could not restart AO after install`, + data: { exitCode, args }, + }); + console.error( + chalk.yellow( + `\nAO was updated, but \`ao ${args.join(" ")}\` failed with exit ${exitCode}. ` + + `Run it manually to restore your sessions.`, + ), + ); + process.exit(exitCode); + } +} + +function runAoLifecycleCommand(args: string[]): Promise { + return new Promise((resolveExit) => { + const child = spawn("ao", args, { + stdio: "inherit", + shell: isWindows(), + windowsHide: true, + }); + child.on("error", (error) => { + console.error(chalk.yellow(`Could not run ao ${args.join(" ")}: ${error.message}`)); + resolveExit(1); + }); + child.on("exit", (code, signal) => { + resolveExit(signal ? 1 : (code ?? 1)); + }); + }); } // --------------------------------------------------------------------------- @@ -222,10 +331,10 @@ async function ensureNoActiveSessions(): Promise { async function handleGitUpdate(opts: { skipSmoke?: boolean; smokeOnly?: boolean; + restore?: boolean; }): Promise { - if (!(await ensureNoActiveSessions())) { - process.exit(1); - } + const lifecyclePlan = await getUpdateLifecyclePlan(); + const shouldRestart = await pauseAoForUpdate(lifecyclePlan); const args: string[] = []; if (opts.skipSmoke) args.push("--skip-smoke"); @@ -241,14 +350,17 @@ async function handleGitUpdate(opts: { summary: `ao update (git) failed: ao-update.sh exited non-zero`, data: { method: "git", exitCode }, }); + if (shouldRestart) { + await restartAoAfterUpdate(lifecyclePlan, { restore: opts.restore !== false }); + } process.exit(exitCode); } invalidateCache(); + if (shouldRestart) { + await restartAoAfterUpdate(lifecyclePlan, { restore: opts.restore !== false }); + } } catch (error) { - if ( - error instanceof Error && - error.message.includes("Script not found: ao-update.sh") - ) { + if (error instanceof Error && error.message.includes("Script not found: ao-update.sh")) { recordActivityEvent({ source: "cli", kind: "cli.update_failed", @@ -263,6 +375,9 @@ async function handleGitUpdate(opts: { "If you're on a package install, reinstall the package.", ), ); + if (shouldRestart) { + await restartAoAfterUpdate(lifecyclePlan, { restore: opts.restore !== false }); + } process.exit(1); } @@ -277,6 +392,9 @@ async function handleGitUpdate(opts: { }, }); console.error(error instanceof Error ? error.message : String(error)); + if (shouldRestart) { + await restartAoAfterUpdate(lifecyclePlan, { restore: opts.restore !== false }); + } process.exit(1); } } @@ -285,7 +403,7 @@ async function handleGitUpdate(opts: { // npm / pnpm / bun global install // --------------------------------------------------------------------------- -async function handleNpmUpdate(method: InstallMethod): Promise { +async function handleNpmUpdate(method: InstallMethod, opts: { restore: boolean }): Promise { const channel = resolveUpdateChannel(); // Snapshot the previously cached channel BEFORE we force a refresh, so we @@ -332,9 +450,7 @@ async function handleNpmUpdate(method: InstallMethod): Promise { // is the right UX for a channel transition, and the install command we'd // run is genuinely different even if the version-compare says "no". const isChannelSwitch = - !info.isOutdated && - previousChannel !== undefined && - previousChannel !== channel; + !info.isOutdated && previousChannel !== undefined && previousChannel !== channel; // First-channel opt-in. previousChannel === undefined means we've never // installed via the auto-updater. A user who just ran `ao config set @@ -363,9 +479,7 @@ async function handleNpmUpdate(method: InstallMethod): Promise { console.log(`Channel: ${chalk.cyan(channel)}`); if (isChannelSwitch) { console.log( - chalk.yellow( - `\nChannel switch detected: was on ${previousChannel}, now ${channel}.`, - ), + chalk.yellow(`\nChannel switch detected: was on ${previousChannel}, now ${channel}.`), ); console.log( chalk.dim( @@ -384,15 +498,12 @@ async function handleNpmUpdate(method: InstallMethod): Promise { const command = getUpdateCommand(method, channel); const apiInvoked = isApiInvoked(); const interactive = isTTY() && !apiInvoked; + const lifecyclePlan = await getUpdateLifecyclePlan(); - // Non-interactive path: API-invoked OR piped output. We still gate on the - // active-session guard (refusing returns true/false), but we never bail - // out just because there's no terminal — the dashboard's "Update" click - // must actually install. The only thing we skip is the confirm prompt. - if (!(await ensureNoActiveSessions())) { - process.exit(1); - } - + // Non-interactive path: API-invoked OR piped output. We still plan the + // stop/start lifecycle, but we never bail out just because there's no + // terminal — the dashboard's "Update" click must actually install. The + // only thing we skip is the confirm prompt. if (interactive) { // Soft auto-install: when the user has opted into stable or nightly we // skip the confirm prompt — they've already said "keep me on this channel." @@ -404,10 +515,7 @@ async function handleNpmUpdate(method: InstallMethod): Promise { isChannelSwitch || isFirstChannelOptIn ? `Switch to ${channel} via ${chalk.cyan(command)}?` : `Run ${chalk.cyan(command)}?`; - const confirmed = await promptConfirm( - promptText, - !(isChannelSwitch || isFirstChannelOptIn), - ); + const confirmed = await promptConfirm(promptText, !(isChannelSwitch || isFirstChannelOptIn)); if (!confirmed) return; } else { console.log(chalk.dim(`Updating: ${command}`)); @@ -422,25 +530,95 @@ async function handleNpmUpdate(method: InstallMethod): Promise { return; } - const exitCode = await runNpmInstall(command); - if (exitCode === 0) { - invalidateCache(); - console.log(chalk.green("\nUpdate complete.")); - } else { + const shouldRestart = await pauseAoForUpdate(lifecyclePlan); + const installResult = await runNpmInstall(command); + if (installResult.exitCode !== 0) { recordActivityEvent({ source: "cli", kind: "cli.update_failed", level: "error", summary: `ao update (${method}) failed: install command exited non-zero`, - data: { method, command, exitCode }, + data: { + method, + command, + exitCode: installResult.exitCode, + classification: classifyInstallFailure(installResult.output).kind, + }, }); - process.exit(exitCode); + printInstallFailure({ + method, + command, + channel, + currentVersion: info.currentVersion, + exitCode: installResult.exitCode, + output: installResult.output, + }); + if (shouldRestart) { + console.log(chalk.dim("\nRestarting AO with the existing installation...")); + await restartAoAfterUpdate(lifecyclePlan, opts); + } + process.exit(1); } + + const verification = await verifyInstalledVersion(info.latestVersion, info.currentVersion); + if (!verification.ok) { + recordActivityEvent({ + source: "cli", + kind: "cli.update_failed", + level: "error", + summary: `ao update (${method}) failed: installed version verification failed`, + data: { + method, + command, + expectedVersion: info.latestVersion, + actualVersion: verification.actualVersion, + output: verification.output, + }, + }); + console.error(chalk.red(`\nAO was not verified after install.`)); + console.error(chalk.yellow(verification.message)); + console.error(chalk.dim(`Expected: ${info.latestVersion}`)); + console.error(chalk.dim(`Current before update: ${info.currentVersion}`)); + if (shouldRestart) { + console.log(chalk.dim("\nRestarting AO before exiting...")); + await restartAoAfterUpdate(lifecyclePlan, opts); + } + process.exit(1); + } + + invalidateCache(); + if (shouldRestart) { + await restartAoAfterUpdate(lifecyclePlan, opts); + } + console.log( + chalk.green( + `\nUpdate complete: ${info.currentVersion} → ${verification.actualVersion}.` + + (shouldRestart ? " AO restarted." : ""), + ), + ); } -function runNpmInstall(command: string): Promise { +interface CommandResult { + exitCode: number; + output: string; +} + +function runNpmInstall(command: string): Promise { const [cmd, ...args] = command.split(" "); - return new Promise((resolveExit, reject) => { + return runCommandCapture(cmd!, args, { echo: true }).then((result) => { + if (result.exitCode !== 0) { + console.error(chalk.yellow(`\n${cmd} exited with code ${result.exitCode}.`)); + } + return result; + }); +} + +function runCommandCapture( + cmd: string, + args: string[], + opts: { echo?: boolean } = {}, +): Promise { + return new Promise((resolveExit) => { // `shell: isWindows()` is required so PATHEXT gets consulted on Windows — // npm/pnpm/bun install as `*.cmd` shims, and Node.js does not look at // PATHEXT for non-shell spawns, so a bare `npm` / `pnpm` / `bun` lookup @@ -448,25 +626,162 @@ function runNpmInstall(command: string): Promise { // keeps the shell window from flashing. Same fix that landed for the // dashboard's /api/update spawn in commit 9f29131d. const child = spawn(cmd!, args, { - stdio: "inherit", + stdio: ["inherit", "pipe", "pipe"], shell: isWindows(), windowsHide: true, }); - child.on("error", reject); + let output = ""; + const collect = (chunk: Buffer | string, stream: NodeJS.WriteStream): void => { + const text = chunk.toString(); + output += text; + if (opts.echo) stream.write(chunk); + }; + child.stdout?.on("data", (chunk: Buffer | string) => collect(chunk, process.stdout)); + child.stderr?.on("data", (chunk: Buffer | string) => collect(chunk, process.stderr)); + child.on("error", (error) => { + output += `${error.name}: ${error.message}`; + resolveExit({ exitCode: 1, output }); + }); child.on("exit", (code, signal) => { if (signal) { - resolveExit(1); + resolveExit({ exitCode: 1, output: `${output}\nTerminated by signal ${signal}` }); return; } - if (code !== 0) { - console.error(chalk.yellow(`\n${cmd} exited with code ${code}.`)); - } - resolveExit(code ?? 1); + resolveExit({ exitCode: code ?? 1, output }); }); }); } +interface VerificationResult { + ok: boolean; + actualVersion?: string; + output: string; + message: string; +} + +async function verifyInstalledVersion( + expectedVersion: string, + previousVersion: string, +): Promise { + const result = await runCommandCapture("ao", ["--version"]); + const output = result.output.trim(); + const actualVersion = parseAoVersion(output); + + if (result.exitCode !== 0) { + return { + ok: false, + output, + message: `\`ao --version\` failed with exit ${result.exitCode}.`, + }; + } + if (!actualVersion) { + return { + ok: false, + output, + message: `Could not parse \`ao --version\` output: ${output || ""}`, + }; + } + if (actualVersion !== expectedVersion) { + return { + ok: false, + actualVersion, + output, + message: + actualVersion === previousVersion + ? `The install command exited successfully, but AO is still on ${previousVersion}.` + : `The install command exited successfully, but AO reports ${actualVersion}.`, + }; + } + + return { ok: true, actualVersion, output, message: "verified" }; +} + +function parseAoVersion(output: string): string | undefined { + const match = output.match(/\b(\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?)\b/); + return match?.[1]; +} + +function classifyInstallFailure(output: string): { kind: string; guidance: string } { + if (/ERR_PNPM_UNEXPECTED_VIRTUAL_STORE/i.test(output)) { + return { + kind: "pnpm_virtual_store", + guidance: + "pnpm's global store metadata is inconsistent. Try `pnpm store prune`, then retry `ao update`. " + + "If pnpm remains stuck, use the npm fallback below.", + }; + } + if (/(?:EACCES|EPERM|permission denied|access denied)/i.test(output)) { + return { + kind: "permission", + guidance: + "The package manager could not write to the global install location. Fix your npm/pnpm global prefix permissions, or retry from a shell with access to that directory.", + }; + } + if (/(?:ENETUNREACH|ECONNRESET|ETIMEDOUT|EAI_AGAIN|network|socket hang up)/i.test(output)) { + return { + kind: "network", + guidance: + "The registry request failed due to a network error. Check connectivity/VPN/proxy settings and retry `ao update`.", + }; + } + if ( + /(?:lockfile|ERR_PNPM_LOCKFILE|ERR_PNPM_OUTDATED_LOCKFILE|ERR_PNPM_BROKEN_LOCKFILE)/i.test( + output, + ) + ) { + return { + kind: "lockfile", + guidance: + "pnpm reported lockfile state problems. Clear the affected global install metadata or retry with the npm fallback below.", + }; + } + if ( + /(?:registry|ERR_PNPM_FETCH|ERR_PNPM_META_FETCH_FAIL|E401|E403|E404|404 Not Found|401 Unauthorized|403 Forbidden)/i.test( + output, + ) + ) { + return { + kind: "registry", + guidance: + "The npm registry rejected or failed the package request. Check registry configuration, auth tokens, and the selected AO update channel.", + }; + } + return { + kind: "unknown", + guidance: + "The package manager failed before AO could verify the new version. Retry `ao update` after addressing the package-manager error below.", + }; +} + +function printInstallFailure(opts: { + method: InstallMethod; + command: string; + channel: ReturnType; + currentVersion: string; + exitCode: number; + output: string; +}): void { + const classification = classifyInstallFailure(opts.output); + const fallbackCommand = getUpdateCommand("npm-global", opts.channel); + + console.error( + chalk.red(`\nAO was not updated. You are still on version ${opts.currentVersion}.`), + ); + console.error( + chalk.yellow( + `The package manager (${opts.method.replace("-global", "")}) failed with exit ${opts.exitCode}.`, + ), + ); + console.error(chalk.yellow(classification.guidance)); + console.error(chalk.dim(`\nTo retry: ao update`)); + if (opts.command !== fallbackCommand) { + console.error(chalk.dim(`You can also try: ${fallbackCommand}`)); + } + console.error(chalk.dim("\nPackage manager output:")); + console.error(opts.output.trim() || ""); +} + // --------------------------------------------------------------------------- // homebrew install (notice only) // --------------------------------------------------------------------------- @@ -480,9 +795,7 @@ async function handleHomebrewUpdate(): Promise { console.log(`Latest version: ${chalk.green(info.latestVersion)}`); } console.log(); - console.log( - `Homebrew installs are managed by brew. Run:\n ${chalk.cyan("brew upgrade ao")}`, - ); + console.log(`Homebrew installs are managed by brew. Run:\n ${chalk.cyan("brew upgrade ao")}`); console.log( chalk.dim( " (AO does not auto-install for brew installs because it would clobber brew's symlinks.)",