Skip to content

Commit 19ad85c

Browse files
umair-ablyclaudesacOO7
authored
feat(init): install @ably/cli globally when launched via npx (#394)
* feat(init): install @ably/cli globally when launched via npx `ably init` is the documented one-command onboarding entry point, but `npx @ably/cli init` only landed the CLI in the ephemeral npx cache — after init exited, `ably` was no longer on PATH. This change makes init detect the npx invocation, prompt the user (default Y), and run `npm install -g @ably/cli@latest` so the CLI persists across shells. - Add `--no-install` flag for users who manage @ably/cli with their own package manager. - Install runs silently in --json mode (no prompt, stdio piped) so the NDJSON stream stays clean for agent consumers. - Install failures are non-fatal: a warning is logged with the captured stderr and the user is told to run `npm install -g @ably/cli` manually. - Extend `promptForConfirmation` with an optional `defaultYes` flag for non-destructive prompts (empty answer counts as yes, suffix `[Y/n]`). - Test hooks (`isRunningFromNpx`, `confirmGlobalInstall`, `installGlobally`) follow the existing `runLogin` pattern so unit tests never shell out to real `npm install -g`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(init): address PR review feedback - promptForConfirmation: add `[Y/n]` to the no-double-append guard so a caller passing both a manual `[Y/n]` suffix and `{ defaultYes: true }` doesn't get the suffix doubled. Adds regression test. - init global-install warning: in non-JSON mode the underlying npm process runs with `stdio: "inherit"` so npm has already printed the real error to the user's terminal. The thrown error's `.message` is just "Command failed: npm install -g @ably/cli@latest" — restating that adds no information. Emit a terse warning in non-JSON mode and keep the detailed warning (with captured stderr) in JSON mode where npm output was piped. Update the failure-path test and add a new JSON-mode failure test asserting captured stderr is surfaced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(init): bound global install with timeout and emit JSON outcome event Address PR review feedback: - execSync had no timeout, so a flaky network or stalled npm registry could leave `ably init` hung indefinitely with no feedback during onboarding. Cap the global install at 120s; ETIMEDOUT flows through the existing catch block as a non-fatal warning. - In `--json` mode the install step was invisible to agents on success (logProgress / logSuccessMessage are silent by design), so agents could only detect install failure, never success or a skip. Emit one structured `install` event per init run covering all outcomes: installed, skipped (no-install-flag | not-npx), failed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(init): check npx context before --no-install when skipping Reorder the early-return guards in maybeInstallGlobally so the npx context is checked first. --no-install is only meaningful when an install would otherwise happen (i.e. when running via npx — the flag's own help text says so). Previously `ably init --no-install --json` from a globally installed binary would emit { status: "skipped", reason: "no-install-flag" } even though the real reason it's skipped is that we're not in npx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Update src/commands/init.ts Co-authored-by: sachin shinde <sachinshinde7676@gmail.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: sachin shinde <sachinshinde7676@gmail.com>
1 parent 9233fd2 commit 19ad85c

4 files changed

Lines changed: 511 additions & 6 deletions

File tree

src/commands/init.ts

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import { execSync } from "node:child_process";
2+
import path from "node:path";
3+
14
import { Flags } from "@oclif/core";
25
import chalk from "chalk";
36

@@ -10,10 +13,18 @@ import {
1013
import { TARGET_CONFIGS } from "../services/skills-installer.js";
1114
import { resolveSkillsTargets } from "../services/skills-target-prompt.js";
1215
import { BaseFlags } from "../types/cli.js";
16+
import { extractErrorInfo } from "../utils/errors.js";
1317
import { displayLogo } from "../utils/logo.js";
1418
import { formatHeading, formatResource } from "../utils/output.js";
19+
import { promptForConfirmation } from "../utils/prompt-confirmation.js";
1520
import isTestMode from "../utils/test-mode.js";
1621

22+
// Bound on the global install step so a hung npm registry can't leave the
23+
// onboarding command stuck with no feedback. On timeout execSync throws
24+
// ETIMEDOUT, which the catch block in maybeInstallGlobally turns into the
25+
// usual non-fatal warning (and JSON failure event).
26+
const GLOBAL_INSTALL_TIMEOUT_MS = 120_000;
27+
1728
export default class Init extends AblyBaseCommand {
1829
static override description =
1930
"Set up Ably for AI-powered development — authenticate and install Agent Skills";
@@ -23,6 +34,7 @@ export default class Init extends AblyBaseCommand {
2334
"<%= config.bin %> <%= command.id %> --target claude-code",
2435
"<%= config.bin %> <%= command.id %> --target cursor --target windsurf",
2536
"<%= config.bin %> <%= command.id %> --target auto",
37+
"<%= config.bin %> <%= command.id %> --no-install",
2638
"<%= config.bin %> <%= command.id %> --json",
2739
];
2840

@@ -35,6 +47,11 @@ export default class Init extends AblyBaseCommand {
3547
default: ["auto"],
3648
description: "Target IDE(s) to install skills for",
3749
}),
50+
"no-install": Flags.boolean({
51+
default: false,
52+
description:
53+
"Skip installing @ably/cli globally (only relevant when launched via npx)",
54+
}),
3855
};
3956

4057
async run(): Promise<void> {
@@ -55,6 +72,8 @@ export default class Init extends AblyBaseCommand {
5572
displayLogo(this.log.bind(this));
5673
}
5774

75+
await this.maybeInstallGlobally(flags);
76+
5877
await this.runAuth(flags);
5978

6079
const resolvedTargets = await resolveSkillsTargets({
@@ -183,4 +202,146 @@ export default class Init extends AblyBaseCommand {
183202
if (process.env.ABLY_ACCESS_TOKEN) return true;
184203
return Boolean(this.configManager.getAccessToken());
185204
}
205+
206+
// When invoked via `npx @ably/cli init`, the running binary lives in an
207+
// ephemeral npx cache that is not on PATH. Without a global install the user
208+
// can't run `ably` again after init exits — defeating the "one-command
209+
// onboarding" promise. Detect that situation and offer to install globally.
210+
private async maybeInstallGlobally(flags: BaseFlags): Promise<void> {
211+
const jsonMode = this.shouldOutputJson(flags);
212+
213+
// Order matters: --no-install is only meaningful when we would otherwise
214+
// install (i.e. when running via npx). Checking the npx context first
215+
// means a normal `ably init --no-install` from a globally installed
216+
// binary reports the accurate reason ("not-npx") rather than the
217+
// irrelevant flag.
218+
if (!this.isRunningFromNpx()) {
219+
this.emitInstallEvent(flags, { status: "skipped", reason: "not-npx" });
220+
return;
221+
}
222+
if (flags["no-install"]) {
223+
this.emitInstallEvent(flags, {
224+
status: "skipped",
225+
reason: "no-install-flag",
226+
});
227+
return;
228+
}
229+
230+
if (!jsonMode) {
231+
const confirmed = await this.confirmGlobalInstall();
232+
if (!confirmed) {
233+
this.logWarning(
234+
"Skipping global install. To install later, Run: npm install -g @ably/cli",
235+
flags,
236+
);
237+
return;
238+
}
239+
}
240+
241+
this.logProgress("Installing @ably/cli globally", flags);
242+
try {
243+
await this.runGlobalInstall(jsonMode);
244+
this.logSuccessMessage("Installed @ably/cli globally.", flags);
245+
this.emitInstallEvent(flags, {
246+
status: "installed",
247+
package: "@ably/cli@latest",
248+
});
249+
} catch (error) {
250+
this.emitInstallEvent(flags, {
251+
status: "failed",
252+
package: "@ably/cli@latest",
253+
error: extractErrorInfo(error),
254+
});
255+
if (jsonMode) {
256+
// npm output was piped, so the thrown error already carries the
257+
// captured stderr — surface it so agents see why install failed.
258+
const detail = error instanceof Error ? error.message : String(error);
259+
this.logWarning(
260+
`Could not install @ably/cli globally automatically (${detail}). Run: npm install -g @ably/cli`,
261+
flags,
262+
);
263+
} else {
264+
// npm output was inherited, so npm has already printed the real error
265+
// to the user's terminal. error.message is just "Command failed: ..."
266+
// which adds no information — keep the warning terse.
267+
this.logWarning(
268+
"Could not install @ably/cli globally. Run: npm install -g @ably/cli",
269+
flags,
270+
);
271+
}
272+
}
273+
}
274+
275+
private emitInstallEvent(
276+
flags: BaseFlags,
277+
install: Record<string, unknown>,
278+
): void {
279+
if (!this.shouldOutputJson(flags)) return;
280+
this.logJsonEvent({ install }, flags);
281+
}
282+
283+
private async confirmGlobalInstall(): Promise<boolean> {
284+
if (isTestMode()) {
285+
const hook = globalThis.__TEST_MOCKS__?.confirmGlobalInstall;
286+
if (typeof hook === "boolean") return hook;
287+
}
288+
return promptForConfirmation(
289+
"Install @ably/cli globally so you can run 'ably' from any shell?",
290+
{ defaultYes: true },
291+
);
292+
}
293+
294+
private isRunningFromNpx(): boolean {
295+
if (isTestMode()) {
296+
const hook = globalThis.__TEST_MOCKS__?.isRunningFromNpx;
297+
if (typeof hook === "boolean") return hook;
298+
}
299+
const entry = process.argv[1] ?? "";
300+
return entry.includes(`${path.sep}_npx${path.sep}`);
301+
}
302+
303+
// Test hook: when the unit tests set globalThis.__TEST_MOCKS__.installGlobally
304+
// to a recording or throwing function, use that instead of shelling out to
305+
// `npm install -g` — which would mutate the developer's machine and require
306+
// network access during unit tests.
307+
//
308+
// In JSON mode we pipe npm's output instead of inheriting so the agent's
309+
// NDJSON stream isn't polluted with "added N packages" / deprecation
310+
// warnings. On failure we re-throw with the captured stderr appended so the
311+
// caller's warning still surfaces the root cause.
312+
private async runGlobalInstall(jsonMode: boolean): Promise<void> {
313+
if (isTestMode()) {
314+
const hook = globalThis.__TEST_MOCKS__?.installGlobally as
315+
| ((pkg: string) => Promise<void>)
316+
| undefined;
317+
if (hook) {
318+
await hook("@ably/cli@latest");
319+
return;
320+
}
321+
}
322+
if (jsonMode) {
323+
try {
324+
execSync("npm install -g @ably/cli@latest", {
325+
stdio: "pipe",
326+
timeout: GLOBAL_INSTALL_TIMEOUT_MS,
327+
});
328+
} catch (error) {
329+
const stderr = (
330+
error as { stderr?: Buffer | string } | undefined
331+
)?.stderr
332+
?.toString()
333+
.trim();
334+
const baseMessage =
335+
error instanceof Error ? error.message : String(error);
336+
throw new Error(stderr ? `${baseMessage}: ${stderr}` : baseMessage, {
337+
cause: error,
338+
});
339+
}
340+
return;
341+
}
342+
execSync("npm install -g @ably/cli@latest", {
343+
stdio: "inherit",
344+
timeout: GLOBAL_INSTALL_TIMEOUT_MS,
345+
});
346+
}
186347
}

src/utils/prompt-confirmation.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,39 @@ import * as readline from "node:readline";
22

33
/**
44
* Prompts the user for confirmation with a yes/no question.
5-
* Automatically appends " [y/n]" to the message if not already present.
65
* Accepts both "y" and "yes" as affirmative responses (case-insensitive).
76
*
87
* @param message - The confirmation message to display to the user
9-
* @returns Promise<boolean> - true if user confirms (y/yes), false otherwise
8+
* @param options.defaultYes - If true, an empty answer counts as yes and the
9+
* default suffix becomes " [Y/n]". Use only for non-destructive prompts.
10+
* @returns Promise<boolean> - true if user confirms, false otherwise
1011
*/
11-
export function promptForConfirmation(message: string): Promise<boolean> {
12+
export function promptForConfirmation(
13+
message: string,
14+
options: { defaultYes?: boolean } = {},
15+
): Promise<boolean> {
1216
const rl = readline.createInterface({
1317
input: process.stdin,
1418
output: process.stdout,
1519
});
1620

17-
// Add " [y/n]" suffix if not already present
21+
const suffix = options.defaultYes ? "[Y/n]" : "[y/n]";
1822
const promptMessage =
1923
message.includes("[yes/no]") ||
2024
message.includes("[y/n]") ||
25+
message.includes("[Y/n]") ||
2126
message.includes("[Y/N]")
2227
? message
23-
: `${message} [y/n]`;
28+
: `${message} ${suffix}`;
2429

2530
return new Promise<boolean>((resolve) => {
2631
rl.question(promptMessage, (answer) => {
2732
rl.close();
2833
const response = answer.toLowerCase().trim();
34+
if (response === "") {
35+
resolve(Boolean(options.defaultYes));
36+
return;
37+
}
2938
resolve(response === "y" || response === "yes");
3039
});
3140
});

0 commit comments

Comments
 (0)