diff --git a/packages/cli/__tests__/commands/spawn.test.ts b/packages/cli/__tests__/commands/spawn.test.ts index c5ed05393..4f149a7ed 100644 --- a/packages/cli/__tests__/commands/spawn.test.ts +++ b/packages/cli/__tests__/commands/spawn.test.ts @@ -52,8 +52,18 @@ vi.mock("@aoagents/ao-core", async (importOriginal) => { }; }); +// Default registry returns no plugins → preflight loop is a no-op. Tests that +// need a specific plugin's preflight to fire override mockRegistryGet. +const mockRegistryGet = vi.fn().mockReturnValue(null); vi.mock("../../src/lib/create-session-manager.js", () => ({ getSessionManager: async (): Promise => mockSessionManager as SessionManager, + getPluginRegistry: async () => ({ + register: vi.fn(), + get: mockRegistryGet, + list: vi.fn().mockReturnValue([]), + loadBuiltins: vi.fn(), + loadFromConfig: vi.fn(), + }), })); vi.mock("../../src/lib/running-state.js", () => ({ @@ -124,6 +134,7 @@ beforeEach(() => { mockSessionManager.claimPR.mockReset(); mockExec.mockReset(); mockGetRunning.mockReset(); + mockRegistryGet.mockReset().mockReturnValue(null); mockGetRunning.mockResolvedValue({ pid: 1234, port: 3000, startedAt: "", projects: ["my-app"] }); }); @@ -673,23 +684,14 @@ describe("spawn command", () => { }); describe("spawn pre-flight checks", () => { - it("fails with clear error when tmux is not installed (default runtime)", async () => { - mockExec.mockRejectedValue(new Error("ENOENT")); + // The spawn CLI now iterates the configured plugins and calls each one's + // optional preflight(). Plugin-internal checks (e.g. checkTmux, gh auth + // status) live in the plugin packages — see runtime-tmux / tracker-github / + // scm-github tests for that coverage. These tests verify the orchestration: + // the right plugins are iterated, and the intent context is forwarded. - await expect(program.parseAsync(["node", "test", "spawn"])).rejects.toThrow( - "process.exit(1)", - ); - - const errors = vi - .mocked(console.error) - .mock.calls.map((c) => String(c[0])) - .join("\n"); - expect(errors).toContain("tmux"); - expect(mockSessionManager.spawn).not.toHaveBeenCalled(); - }); - - it("skips tmux check when runtime is not tmux", async () => { - const fakeSession: Session = { + function makeFakeSession(overrides: Partial = {}): Session { + return { id: "app-1", projectId: "my-app", status: "spawning", @@ -698,98 +700,77 @@ describe("spawn pre-flight checks", () => { issueId: null, pr: null, workspacePath: "/tmp/wt", - runtimeHandle: { id: "proc-1", runtimeName: "process", data: {} }, + runtimeHandle: { id: "hash-1", runtimeName: "tmux", data: {} }, agentInfo: null, createdAt: new Date(), lastActivityAt: new Date(), metadata: {}, + ...overrides, }; - mockSessionManager.spawn.mockResolvedValue(fakeSession); + } - // Set runtime to "process" - (mockConfigRef.current as Record).defaults = { - runtime: "process", - agent: "claude-code", - workspace: "worktree", - notifiers: ["desktop"], - }; + it("surfaces a plugin's preflight error and aborts before sm.spawn", async () => { + mockRegistryGet.mockImplementation((slot: string) => { + if (slot === "runtime") { + return { + name: "tmux", + preflight: vi + .fn() + .mockRejectedValue(new Error("tmux is not installed. Install it: brew install tmux")), + }; + } + return null; + }); - // exec would fail for tmux but should never be called - mockExec.mockRejectedValue(new Error("ENOENT")); + await expect(program.parseAsync(["node", "test", "spawn"])).rejects.toThrow("process.exit(1)"); - await program.parseAsync(["node", "test", "spawn"]); - - expect(mockSessionManager.spawn).toHaveBeenCalled(); + const errors = vi + .mocked(console.error) + .mock.calls.map((c) => String(c[0])) + .join("\n"); + expect(errors).toContain("tmux is not installed"); + expect(mockSessionManager.spawn).not.toHaveBeenCalled(); }); - it("checks gh auth when tracker is github", async () => { + it("skips scm.preflight when --claim-pr is not provided", async () => { + const trackerPreflight = vi.fn().mockResolvedValue(undefined); + const scmPreflight = vi.fn().mockResolvedValue(undefined); + mockRegistryGet.mockImplementation((slot: string) => { + if (slot === "tracker") return { name: "github", preflight: trackerPreflight }; + if (slot === "scm") return { name: "github", preflight: scmPreflight }; + return null; + }); + const projects = (mockConfigRef.current as Record).projects as Record< string, Record >; projects["my-app"].tracker = { plugin: "github" }; + projects["my-app"].scm = { plugin: "github" }; - // tmux check passes, gh --version passes, gh auth status fails - mockExec - .mockResolvedValueOnce({ stdout: "tmux 3.3a", stderr: "" }) // tmux -V - .mockResolvedValueOnce({ stdout: "gh version 2.40", stderr: "" }) // gh --version - .mockRejectedValueOnce(new Error("not logged in")); // gh auth status + mockSessionManager.spawn.mockResolvedValue(makeFakeSession()); - await expect(program.parseAsync(["node", "test", "spawn"])).rejects.toThrow( - "process.exit(1)", - ); + await program.parseAsync(["node", "test", "spawn"]); - const errors = vi - .mocked(console.error) - .mock.calls.map((c) => String(c[0])) - .join("\n"); - expect(errors).toContain("not authenticated"); - expect(mockSessionManager.spawn).not.toHaveBeenCalled(); + expect(trackerPreflight).toHaveBeenCalled(); + expect(scmPreflight).not.toHaveBeenCalled(); + expect(mockSessionManager.spawn).toHaveBeenCalled(); }); - it("checks gh auth when --claim-pr targets a github SCM project", async () => { + it("calls scm.preflight with willClaimExistingPR=true when --claim-pr is provided", async () => { + const scmPreflight = vi.fn().mockResolvedValue(undefined); + mockRegistryGet.mockImplementation((slot: string) => { + if (slot === "scm") return { name: "github", preflight: scmPreflight }; + return null; + }); + const projects = (mockConfigRef.current as Record).projects as Record< string, Record >; - projects["my-app"].tracker = { plugin: "linear" }; projects["my-app"].scm = { plugin: "github" }; - mockExec - .mockResolvedValueOnce({ stdout: "tmux 3.3a", stderr: "" }) - .mockResolvedValueOnce({ stdout: "gh version 2.40", stderr: "" }) - .mockRejectedValueOnce(new Error("not logged in")); - - await expect( - program.parseAsync(["node", "test", "spawn", "--claim-pr", "123"]), - ).rejects.toThrow("process.exit(1)"); - - const errors = vi - .mocked(console.error) - .mock.calls.map((c) => String(c[0])) - .join("\n"); - expect(errors).toContain("not authenticated"); - expect(mockSessionManager.spawn).not.toHaveBeenCalled(); - }); - - it("handles tracker+scm github preflight when claiming during spawn", async () => { - const fakeSession: Session = { - id: "app-1", - projectId: "my-app", - status: "spawning", - activity: null, - branch: null, - issueId: null, - pr: null, - workspacePath: "/tmp/wt", - runtimeHandle: { id: "hash-app-1", runtimeName: "tmux", data: {} }, - agentInfo: null, - createdAt: new Date(), - lastActivityAt: new Date(), - metadata: {}, - }; - - mockSessionManager.spawn.mockResolvedValue(fakeSession); + mockSessionManager.spawn.mockResolvedValue(makeFakeSession()); mockSessionManager.claimPR.mockResolvedValue({ sessionId: "app-1", projectId: "my-app", @@ -808,86 +789,59 @@ describe("spawn pre-flight checks", () => { takenOverFrom: [], }); - const projects = (mockConfigRef.current as Record).projects as Record< - string, - Record - >; - projects["my-app"].tracker = { plugin: "github" }; - projects["my-app"].scm = { plugin: "github" }; - - mockExec - .mockResolvedValueOnce({ stdout: "tmux 3.3a", stderr: "" }) - .mockResolvedValueOnce({ stdout: "gh version 2.40", stderr: "" }) - .mockResolvedValueOnce({ stdout: "Logged in", stderr: "" }); - await program.parseAsync(["node", "test", "spawn", "--claim-pr", "123"]); - expect(mockExec).toHaveBeenCalledWith("tmux", ["-V"]); - const ghCalls = mockExec.mock.calls.filter(([command]) => command === "gh"); - expect(ghCalls).toHaveLength(2); - expect(mockSessionManager.spawn).toHaveBeenCalled(); - expect(mockSessionManager.claimPR).toHaveBeenCalledWith("app-1", "123", { - assignOnGithub: undefined, - }); + expect(scmPreflight).toHaveBeenCalledTimes(1); + const ctx = scmPreflight.mock.calls[0]?.[0] as { intent: { willClaimExistingPR: boolean } }; + expect(ctx.intent.willClaimExistingPR).toBe(true); }); - it("skips gh auth check when tracker is not github", async () => { - const fakeSession: Session = { - id: "app-1", - projectId: "my-app", - status: "spawning", - activity: null, - branch: null, - issueId: null, - pr: null, - workspacePath: "/tmp/wt", - runtimeHandle: { id: "hash-1", runtimeName: "tmux", data: {} }, - agentInfo: null, - createdAt: new Date(), - lastActivityAt: new Date(), - metadata: {}, - }; - mockSessionManager.spawn.mockResolvedValue(fakeSession); + it("does not iterate the tracker slot when no tracker is configured", async () => { + const trackerPreflight = vi.fn().mockResolvedValue(undefined); + mockRegistryGet.mockImplementation((slot: string) => { + if (slot === "tracker") return { name: "github", preflight: trackerPreflight }; + return null; + }); - const projects = (mockConfigRef.current as Record).projects as Record< - string, - Record - >; - projects["my-app"].tracker = { plugin: "linear" }; - - // tmux check passes — gh should never be called - mockExec.mockResolvedValue({ stdout: "tmux 3.3a", stderr: "" }); + // Project intentionally has no tracker configured. + mockSessionManager.spawn.mockResolvedValue(makeFakeSession()); await program.parseAsync(["node", "test", "spawn"]); - // Should only call tmux -V, not gh - expect(mockExec).toHaveBeenCalledWith("tmux", ["-V"]); - expect(mockExec).not.toHaveBeenCalledWith("gh", expect.anything()); - expect(mockSessionManager.spawn).toHaveBeenCalled(); + expect(trackerPreflight).not.toHaveBeenCalled(); }); - it("distinguishes gh not installed from gh not authenticated", async () => { + it("collects every plugin's preflight failure into one combined error", async () => { + const runtimePreflight = vi.fn().mockRejectedValue(new Error("tmux is not installed")); + const trackerPreflight = vi + .fn() + .mockRejectedValue(new Error("GitHub CLI is not authenticated. Run: gh auth login")); + mockRegistryGet.mockImplementation((slot: string) => { + if (slot === "runtime") return { name: "tmux", preflight: runtimePreflight }; + if (slot === "tracker") return { name: "github", preflight: trackerPreflight }; + return null; + }); + const projects = (mockConfigRef.current as Record).projects as Record< string, Record >; projects["my-app"].tracker = { plugin: "github" }; - // tmux passes, gh --version fails (not installed) - mockExec - .mockResolvedValueOnce({ stdout: "tmux 3.3a", stderr: "" }) // tmux -V - .mockRejectedValueOnce(new Error("ENOENT")); // gh --version fails + await expect(program.parseAsync(["node", "test", "spawn"])).rejects.toThrow("process.exit(1)"); - await expect(program.parseAsync(["node", "test", "spawn"])).rejects.toThrow( - "process.exit(1)", - ); + // Both preflights ran (collect-all, not fail-fast). + expect(runtimePreflight).toHaveBeenCalled(); + expect(trackerPreflight).toHaveBeenCalled(); const errors = vi .mocked(console.error) .mock.calls.map((c) => String(c[0])) .join("\n"); - expect(errors).toContain("not installed"); - expect(errors).not.toContain("not authenticated"); + expect(errors).toContain("2 preflight checks failed"); + expect(errors).toContain("tmux is not installed"); + expect(errors).toContain("gh auth login"); + expect(mockSessionManager.spawn).not.toHaveBeenCalled(); }); }); diff --git a/packages/cli/__tests__/commands/start.test.ts b/packages/cli/__tests__/commands/start.test.ts index 6eefaee4d..aec78b024 100644 --- a/packages/cli/__tests__/commands/start.test.ts +++ b/packages/cli/__tests__/commands/start.test.ts @@ -175,7 +175,6 @@ vi.mock("../../src/lib/preflight.js", () => ({ preflight: { checkPort: vi.fn(), checkBuilt: vi.fn(), - checkTmux: vi.fn().mockResolvedValue(undefined), }, })); diff --git a/packages/cli/__tests__/lib/preflight.test.ts b/packages/cli/__tests__/lib/preflight.test.ts index 92c41a31a..3593629fa 100644 --- a/packages/cli/__tests__/lib/preflight.test.ts +++ b/packages/cli/__tests__/lib/preflight.test.ts @@ -1,15 +1,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -const { mockExec, mockIsPortAvailable, mockExistsSync } = vi.hoisted(() => ({ - mockExec: vi.fn(), +const { mockIsPortAvailable, mockExistsSync } = vi.hoisted(() => ({ mockIsPortAvailable: vi.fn(), mockExistsSync: vi.fn(), })); -vi.mock("../../src/lib/shell.js", () => ({ - exec: mockExec, -})); - vi.mock("../../src/lib/web-dir.js", () => ({ isPortAvailable: mockIsPortAvailable, })); @@ -26,7 +21,6 @@ vi.mock("../../src/lib/dashboard-rebuild.js", () => ({ import { preflight } from "../../src/lib/preflight.js"; beforeEach(() => { - mockExec.mockReset(); mockIsPortAvailable.mockReset(); mockExistsSync.mockReset(); }); @@ -130,65 +124,5 @@ describe("preflight.checkBuilt", () => { }); }); -describe("preflight.checkTmux", () => { - it("passes when tmux is already installed", async () => { - mockExec.mockResolvedValue({ stdout: "tmux 3.3a", stderr: "" }); - await expect(preflight.checkTmux()).resolves.toBeUndefined(); - expect(mockExec).toHaveBeenCalledWith("tmux", ["-V"]); - }); - - it("throws with install instructions when tmux is missing", async () => { - mockExec.mockRejectedValue(new Error("ENOENT")); - const err = await preflight.checkTmux().catch((e: Error) => e); - expect(err).toBeInstanceOf(Error); - expect(err.message).toContain("tmux is not installed"); - expect(err.message).toContain("Install it:"); - expect(mockExec).toHaveBeenCalledTimes(1); - expect(mockExec).toHaveBeenCalledWith("tmux", ["-V"]); - }); -}); - -describe("preflight.checkGhAuth", () => { - it("passes when gh is installed and authenticated", async () => { - mockExec.mockResolvedValue({ stdout: "ok", stderr: "" }); - await expect(preflight.checkGhAuth()).resolves.toBeUndefined(); - expect(mockExec).toHaveBeenCalledWith("gh", ["--version"]); - expect(mockExec).toHaveBeenCalledWith("gh", ["auth", "status"]); - }); - - it("throws 'not installed' when gh is missing (ENOENT)", async () => { - mockExec.mockRejectedValue(new Error("ENOENT")); - await expect(preflight.checkGhAuth()).rejects.toThrow( - "GitHub CLI (gh) is not installed", - ); - // Should only call --version, not auth status - expect(mockExec).toHaveBeenCalledTimes(1); - expect(mockExec).toHaveBeenCalledWith("gh", ["--version"]); - }); - - it("throws 'not authenticated' when gh exists but auth fails", async () => { - mockExec - .mockResolvedValueOnce({ stdout: "gh version 2.40", stderr: "" }) // --version succeeds - .mockRejectedValueOnce(new Error("not logged in")); // auth status fails - await expect(preflight.checkGhAuth()).rejects.toThrow( - "GitHub CLI is not authenticated", - ); - expect(mockExec).toHaveBeenCalledTimes(2); - }); - - it("includes correct fix instructions for each failure", async () => { - // Not installed → install link - mockExec.mockRejectedValue(new Error("ENOENT")); - await expect(preflight.checkGhAuth()).rejects.toThrow( - "https://cli.github.com/", - ); - - mockExec.mockReset(); - - // Not authenticated → auth login - mockExec - .mockResolvedValueOnce({ stdout: "gh version 2.40", stderr: "" }) - .mockRejectedValueOnce(new Error("not logged in")); - await expect(preflight.checkGhAuth()).rejects.toThrow("gh auth login"); - }); -}); +// checkTmux + checkGhAuth moved into the runtime-tmux / tracker-github / scm-github +// plugins as their own preflight() methods. See those plugins' tests for coverage. diff --git a/packages/cli/src/commands/spawn.ts b/packages/cli/src/commands/spawn.ts index ae5d5bb68..12174f7e1 100644 --- a/packages/cli/src/commands/spawn.ts +++ b/packages/cli/src/commands/spawn.ts @@ -7,12 +7,12 @@ import { resolveSpawnTarget, TERMINAL_STATUSES, type OrchestratorConfig, + type PreflightContext, } from "@aoagents/ao-core"; import { DEFAULT_PORT } from "../lib/constants.js"; import { exec } from "../lib/shell.js"; import { banner } from "../lib/format.js"; -import { getSessionManager } from "../lib/create-session-manager.js"; -import { preflight } from "../lib/preflight.js"; +import { getPluginRegistry, getSessionManager } from "../lib/create-session-manager.js"; import { findProjectForDirectory } from "../lib/project-resolution.js"; import { getRunning } from "../lib/running-state.js"; import { projectSessionUrl } from "../lib/routes.js"; @@ -51,6 +51,49 @@ function autoDetectProject(config: OrchestratorConfig): string { ); } +/** + * Non-throwing variant — returns null when the project can't be resolved + * unambiguously. Used to feed `resolveSpawnTarget`'s fallback parameter so + * the prefix/no-prefix and issue/no-issue paths share one code path. + */ +function tryAutoDetectProject(config: OrchestratorConfig): string | null { + try { + return autoDetectProject(config); + } catch { + return null; + } +} + +/** + * Resolve the project + issue from a single optional CLI argument. + * + * Single source of truth for the four cases: + * - `ao spawn` → auto-detect project, no issue + * - `ao spawn 42` → auto-detect project, issue=42 + * - `ao spawn xid/42` → prefix match, issue=42 + * - `ao spawn x402-identity/42` → exact projectId match, issue=42 + * + * Throws (via autoDetectProject) when the project can't be resolved — the + * caller wraps in one try/catch instead of duplicating it across branches. + */ +function resolveProjectAndIssue( + config: OrchestratorConfig, + issue: string | undefined, +): { projectId: string; issueId?: string } { + const fallback = tryAutoDetectProject(config); + if (issue) { + const target = resolveSpawnTarget(config.projects, issue, fallback ?? undefined); + if (target) return { projectId: target.projectId, issueId: target.issueId }; + autoDetectProject(config); // throws with the real error message + throw new Error("unreachable"); + } + if (!fallback) { + autoDetectProject(config); // throws + throw new Error("unreachable"); + } + return { projectId: fallback }; +} + interface SpawnClaimOptions { claimPr?: string; assignOnGithub?: boolean; @@ -84,8 +127,16 @@ async function warnIfAONotRunning(projectId: string): Promise { /** * Run pre-flight checks for a project once, before any sessions are spawned. - * Validates runtime and tracker prerequisites so failures surface immediately - * rather than repeating per-session in a batch. + * + * Iterates the plugins selected for this spawn and calls each one's optional + * `preflight()`. Plugins own their own prerequisites (binary present, auth + * configured, etc.) so this CLI helper does not need to know which plugin + * needs which tool. Adding a new runtime/tracker/scm plugin only requires + * the plugin to declare its own preflight — no edits here. + * + * Collects every plugin's failure rather than aborting at the first one, so a + * user with multiple broken prerequisites (e.g. tmux missing AND gh logged + * out) sees both errors in a single run instead of fixing one and re-invoking. */ async function runSpawnPreflight( config: OrchestratorConfig, @@ -93,15 +144,54 @@ async function runSpawnPreflight( options?: SpawnClaimOptions, ): Promise { const project = config.projects[projectId]; - const runtime = project?.runtime ?? config.defaults.runtime; - if (runtime === "tmux") { - await preflight.checkTmux(); + if (!project) return; + + const ctx: PreflightContext = { + project, + intent: { + role: "worker", + willClaimExistingPR: !!options?.claimPr, + }, + }; + + const registry = await getPluginRegistry(config); + // DefaultPluginsSchema (config.ts) defaults runtime/agent/workspace via + // .default(), so these are guaranteed strings — no literal fallback needed. + const runtimeName = project.runtime ?? config.defaults.runtime; + const agentName = project.agent ?? config.defaults.agent; + const workspaceName = project.workspace ?? config.defaults.workspace; + const trackerName = project.tracker?.plugin; + const scmName = project.scm?.plugin; + + // Only iterate plugins that the spawn will actually exercise. SCM is + // skipped unless the user passed --claim-pr; otherwise an unconfigured + // gh auth would block spawns that don't touch PRs. + const candidates: Array = [ + registry.get("runtime", runtimeName), + registry.get("agent", agentName), + registry.get("workspace", workspaceName), + trackerName ? registry.get("tracker", trackerName) : null, + options?.claimPr && scmName ? registry.get("scm", scmName) : null, + ]; + + const errors: Error[] = []; + for (const plugin of candidates) { + const preflight = (plugin as { preflight?: (ctx: PreflightContext) => Promise } | null) + ?.preflight; + if (!preflight) continue; + try { + await preflight.call(plugin, ctx); + } catch (err) { + errors.push(err instanceof Error ? err : new Error(String(err))); + } } - const needsGitHubAuth = - project?.tracker?.plugin === "github" || - (options?.claimPr && project?.scm?.plugin === "github"); - if (needsGitHubAuth) { - await preflight.checkGhAuth(); + + if (errors.length === 1) throw errors[0]; + if (errors.length > 1) { + throw new Error( + `${errors.length} preflight checks failed:\n` + + errors.map((e, i) => ` ${i + 1}. ${e.message}`).join("\n"), + ); } } @@ -113,7 +203,7 @@ async function spawnSession( agent?: string, claimOptions?: SpawnClaimOptions, prompt?: string, -): Promise { +): Promise { const spinner = ora("Creating session").start(); try { @@ -181,7 +271,6 @@ async function spawnSession( // Output for scripting console.log(`SESSION=${session.id}`); - return session.id; } catch (err) { spinner.fail("Failed to create or initialize session"); throw err; @@ -228,29 +317,11 @@ export function registerSpawn(program: Command): void { const config = loadConfig(); let projectId: string; let issueId: string | undefined; - - if (issue) { - const prefixed = resolveSpawnTarget(config.projects, issue); - if (prefixed) { - projectId = prefixed.projectId; - issueId = prefixed.issueId; - } else { - issueId = issue; - try { - projectId = autoDetectProject(config); - } catch (err) { - console.error(chalk.red(err instanceof Error ? err.message : String(err))); - process.exit(1); - } - } - } else { - // No args: auto-detect project, no issue - try { - projectId = autoDetectProject(config); - } catch (err) { - console.error(chalk.red(err instanceof Error ? err.message : String(err))); - process.exit(1); - } + try { + ({ projectId, issueId } = resolveProjectAndIssue(config, issue)); + } catch (err) { + console.error(chalk.red(err instanceof Error ? err.message : String(err))); + process.exit(1); } if (!opts.claimPr && opts.assignOnGithub) { diff --git a/packages/cli/src/lib/preflight.ts b/packages/cli/src/lib/preflight.ts index 717cbec1d..b2770bf09 100644 --- a/packages/cli/src/lib/preflight.ts +++ b/packages/cli/src/lib/preflight.ts @@ -1,8 +1,9 @@ /** - * Pre-flight checks for `ao start` and `ao spawn`. + * Pre-flight checks for `ao start` (port + dashboard build artifacts). * - * Validates runtime prerequisites before entering the main command flow, - * giving clear errors instead of cryptic failures. + * Tool/auth checks for `ao spawn` live on each plugin's `preflight()` method. + * Adding a new runtime/tracker/scm therefore doesn't require editing this + * file — the plugin declares its own prereqs. * * All checks throw on failure so callers can catch and handle uniformly. */ @@ -10,7 +11,6 @@ import { existsSync } from "node:fs"; import { resolve, dirname } from "node:path"; import { isPortAvailable } from "./web-dir.js"; -import { exec } from "./shell.js"; import { isInstalledUnderNodeModules } from "./dashboard-rebuild.js"; /** @@ -76,49 +76,7 @@ function findPackageUp(startDir: string, ...segments: string[]): string | null { return null; } -/** - * Check that tmux is installed. - * Throws with platform-appropriate manual install instructions when missing. - */ -async function checkTmux(): Promise { - try { - await exec("tmux", ["-V"]); - return; - } catch { - // tmux not found - } - - const hint = - process.platform === "darwin" - ? "brew install tmux" - : process.platform === "win32" - ? "tmux is not available on Windows. Use WSL: wsl --install, then: sudo apt install tmux" - : "sudo apt install tmux (Debian/Ubuntu) or sudo dnf install tmux (Fedora)"; - throw new Error(`tmux is not installed. Install it: ${hint}`); -} - -/** - * Check that the GitHub CLI is installed and authenticated. - * Distinguishes between "not installed" and "not authenticated" - * so the user gets the right troubleshooting guidance. - */ -async function checkGhAuth(): Promise { - try { - await exec("gh", ["--version"]); - } catch { - throw new Error("GitHub CLI (gh) is not installed. Install it: https://cli.github.com/"); - } - - try { - await exec("gh", ["auth", "status"]); - } catch { - throw new Error("GitHub CLI is not authenticated. Run: gh auth login"); - } -} - export const preflight = { checkPort, checkBuilt, - checkTmux, - checkGhAuth, }; diff --git a/packages/core/src/__tests__/process-cache.test.ts b/packages/core/src/__tests__/process-cache.test.ts new file mode 100644 index 000000000..6dc936084 --- /dev/null +++ b/packages/core/src/__tests__/process-cache.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { memoizeAsync, _clearProcessCacheForTests } from "../process-cache.js"; + +beforeEach(() => { + _clearProcessCacheForTests(); +}); + +describe("memoizeAsync", () => { + it("runs the underlying fn only once for a given key", async () => { + const fn = vi.fn().mockResolvedValue("ok"); + + await memoizeAsync("k", fn); + await memoizeAsync("k", fn); + await memoizeAsync("k", fn); + + expect(fn).toHaveBeenCalledTimes(1); + }); + + it("returns the same resolved value across calls", async () => { + const fn = vi.fn().mockResolvedValue({ value: 42 }); + + const a = await memoizeAsync("k", fn); + const b = await memoizeAsync("k", fn); + + expect(a).toBe(b); + }); + + it("uses different keys for different work", async () => { + const fnA = vi.fn().mockResolvedValue("a"); + const fnB = vi.fn().mockResolvedValue("b"); + + expect(await memoizeAsync("a", fnA)).toBe("a"); + expect(await memoizeAsync("b", fnB)).toBe("b"); + expect(fnA).toHaveBeenCalledTimes(1); + expect(fnB).toHaveBeenCalledTimes(1); + }); + + it("caches rejections too — failed checks don't re-run within a process", async () => { + const fn = vi.fn().mockRejectedValue(new Error("boom")); + + await expect(memoizeAsync("k", fn)).rejects.toThrow("boom"); + await expect(memoizeAsync("k", fn)).rejects.toThrow("boom"); + + expect(fn).toHaveBeenCalledTimes(1); + }); + + it("returns the in-flight promise to concurrent callers (no double-fire)", async () => { + let resolveFn: (v: string) => void = () => {}; + const fn = vi.fn( + () => + new Promise((resolve) => { + resolveFn = resolve; + }), + ); + + const a = memoizeAsync("k", fn); + const b = memoizeAsync("k", fn); + + expect(fn).toHaveBeenCalledTimes(1); + + resolveFn("done"); + expect(await a).toBe("done"); + expect(await b).toBe("done"); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 593b58523..fd391ed52 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -121,6 +121,10 @@ export { export { createSessionManager } from "./session-manager.js"; export type { SessionManagerDeps } from "./session-manager.js"; +// Process-scoped async memoization — used by plugins to dedupe shared +// prerequisite checks (e.g. multiple github plugins checking gh auth). +export { memoizeAsync, _clearProcessCacheForTests } from "./process-cache.js"; + // Lifecycle manager — state machine + reaction engine export { createLifecycleManager } from "./lifecycle-manager.js"; export type { LifecycleManagerDeps } from "./lifecycle-manager.js"; diff --git a/packages/core/src/process-cache.ts b/packages/core/src/process-cache.ts new file mode 100644 index 000000000..cf1b27f25 --- /dev/null +++ b/packages/core/src/process-cache.ts @@ -0,0 +1,44 @@ +/** + * Process-scoped async memoization for expensive checks shared across plugins. + * + * Use cases: prerequisite checks (binary present, auth valid) that multiple + * plugins want to perform but only need to actually run once per CLI + * invocation. Cache key chooses the dedup boundary — plugins that share a + * key share the result. + * + * Both successes and failures are cached: if a check fails the user must fix + * the underlying issue and re-run, so re-checking within the same process is + * pointless and would muddy the error stream with duplicate messages. + * + * **Key namespacing convention:** + * + * The cache is shared across every caller in the process, so two plugins + * passing the same key are explicitly opting into shared state. That is the + * intended use for cross-cutting checks like the `gh` CLI auth status (used + * by both `tracker-github` and `scm-github`). + * + * For plugin-internal caching (where you do *not* want sharing), namespace + * the key with your plugin name to avoid silent collisions: + * - shared cross-plugin check: `"gh-cli-auth"` (intentional sharing) + * - plugin-internal check: `"tracker-github:rate-limit-check"` + * + * If two plugins use the same key for semantically different work, callers + * will silently receive each other's resolved values — a debugging nightmare. + * When in doubt, namespace. + */ + +const cache = new Map>(); + +export function memoizeAsync(key: string, fn: () => Promise): Promise { + let cached = cache.get(key); + if (!cached) { + cached = fn(); + cache.set(key, cached); + } + return cached as Promise; +} + +/** Test-only — clears the process cache. */ +export function _clearProcessCacheForTests(): void { + cache.clear(); +} diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index b7eac5aea..016856407 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -409,6 +409,13 @@ export interface Runtime { /** Get info needed to attach a human to this session (for Terminal plugin) */ getAttachInfo?(handle: RuntimeHandle): Promise; + + /** + * Optional: validate that this runtime's prerequisites are present before + * it is exercised by `ao spawn`. Throw with an actionable, human-readable + * message; the CLI catches and formats the error. + */ + preflight?(context: PreflightContext): Promise; } export interface RuntimeCreateConfig { @@ -540,6 +547,12 @@ export interface Agent { * `getActivityState` already reads richer data from the agent's own session files. */ recordActivity?(session: Session, terminalOutput: string): Promise; + + /** + * Optional: validate that this agent's prerequisites are present before + * it is exercised by `ao spawn`. Throw with an actionable error message. + */ + preflight?(context: PreflightContext): Promise; } export interface AgentLaunchConfig { @@ -640,6 +653,12 @@ export interface Workspace { /** Optional: restore a workspace (e.g. recreate a worktree for an existing branch) */ restore?(config: WorkspaceCreateConfig, workspacePath: string): Promise; + + /** + * Optional: validate that this workspace's prerequisites (e.g. git in PATH, + * write access to the worktree root) are present before `ao spawn`. + */ + preflight?(context: PreflightContext): Promise; } export interface WorkspaceCreateConfig { @@ -694,6 +713,13 @@ export interface Tracker { /** Optional: create a new issue */ createIssue?(input: CreateIssueInput, project: ProjectConfig): Promise; + + /** + * Optional: validate that this tracker's prerequisites (auth tokens, CLI + * tools) are present before `ao spawn` runs. Throw with an actionable + * error message. + */ + preflight?(context: PreflightContext): Promise; } export interface Issue { @@ -834,6 +860,14 @@ export interface SCM { * @returns Map keyed by "${owner}/${repo}#${number}" containing enrichment data */ enrichSessionsPRBatch?(prs: PRInfo[], observer?: BatchObserver, repos?: string[]): Promise>; + + /** + * Optional: validate that this SCM's prerequisites (auth, CLI tools) are + * present before `ao spawn` runs. Plugins should consult + * `context.intent.willClaimExistingPR` and skip PR-write prereqs when the + * spawn won't exercise them. + */ + preflight?(context: PreflightContext): Promise; } /** @@ -1664,6 +1698,32 @@ export interface PluginModule { detect?(): boolean; } +/** + * Context passed to a plugin's `preflight()` method. + * + * Describes the **intent** of the operation (what it will do), not the CLI + * flags that triggered it. Plugins should never know about specific flag + * names — translate flags into intent at the CLI boundary so adding a new + * flag doesn't ripple into every plugin that cares about a related operation. + */ +export interface PreflightContext { + /** The project the operation runs against. */ + project: ProjectConfig; + + /** What the operation will do. Plugins decide whether their prereqs apply. */ + intent: { + /** Whether the spawn is for a worker session or the orchestrator. */ + role: "worker" | "orchestrator"; + + /** + * Whether the operation will exercise SCM PR-write paths + * (e.g. claiming an existing PR for the new session). When false, an SCM + * plugin's preflight can skip PR-write prereqs. + */ + willClaimExistingPR: boolean; + }; +} + // ============================================================================= // SESSION METADATA // ============================================================================= diff --git a/packages/plugins/runtime-tmux/src/__tests__/index.test.ts b/packages/plugins/runtime-tmux/src/__tests__/index.test.ts index b4f37b2b2..092ea98e2 100644 --- a/packages/plugins/runtime-tmux/src/__tests__/index.test.ts +++ b/packages/plugins/runtime-tmux/src/__tests__/index.test.ts @@ -578,3 +578,25 @@ describe("runtime.getAttachInfo()", () => { }); }); }); + +describe("runtime.preflight()", () => { + it("resolves when `tmux -V` succeeds", async () => { + mockTmuxSuccess("tmux 3.4"); + const runtime = create(); + await expect(runtime.preflight!({} as never)).resolves.toBeUndefined(); + expect(mockExecFileCustom).toHaveBeenCalledWith("tmux", ["-V"], expectedTmuxOptions); + }); + + it("throws with platform-specific install hint when tmux is missing", async () => { + mockTmuxError("ENOENT"); + const runtime = create(); + const err = (await runtime.preflight!({} as never).catch((e: unknown) => e)) as Error; + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain("tmux is not installed"); + expect(err.message).toContain("Install it:"); + // Hint must include something runnable for the host platform. + if (process.platform === "darwin") expect(err.message).toContain("brew install tmux"); + else if (process.platform === "win32") expect(err.message).toContain("WSL"); + else expect(err.message).toMatch(/apt install tmux|dnf install tmux/); + }); +}); diff --git a/packages/plugins/runtime-tmux/src/index.ts b/packages/plugins/runtime-tmux/src/index.ts index c296c6ba9..547e594dc 100644 --- a/packages/plugins/runtime-tmux/src/index.ts +++ b/packages/plugins/runtime-tmux/src/index.ts @@ -190,6 +190,20 @@ export function create(): Runtime { command: `tmux attach -t ${handle.id}`, }; }, + + async preflight(): Promise { + try { + await execFileAsync("tmux", ["-V"], { timeout: TMUX_COMMAND_TIMEOUT_MS }); + } catch { + const hint = + process.platform === "darwin" + ? "brew install tmux" + : process.platform === "win32" + ? "tmux is not available on Windows. Use WSL: wsl --install, then: sudo apt install tmux" + : "sudo apt install tmux (Debian/Ubuntu) or sudo dnf install tmux (Fedora)"; + throw new Error(`tmux is not installed. Install it: ${hint}`); + } + }, }; } diff --git a/packages/plugins/scm-github/src/index.ts b/packages/plugins/scm-github/src/index.ts index 05baa166c..5c2ed2f90 100644 --- a/packages/plugins/scm-github/src/index.ts +++ b/packages/plugins/scm-github/src/index.ts @@ -10,7 +10,9 @@ import { promisify } from "node:util"; import { CI_STATUS, execGhObserved, + memoizeAsync, type PluginModule, + type PreflightContext, type SCM, type SCMWebhookEvent, type SCMWebhookRequest, @@ -1251,6 +1253,28 @@ function createGitHubSCM(): SCM { const batchResult = await enrichSessionsPRBatchImpl(prs, observer, repos); return batchResult.enrichment; }, + + async preflight(context: PreflightContext): Promise { + // SCM is only exercised at spawn time when --claim-pr is set. Skip the + // gh-auth check otherwise so spawns that don't touch PRs don't require + // gh credentials. Lifecycle polling has its own auth handling. + if (!context.intent.willClaimExistingPR) return; + // Memoize across plugins: shares the "gh-cli-auth" cache key with + // tracker-github so spawns that touch both only run gh --version + gh + // auth status once total, not twice. + await memoizeAsync("gh-cli-auth", async () => { + try { + await execFileAsync("gh", ["--version"]); + } catch { + throw new Error("GitHub CLI (gh) is not installed. Install it: https://cli.github.com/"); + } + try { + await execFileAsync("gh", ["auth", "status"]); + } catch { + throw new Error("GitHub CLI is not authenticated. Run: gh auth login"); + } + }); + }, }; } diff --git a/packages/plugins/scm-github/test/index.test.ts b/packages/plugins/scm-github/test/index.test.ts index cb0e0b6be..8f6b2d1f5 100644 --- a/packages/plugins/scm-github/test/index.test.ts +++ b/packages/plugins/scm-github/test/index.test.ts @@ -15,7 +15,7 @@ vi.mock("node:child_process", () => { }); import { create, manifest } from "../src/index.js"; -import { createActivitySignal, type PRInfo, type SCMWebhookRequest, type Session, type ProjectConfig } from "@aoagents/ao-core"; +import { _clearProcessCacheForTests, createActivitySignal, type PreflightContext, type PRInfo, type SCMWebhookRequest, type Session, type ProjectConfig } from "@aoagents/ao-core"; // --------------------------------------------------------------------------- // Fixtures @@ -1427,6 +1427,41 @@ describe("scm-github plugin", () => { expect(ghMock).toHaveBeenCalledTimes(1); }); }); + + describe("preflight", () => { + function ctxWith(willClaim: boolean): PreflightContext { + return { project, intent: { role: "worker", willClaimExistingPR: willClaim } }; + } + + beforeEach(() => { + _clearProcessCacheForTests(); + }); + + it("is a no-op when willClaimExistingPR is false (no gh calls made)", async () => { + await expect(scm.preflight!(ctxWith(false))).resolves.toBeUndefined(); + expect(ghMock).not.toHaveBeenCalled(); + }); + + it("checks gh installed + authenticated when willClaimExistingPR is true", async () => { + mockGhRaw("gh version 2.40.0"); + mockGhRaw("Logged in to github.com as alice"); + await expect(scm.preflight!(ctxWith(true))).resolves.toBeUndefined(); + expect(ghMock).toHaveBeenCalledTimes(2); + }); + + it("throws 'not installed' when `gh --version` fails", async () => { + mockGhError("ENOENT"); + const err = (await scm.preflight!(ctxWith(true)).catch((e: unknown) => e)) as Error; + expect(err.message).toContain("GitHub CLI (gh) is not installed"); + }); + + it("throws 'not authenticated' when `gh auth status` fails", async () => { + mockGhRaw("gh version 2.40.0"); + mockGhError("not logged in"); + const err = (await scm.preflight!(ctxWith(true)).catch((e: unknown) => e)) as Error; + expect(err.message).toContain("not authenticated"); + }); + }); }); function mockGhRaw(stdout: string) { diff --git a/packages/plugins/tracker-github/src/index.ts b/packages/plugins/tracker-github/src/index.ts index 8bb0ef4d9..272b7d78e 100644 --- a/packages/plugins/tracker-github/src/index.ts +++ b/packages/plugins/tracker-github/src/index.ts @@ -6,6 +6,7 @@ import { execGhObserved, + memoizeAsync, type PluginModule, type Tracker, type Issue, @@ -29,6 +30,26 @@ async function gh(args: string[]): Promise { } } +/** + * Process-scoped gh auth check shared with scm-github via the same cache key + * (`gh-cli-auth`). Both plugins call into this — the second caller hits the + * cached promise and adds zero subprocess overhead. + */ +async function checkGhCliAuth(): Promise { + return memoizeAsync("gh-cli-auth", async () => { + try { + await gh(["--version"]); + } catch { + throw new Error("GitHub CLI (gh) is not installed. Install it: https://cli.github.com/"); + } + try { + await gh(["auth", "status"]); + } catch { + throw new Error("GitHub CLI is not authenticated. Run: gh auth login"); + } + }); +} + function getErrorText(err: unknown): string { if (!(err instanceof Error)) return ""; @@ -415,6 +436,15 @@ function createGitHubTracker(): Tracker { return tracker.getIssue(number, project); }, + + async preflight(): Promise { + // Memoize across plugins: tracker-github and scm-github both check the + // same gh CLI / auth state. Sharing key "gh-cli-auth" via process-cache + // means both plugins' preflights resolve to the same in-flight check + // (or cached result) — halving execs on the happy path and giving one + // error message instead of two on the failure path. + await checkGhCliAuth(); + }, }; return tracker; diff --git a/packages/plugins/tracker-github/test/index.test.ts b/packages/plugins/tracker-github/test/index.test.ts index a838bec6e..7ecd39a1b 100644 --- a/packages/plugins/tracker-github/test/index.test.ts +++ b/packages/plugins/tracker-github/test/index.test.ts @@ -13,7 +13,11 @@ vi.mock("node:child_process", () => { }); import { create, manifest } from "../src/index.js"; -import type { ProjectConfig } from "@aoagents/ao-core"; +import { + _clearProcessCacheForTests, + type PreflightContext, + type ProjectConfig, +} from "@aoagents/ao-core"; // --------------------------------------------------------------------------- // Fixtures @@ -565,4 +569,39 @@ describe("tracker-github plugin", () => { ).rejects.toThrow("Failed to parse issue URL"); }); }); + + describe("preflight", () => { + const ctx: PreflightContext = { + project, + intent: { role: "worker", willClaimExistingPR: false }, + }; + + beforeEach(() => { + // Process cache spans tests; clear so each one starts fresh. + _clearProcessCacheForTests(); + }); + + it("resolves when gh is installed and authenticated", async () => { + mockGhRaw("gh version 2.40.0"); // gh --version + mockGhRaw("Logged in to github.com as alice"); // gh auth status + await expect(tracker.preflight!(ctx)).resolves.toBeUndefined(); + }); + + it("throws 'not installed' when `gh --version` fails", async () => { + mockGhError("ENOENT"); + const err = (await tracker.preflight!(ctx).catch((e: unknown) => e)) as Error; + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain("GitHub CLI (gh) is not installed"); + expect(err.message).toContain("https://cli.github.com/"); + }); + + it("throws 'not authenticated' when `gh auth status` fails", async () => { + mockGhRaw("gh version 2.40.0"); + mockGhError("not logged in"); + const err = (await tracker.preflight!(ctx).catch((e: unknown) => e)) as Error; + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain("not authenticated"); + expect(err.message).toContain("gh auth login"); + }); + }); });