From 9a0ad53d8cea5427590143f8136d79c61acee045 Mon Sep 17 00:00:00 2001 From: Matthew O'Riordan Date: Wed, 17 Jun 2026 22:26:11 +0200 Subject: [PATCH] fix: make `npx @ably/cli ` work without a redundant `ably` token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `npx @ably/cli init` (and any other command) failed with "could not determine executable to run", forcing users into the verbose `npx -p @ably/cli ably init`. People expect a single command with npx; the fact that we couldn't do that was a packaging smell. Root cause: the package declared two `bin` entries pointing at different targets (`ably` -> ./bin/run.js, `ably-interactive` -> ./bin/ably-interactive). npm's bin resolver (libnpmexec/get-bin-from-manifest) only auto-selects an executable when either every bin points at the same target, or a bin is named after the unscoped package name (`cli`). Two differently-targeted bins, neither named `cli`, hit the "could not determine executable to run" branch. Fix: expose a single `bin` (`ably`) — the same shape every other scoped CLI uses (`@angular/cli` -> ng, `@vue/cli` -> vue). `npx @ably/cli ` now resolves deterministically. Verified against npm's own resolver (old manifest throws the exact error; new manifest returns `ably`) and end-to-end via `npx --version`. Backwards compatible: `npx -p @ably/cli ably ` is unaffected, and run.js now drops a redundant leading `ably` token so the historical `npx @ably/cli ably ` form keeps working too. There is no top-level `ably` command, so a leading `ably` is unambiguously the repeated bin name. Global installs (`npm install -g @ably/cli` then `ably ...`) are unchanged. The `bin/ably-interactive` auto-restart wrapper script is retained in the package (still used by `ably interactive` and its tests) but is no longer installed as a separate global executable. The one runtime consumer of that binary, ably/cli-terminal-server, installs `@ably/cli@latest`, so its image build needs a companion one-line Dockerfile symlink landed alongside this change. Tests: a hermetic unit test re-implements npm's resolver and asserts the package always resolves to a single `ably` executable; an integration test covers the redundant-`ably` backwards-compatible invocation. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 6 ++ bin/run.js | 11 +++ package.json | 3 +- .../commands/npx-backwards-compat.test.ts | 52 ++++++++++++++ test/unit/package/npx-bin-resolution.test.ts | 70 +++++++++++++++++++ 5 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 test/integration/commands/npx-backwards-compat.test.ts create mode 100644 test/unit/package/npx-bin-resolution.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 86cc6e99f..d490d00ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **`npx @ably/cli ` now works without a redundant `ably` token.** The package now exposes a single `bin` (`ably`), so `npm exec`/`npx` can resolve the executable deterministically. Previously the package declared two bins pointing at different targets, which tripped npm's bin resolver and forced the verbose `npx -p @ably/cli ably `. For backwards compatibility, a redundant leading `ably` token (`npx @ably/cli ably `) is still accepted and stripped. Globally installed usage (`npm install -g @ably/cli`; then `ably ...`) is unchanged. The `bin/ably-interactive` auto-restart wrapper script is retained in the package but is no longer installed as a separate global executable. + ## [1.2.0] - 2026-06-04 ### Added diff --git a/bin/run.js b/bin/run.js index 71f5a5e5b..ba9ce9e8a 100755 --- a/bin/run.js +++ b/bin/run.js @@ -1,5 +1,16 @@ #!/usr/bin/env node +// Backwards-compatible invocation: `npx @ably/cli ably ` passes a +// redundant leading `ably` token through to the CLI (the package is `@ably/cli` +// and the bin is `ably`, so people naturally repeat it). Now that +// `npx @ably/cli ` resolves the single `ably` bin directly, tolerate a +// leading `ably` so old docs, scripts and muscle memory keep working. There is +// no top-level `ably` command, so a leading `ably` is unambiguously the +// redundant binary name and is safe to drop. +if (process.argv[2] === 'ably') { + process.argv.splice(2, 1); +} + // For interactive mode, ensure SIGINT exits with code 130 if (process.argv.includes('interactive')) { process.env.ABLY_INTERACTIVE_MODE = 'true'; diff --git a/package.json b/package.json index 83d7c4756..4b2b7fe86 100644 --- a/package.json +++ b/package.json @@ -62,8 +62,7 @@ "author": "Ably ", "license": "Apache-2.0", "bin": { - "ably": "./bin/run.js", - "ably-interactive": "./bin/ably-interactive" + "ably": "./bin/run.js" }, "type": "module", "oclif": { diff --git a/test/integration/commands/npx-backwards-compat.test.ts b/test/integration/commands/npx-backwards-compat.test.ts new file mode 100644 index 000000000..e4b41ea60 --- /dev/null +++ b/test/integration/commands/npx-backwards-compat.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from "vitest"; +import { spawn } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const binPath = path.join(__dirname, "..", "..", "..", "bin", "run.js"); + +function run( + args: string[], +): Promise<{ code: number | null; stdout: string; stderr: string }> { + return new Promise((resolve) => { + const proc = spawn("node", [binPath, ...args], { env: { ...process.env } }); + let stdout = ""; + let stderr = ""; + proc.stdout.on("data", (d) => (stdout += d.toString())); + proc.stderr.on("data", (d) => (stderr += d.toString())); + proc.on("close", (code) => resolve({ code, stdout, stderr })); + }); +} + +/** + * `npx @ably/cli ably ` was historically the way to run the CLI (the + * package is `@ably/cli`, the bin is `ably`, so the token gets repeated). Now + * that the package is single-bin and `npx @ably/cli ` resolves + * directly, run.js drops a redundant leading `ably` so the old form keeps + * working. These tests guard that backwards-compatible behaviour. + * + * `version` is used because it runs fully offline (no API key required). + */ +describe("npx @ably/cli ably backwards compatibility", () => { + it("runs a command with a redundant leading `ably` token", async () => { + const redundant = await run(["ably", "version"]); + expect(redundant.code).toBe(0); + expect(redundant.stdout).toContain("@ably/cli/"); + expect(redundant.stderr).not.toMatch(/not found/i); + }); + + it("behaves identically to the plain invocation", async () => { + const [plain, redundant] = await Promise.all([ + run(["version"]), + run(["ably", "version"]), + ]); + expect(redundant.stdout.trim()).toBe(plain.stdout.trim()); + }); + + it("strips only a single leading `ably` (there is no `ably` command)", async () => { + const doubled = await run(["ably", "ably", "version"]); + expect(doubled.code).not.toBe(0); + expect(doubled.stderr).toMatch(/not found/i); + }); +}); diff --git a/test/unit/package/npx-bin-resolution.test.ts b/test/unit/package/npx-bin-resolution.test.ts new file mode 100644 index 000000000..5172d778d --- /dev/null +++ b/test/unit/package/npx-bin-resolution.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import path from "node:path"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const pkg = JSON.parse( + readFileSync(path.join(__dirname, "../../../package.json"), "utf8"), +) as { name: string; bin: Record }; + +/** + * Faithful re-implementation of npm's bin resolver, used by `npx`/`npm exec` + * to decide which executable to run for a bare `npx ` invocation. + * + * Source of truth (kept in sync intentionally — this is a small, stable algo): + * node_modules/libnpmexec/lib/get-bin-from-manifest.js + * + * 1. If every bin entry points at the SAME target, run the first one. + * 2. Else if a bin is named after the unscoped package name, run that. + * 3. Else throw "could not determine executable to run". + * + * This is why `npx @ably/cli ` must "just work" with no redundant + * `ably` token: the package has to satisfy rule 1 or rule 2. A second bin that + * points at a DIFFERENT target (e.g. a separate `ably-interactive` wrapper) + * trips rule 3 and forces users into `npx -p @ably/cli ably `. + */ +function getBinFromManifest(mani: { + name: string; + bin?: Record; +}): string { + const bin = mani.bin ?? {}; + if (new Set(Object.values(bin)).size === 1) { + return Object.keys(bin)[0]; + } + const unscoped = mani.name.replace(/^@[^/]+\//, ""); + if (bin[unscoped]) { + return unscoped; + } + throw new Error("could not determine executable to run"); +} + +describe("npx @ably/cli bin resolution", () => { + it("resolves to a single, deterministic executable (no redundant token)", () => { + // If this throws, `npx @ably/cli ` breaks with + // "could not determine executable to run" and users are forced back to + // `npx -p @ably/cli ably `. + expect(() => getBinFromManifest(pkg)).not.toThrow(); + expect(getBinFromManifest(pkg)).toBe("ably"); + }); + + it("the resolved bin is the main oclif entrypoint", () => { + const binName = getBinFromManifest(pkg); + expect(pkg.bin[binName]).toBe("./bin/run.js"); + }); + + // Documents the failure mode the single-bin shape protects against, so a + // future change that re-introduces a second, differently-targeted bin fails + // loudly here rather than silently regressing the npx experience. + it("regression guard: a second bin with a different target breaks npx", () => { + expect(() => + getBinFromManifest({ + name: "@ably/cli", + bin: { + ably: "./bin/run.js", + "ably-interactive": "./bin/ably-interactive", + }, + }), + ).toThrow(/could not determine executable to run/); + }); +});