diff --git a/src/commands/init.ts b/src/commands/init.ts index bffbc6d23..af0bca492 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -1,3 +1,6 @@ +import { execSync } from "node:child_process"; +import path from "node:path"; + import { Flags } from "@oclif/core"; import chalk from "chalk"; @@ -10,10 +13,18 @@ import { import { TARGET_CONFIGS } from "../services/skills-installer.js"; import { resolveSkillsTargets } from "../services/skills-target-prompt.js"; import { BaseFlags } from "../types/cli.js"; +import { extractErrorInfo } from "../utils/errors.js"; import { displayLogo } from "../utils/logo.js"; import { formatHeading, formatResource } from "../utils/output.js"; +import { promptForConfirmation } from "../utils/prompt-confirmation.js"; import isTestMode from "../utils/test-mode.js"; +// Bound on the global install step so a hung npm registry can't leave the +// onboarding command stuck with no feedback. On timeout execSync throws +// ETIMEDOUT, which the catch block in maybeInstallGlobally turns into the +// usual non-fatal warning (and JSON failure event). +const GLOBAL_INSTALL_TIMEOUT_MS = 120_000; + export default class Init extends AblyBaseCommand { static override description = "Set up Ably for AI-powered development — authenticate and install Agent Skills"; @@ -23,6 +34,7 @@ export default class Init extends AblyBaseCommand { "<%= config.bin %> <%= command.id %> --target claude-code", "<%= config.bin %> <%= command.id %> --target cursor --target windsurf", "<%= config.bin %> <%= command.id %> --target auto", + "<%= config.bin %> <%= command.id %> --no-install", "<%= config.bin %> <%= command.id %> --json", ]; @@ -35,6 +47,11 @@ export default class Init extends AblyBaseCommand { default: ["auto"], description: "Target IDE(s) to install skills for", }), + "no-install": Flags.boolean({ + default: false, + description: + "Skip installing @ably/cli globally (only relevant when launched via npx)", + }), }; async run(): Promise { @@ -55,6 +72,8 @@ export default class Init extends AblyBaseCommand { displayLogo(this.log.bind(this)); } + await this.maybeInstallGlobally(flags); + await this.runAuth(flags); const resolvedTargets = await resolveSkillsTargets({ @@ -183,4 +202,146 @@ export default class Init extends AblyBaseCommand { if (process.env.ABLY_ACCESS_TOKEN) return true; return Boolean(this.configManager.getAccessToken()); } + + // When invoked via `npx @ably/cli init`, the running binary lives in an + // ephemeral npx cache that is not on PATH. Without a global install the user + // can't run `ably` again after init exits — defeating the "one-command + // onboarding" promise. Detect that situation and offer to install globally. + private async maybeInstallGlobally(flags: BaseFlags): Promise { + const jsonMode = this.shouldOutputJson(flags); + + // Order matters: --no-install is only meaningful when we would otherwise + // install (i.e. when running via npx). Checking the npx context first + // means a normal `ably init --no-install` from a globally installed + // binary reports the accurate reason ("not-npx") rather than the + // irrelevant flag. + if (!this.isRunningFromNpx()) { + this.emitInstallEvent(flags, { status: "skipped", reason: "not-npx" }); + return; + } + if (flags["no-install"]) { + this.emitInstallEvent(flags, { + status: "skipped", + reason: "no-install-flag", + }); + return; + } + + if (!jsonMode) { + const confirmed = await this.confirmGlobalInstall(); + if (!confirmed) { + this.logWarning( + "Skipping global install. To install later, Run: npm install -g @ably/cli", + flags, + ); + return; + } + } + + this.logProgress("Installing @ably/cli globally", flags); + try { + await this.runGlobalInstall(jsonMode); + this.logSuccessMessage("Installed @ably/cli globally.", flags); + this.emitInstallEvent(flags, { + status: "installed", + package: "@ably/cli@latest", + }); + } catch (error) { + this.emitInstallEvent(flags, { + status: "failed", + package: "@ably/cli@latest", + error: extractErrorInfo(error), + }); + if (jsonMode) { + // npm output was piped, so the thrown error already carries the + // captured stderr — surface it so agents see why install failed. + const detail = error instanceof Error ? error.message : String(error); + this.logWarning( + `Could not install @ably/cli globally automatically (${detail}). Run: npm install -g @ably/cli`, + flags, + ); + } else { + // npm output was inherited, so npm has already printed the real error + // to the user's terminal. error.message is just "Command failed: ..." + // which adds no information — keep the warning terse. + this.logWarning( + "Could not install @ably/cli globally. Run: npm install -g @ably/cli", + flags, + ); + } + } + } + + private emitInstallEvent( + flags: BaseFlags, + install: Record, + ): void { + if (!this.shouldOutputJson(flags)) return; + this.logJsonEvent({ install }, flags); + } + + private async confirmGlobalInstall(): Promise { + if (isTestMode()) { + const hook = globalThis.__TEST_MOCKS__?.confirmGlobalInstall; + if (typeof hook === "boolean") return hook; + } + return promptForConfirmation( + "Install @ably/cli globally so you can run 'ably' from any shell?", + { defaultYes: true }, + ); + } + + private isRunningFromNpx(): boolean { + if (isTestMode()) { + const hook = globalThis.__TEST_MOCKS__?.isRunningFromNpx; + if (typeof hook === "boolean") return hook; + } + const entry = process.argv[1] ?? ""; + return entry.includes(`${path.sep}_npx${path.sep}`); + } + + // Test hook: when the unit tests set globalThis.__TEST_MOCKS__.installGlobally + // to a recording or throwing function, use that instead of shelling out to + // `npm install -g` — which would mutate the developer's machine and require + // network access during unit tests. + // + // In JSON mode we pipe npm's output instead of inheriting so the agent's + // NDJSON stream isn't polluted with "added N packages" / deprecation + // warnings. On failure we re-throw with the captured stderr appended so the + // caller's warning still surfaces the root cause. + private async runGlobalInstall(jsonMode: boolean): Promise { + if (isTestMode()) { + const hook = globalThis.__TEST_MOCKS__?.installGlobally as + | ((pkg: string) => Promise) + | undefined; + if (hook) { + await hook("@ably/cli@latest"); + return; + } + } + if (jsonMode) { + try { + execSync("npm install -g @ably/cli@latest", { + stdio: "pipe", + timeout: GLOBAL_INSTALL_TIMEOUT_MS, + }); + } catch (error) { + const stderr = ( + error as { stderr?: Buffer | string } | undefined + )?.stderr + ?.toString() + .trim(); + const baseMessage = + error instanceof Error ? error.message : String(error); + throw new Error(stderr ? `${baseMessage}: ${stderr}` : baseMessage, { + cause: error, + }); + } + return; + } + execSync("npm install -g @ably/cli@latest", { + stdio: "inherit", + timeout: GLOBAL_INSTALL_TIMEOUT_MS, + }); + } } diff --git a/src/utils/prompt-confirmation.ts b/src/utils/prompt-confirmation.ts index 15d80f6c0..eb61131a5 100644 --- a/src/utils/prompt-confirmation.ts +++ b/src/utils/prompt-confirmation.ts @@ -2,30 +2,39 @@ import * as readline from "node:readline"; /** * Prompts the user for confirmation with a yes/no question. - * Automatically appends " [y/n]" to the message if not already present. * Accepts both "y" and "yes" as affirmative responses (case-insensitive). * * @param message - The confirmation message to display to the user - * @returns Promise - true if user confirms (y/yes), false otherwise + * @param options.defaultYes - If true, an empty answer counts as yes and the + * default suffix becomes " [Y/n]". Use only for non-destructive prompts. + * @returns Promise - true if user confirms, false otherwise */ -export function promptForConfirmation(message: string): Promise { +export function promptForConfirmation( + message: string, + options: { defaultYes?: boolean } = {}, +): Promise { const rl = readline.createInterface({ input: process.stdin, output: process.stdout, }); - // Add " [y/n]" suffix if not already present + const suffix = options.defaultYes ? "[Y/n]" : "[y/n]"; const promptMessage = message.includes("[yes/no]") || message.includes("[y/n]") || + message.includes("[Y/n]") || message.includes("[Y/N]") ? message - : `${message} [y/n]`; + : `${message} ${suffix}`; return new Promise((resolve) => { rl.question(promptMessage, (answer) => { rl.close(); const response = answer.toLowerCase().trim(); + if (response === "") { + resolve(Boolean(options.defaultYes)); + return; + } resolve(response === "y" || response === "yes"); }); }); diff --git a/test/unit/commands/init.test.ts b/test/unit/commands/init.test.ts index 49aa5862b..d1fd53b7e 100644 --- a/test/unit/commands/init.test.ts +++ b/test/unit/commands/init.test.ts @@ -147,6 +147,22 @@ function mockFetchWithTarball(buffer: Buffer): void { }); } +function findInstallEvent(stdout: string): Record | undefined { + for (const line of stdout.split("\n")) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const parsed = JSON.parse(trimmed) as Record; + if (parsed.install) return parsed; + } catch { + // Not JSON — ignore. NDJSON streams may have non-JSON lines + // (logo, prompts) in non-JSON mode, but in --json mode all + // output should be JSON. + } + } + return undefined; +} + describe("init command", () => { let tempDir: string; let originalCwd: string; @@ -217,13 +233,23 @@ describe("init command", () => { delete (globalThis.__TEST_MOCKS__ as Record).runLogin; delete (globalThis.__TEST_MOCKS__ as Record) .verifyAttestation; + delete (globalThis.__TEST_MOCKS__ as Record) + .isRunningFromNpx; + delete (globalThis.__TEST_MOCKS__ as Record) + .confirmGlobalInstall; + delete (globalThis.__TEST_MOCKS__ as Record) + .installGlobally; } vi.restoreAllMocks(); }); standardHelpTests("init", import.meta.url); standardArgValidationTests("init", import.meta.url); - standardFlagTests("init", import.meta.url, ["--target", "--json"]); + standardFlagTests("init", import.meta.url, [ + "--target", + "--json", + "--no-install", + ]); describe("functionality", () => { it("should install skills to the requested target when already authenticated", async () => { @@ -495,4 +521,273 @@ describe("init command", () => { ).toBe(true); }); }); + + describe("global install (npx onboarding)", () => { + // The default test environment looks "not from npx" — these tests have to + // opt in via __TEST_MOCKS__.isRunningFromNpx to exercise the install path. + it("should not attempt a global install when not launched via npx", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + + const installCalls: string[] = []; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async (pkg: string) => { + installCalls.push(pkg); + }; + + const { error } = await runCommand( + ["init", "--target", "cursor"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + expect(installCalls).toEqual([]); + }); + + it("should not attempt a global install when --no-install is passed (even from npx)", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + (globalThis.__TEST_MOCKS__ as Record).isRunningFromNpx = + true; + + const installCalls: string[] = []; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async (pkg: string) => { + installCalls.push(pkg); + }; + + const { error } = await runCommand( + ["init", "--target", "cursor", "--no-install"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + expect(installCalls).toEqual([]); + }); + + it("should install globally in JSON mode without prompting", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + (globalThis.__TEST_MOCKS__ as Record).isRunningFromNpx = + true; + + const installCalls: string[] = []; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async (pkg: string) => { + installCalls.push(pkg); + }; + + const { error } = await runCommand( + ["init", "--target", "cursor", "--json"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + expect(installCalls).toEqual(["@ably/cli@latest"]); + }); + + it("should install globally in interactive mode when the user confirms", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + (globalThis.__TEST_MOCKS__ as Record).isRunningFromNpx = + true; + ( + globalThis.__TEST_MOCKS__ as Record + ).confirmGlobalInstall = true; + + const installCalls: string[] = []; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async (pkg: string) => { + installCalls.push(pkg); + }; + + const { stderr, error } = await runCommand( + ["init", "--target", "cursor"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + expect(installCalls).toEqual(["@ably/cli@latest"]); + expect(stderr).toMatch(/Installed @ably\/cli globally/); + }); + + it("should skip install and warn when the user declines the prompt", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + (globalThis.__TEST_MOCKS__ as Record).isRunningFromNpx = + true; + ( + globalThis.__TEST_MOCKS__ as Record + ).confirmGlobalInstall = false; + + const installCalls: string[] = []; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async (pkg: string) => { + installCalls.push(pkg); + }; + + const { stderr, error } = await runCommand( + ["init", "--target", "cursor"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + expect(installCalls).toEqual([]); + expect(stderr).toMatch(/Skipping global install/); + }); + + it("should warn rather than fail when the global install command errors (non-JSON)", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + (globalThis.__TEST_MOCKS__ as Record).isRunningFromNpx = + true; + ( + globalThis.__TEST_MOCKS__ as Record + ).confirmGlobalInstall = true; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async () => { + throw new Error("EACCES: permission denied"); + }; + + const { stderr, stdout, error } = await runCommand( + ["init", "--target", "cursor"], + import.meta.url, + ); + + // Install failure must not be fatal — the rest of init (auth, skills) + // is still useful, and the user can run `npm install -g` themselves. + expect(error).toBeUndefined(); + // Non-JSON mode: terse warning. npm would have printed the real error + // to inherited stderr in production, so we don't restate it. + expect(stderr).toMatch( + /Could not install @ably\/cli globally\. Run: npm install -g @ably\/cli/, + ); + // Specifically should NOT include the raw error detail in non-JSON mode. + expect(stderr).not.toMatch(/EACCES/); + expect(stdout).not.toMatch(/EACCES/); + }); + + it("should surface captured stderr in the JSON-mode failure warning", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + (globalThis.__TEST_MOCKS__ as Record).isRunningFromNpx = + true; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async () => { + throw new Error("EACCES: permission denied at /usr/local/lib"); + }; + + const { stdout, error } = await runCommand( + ["init", "--target", "cursor", "--json"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + // JSON mode emits the warning as a structured NDJSON line via + // logWarning. The captured stderr (the thrown error's message in the + // test hook case) should be embedded so agents see why install failed. + expect(stdout).toMatch(/EACCES: permission denied at \/usr\/local\/lib/); + }); + + // Agents driving `ably init --json` need a structured signal for the + // install step's outcome. logProgress / logSuccessMessage are silent in + // JSON mode by design, so without a dedicated event the install step is + // invisible unless it fails. We emit one `install` event per init run. + describe("JSON install events", () => { + it("emits status=installed on a successful global install", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + ( + globalThis.__TEST_MOCKS__ as Record + ).isRunningFromNpx = true; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async () => {}; + + const { stdout, error } = await runCommand( + ["init", "--target", "cursor", "--json"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const event = findInstallEvent(stdout); + expect(event?.install).toEqual({ + status: "installed", + package: "@ably/cli@latest", + }); + }); + + it("emits status=skipped reason=no-install-flag when --no-install is passed", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + ( + globalThis.__TEST_MOCKS__ as Record + ).isRunningFromNpx = true; + + const { stdout, error } = await runCommand( + ["init", "--target", "cursor", "--json", "--no-install"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const event = findInstallEvent(stdout); + expect(event?.install).toEqual({ + status: "skipped", + reason: "no-install-flag", + }); + }); + + it("emits status=skipped reason=not-npx when --no-install is passed outside npx", async () => { + // The flag is only meaningful in the npx flow. From a globally + // installed binary, the more accurate reason for skipping is that + // we're not in npx — not the flag. + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + // No isRunningFromNpx override → defaults to false. + + const { stdout, error } = await runCommand( + ["init", "--target", "cursor", "--json", "--no-install"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const event = findInstallEvent(stdout); + expect(event?.install).toEqual({ + status: "skipped", + reason: "not-npx", + }); + }); + + it("emits status=skipped reason=not-npx when running outside npx", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + // Don't set isRunningFromNpx — default is false. + + const { stdout, error } = await runCommand( + ["init", "--target", "cursor", "--json"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const event = findInstallEvent(stdout); + expect(event?.install).toEqual({ + status: "skipped", + reason: "not-npx", + }); + }); + + it("emits status=failed with error details when install throws", async () => { + mockFetchWithTarball(await buildSkillsTarball("ably-pubsub")); + ( + globalThis.__TEST_MOCKS__ as Record + ).isRunningFromNpx = true; + (globalThis.__TEST_MOCKS__ as Record).installGlobally = + async () => { + throw new Error("EACCES: permission denied"); + }; + + const { stdout, error } = await runCommand( + ["init", "--target", "cursor", "--json"], + import.meta.url, + ); + + expect(error).toBeUndefined(); + const event = findInstallEvent(stdout); + const install = event?.install as + | { status: string; package: string; error: { message: string } } + | undefined; + expect(install?.status).toBe("failed"); + expect(install?.package).toBe("@ably/cli@latest"); + expect(install?.error.message).toMatch(/EACCES/); + }); + }); + }); }); diff --git a/test/unit/utils/prompt-confirmation.test.ts b/test/unit/utils/prompt-confirmation.test.ts index 599685496..bbbb827aa 100644 --- a/test/unit/utils/prompt-confirmation.test.ts +++ b/test/unit/utils/prompt-confirmation.test.ts @@ -45,4 +45,44 @@ describe("promptForConfirmation", () => { await promptForConfirmation("Are you sure?"); expect(capturedQuery).toBe("Are you sure? [y/n]"); }); + + describe("defaultYes", () => { + it("appends [Y/n] suffix when defaultYes is true", async () => { + let capturedQuery = ""; + mockQuestion = (query, callback) => { + capturedQuery = query; + callback("y"); + }; + await promptForConfirmation("Install globally?", { defaultYes: true }); + expect(capturedQuery).toBe("Install globally? [Y/n]"); + }); + + it("treats empty input as yes when defaultYes is true", async () => { + mockQuestion = (_query, callback) => callback(""); + const result = await promptForConfirmation("Install globally?", { + defaultYes: true, + }); + expect(result).toBe(true); + }); + + it("still treats explicit 'n' as no when defaultYes is true", async () => { + mockQuestion = (_query, callback) => callback("n"); + const result = await promptForConfirmation("Install globally?", { + defaultYes: true, + }); + expect(result).toBe(false); + }); + + it("does not double-append [Y/n] when the message already contains it", async () => { + let capturedQuery = ""; + mockQuestion = (query, callback) => { + capturedQuery = query; + callback("y"); + }; + await promptForConfirmation("Install globally? [Y/n]", { + defaultYes: true, + }); + expect(capturedQuery).toBe("Install globally? [Y/n]"); + }); + }); });