Skip to content

fix: make npx @ably/cli <command> work without the redundant ably#419

Open
mattheworiordan wants to merge 1 commit into
mainfrom
fix/npx-single-command
Open

fix: make npx @ably/cli <command> work without the redundant ably#419
mattheworiordan wants to merge 1 commit into
mainfrom
fix/npx-single-command

Conversation

@mattheworiordan

@mattheworiordan mattheworiordan commented Jun 17, 2026

Copy link
Copy Markdown
Member

Whilst watching someone today onboard and try the one click install, it became evident npx -p @ably/cli ably init has unnecessary duplication of ably, and looking at Neon for comparison, they've kept it simpler with npx neonctl init which works well.

Note trying npx @ably/cli init fails with "could not determine executable to run", so you fall back to npx -p @ably/cli ably init. The repeated ably is pointless, and one command is what people expect from npx. We couldn't give them that, which is a smell. This fixes it.

The cause: the package shipped two bins pointing at different files (ably and ably-interactive). npm's resolver only auto-picks an executable when there's a single bin, or they all point at the same target. Two different ones, so npx can't choose. The fix is a single ably bin, same shape as every other scoped CLI (@angular/cli gives you ng, @vue/cli gives you vue). npx @ably/cli <command> now resolves cleanly.

Nothing breaks:

  • npx @ably/cli <command> now works
  • npx -p @ably/cli ably <command> still works
  • npx @ably/cli ably <command> still works (run.js drops the redundant leading ably)
  • npm install -g @ably/cli then ably <command> is unchanged

One thing to coordinate: ably-interactive is no longer a separate global binary, though the wrapper script still ships in the package. The only runtime consumer is our terminal server, which installs @ably/cli@latest, so it needs a one-line Dockerfile symlink to land alongside this. Companion PR links to this PR.

Tests: a unit test pins the single-bin resolution and fails loudly if a second bin creeps back, and an integration test covers the redundant-ably form.

Docs follow-up if we're happy with this: the onboarding that says npx -p @ably/cli ably init can become npx @ably/cli init.

…oken

`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 <command>` 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 <packed tarball> --version`.

Backwards compatible: `npx -p @ably/cli ably <command>` is unaffected, and
run.js now drops a redundant leading `ably` token so the historical
`npx @ably/cli ably <command>` 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 <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cli-web-cli Ready Ready Preview, Comment Jun 17, 2026 9:37pm

Request Review

@claude-code-ably-assistant

Copy link
Copy Markdown

Walkthrough

This PR fixes npx @ably/cli <command> failing with "could not determine executable to run". The root cause was package.json declaring two bin entries pointing at different files (ablyrun.js, ably-interactivebin/ably-interactive), which trips npm's bin resolver. The fix removes ably-interactive from bin, leaving a single entry so npx resolves deterministically. A backwards-compat shim in bin/run.js strips the redundant leading ably token so the old npx @ably/cli ably <command> form keeps working.

Changes

Area Files Summary
Config package.json Remove ably-interactive bin entry; keep only ably → ./bin/run.js
Entrypoint bin/run.js Splice out a redundant leading ably argv token before oclif runs
Tests test/unit/package/npx-bin-resolution.test.ts Re-implements npm's libnpmexec bin-resolver algorithm; regression-guards single-bin shape
Tests test/integration/commands/npx-backwards-compat.test.ts Spawns the real binary to verify plain and ably ably version forms are equivalent
Docs CHANGELOG.md Unreleased entry documenting the npx behaviour change and compatibility guarantees

Review Notes

  • Deployment dependency: ably-interactive is no longer a global binary after npm install -g @ably/cli. The terminal server (which runs @ably/cli@latest) needs a companion Dockerfile symlink — the PR description links to it but reviewers should confirm that companion lands before or alongside this release.
  • Unit test re-implements third-party logic: npx-bin-resolution.test.ts hand-codes the libnpmexec bin-selector algorithm and explicitly says it is "kept in sync intentionally". This is a reasonable tradeoff given the algorithm is small and stable, but it's worth noting that a future npm version bump could silently diverge.
  • bin/run.js token-strip is unconditional: The splice runs on every invocation of run.js where argv[2] === 'ably'. The comment argues this is safe because there is no top-level ably subcommand — worth verifying that constraint stays true if new top-level commands are ever added.
  • Integration test uses spawn directly: This is appropriate since it exercises the real binary path, not the oclif test harness. No auth is needed — tests use ably version which is fully offline.
  • No new runtime dependencies introduced.

@claude-code-ably-assistant claude-code-ably-assistant Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Clean, well-targeted fix. The root cause analysis is correct (two bins pointing at different targets trips npm's resolver) and the solution is the right one. No bugs found.


What this PR does:

  • Removes the ably-interactive bin entry from package.json, leaving a single ably bin
  • Adds backwards-compat stripping of a redundant leading 'ably' token in bin/run.js so 'npx @ably/cli ably ' keeps working
  • Adds a unit test pinning the npm resolution algorithm and a regression guard if a second bin creeps back
  • Adds an integration test covering the backwards-compat stripping

Correctness checks:

bin/run.js stripping logic: process.argv.splice(2, 1) fires before execute(), so oclif never sees the redundant token. The process.argv.includes('interactive') check runs after the strip, so 'npx @ably/cli ably interactive' correctly sets ABLY_INTERACTIVE_MODE. No issues.

Edge cases covered:

  • No args: process.argv[2] is undefined, comparison is false — safe
  • Double 'ably': only the first is stripped, second becomes the command name, oclif returns 'not found' — covered by test 3
  • 'ably --help' strips to top-level help — correct

The algorithm copy in the unit test: The test reimplements npm's bin resolution from libnpmexec/lib/get-bin-from-manifest.js and the comment explicitly flags it as an intentional copy kept in sync. This is the right tradeoff: you get a regression guard that fails loudly if a second bin returns, at the cost of the algorithm potentially drifting from npm. Worth the call — this algorithm has been stable across npm major versions.


One thing to coordinate before merge:

The PR description flags it already, but worth calling out explicitly: removing ably-interactive as a global bin BREAKS THE TERMINAL SERVER until the companion Dockerfile symlink lands. These two PRs need to be merged and deployed in the right order. If the terminal server picks up this package version before the Dockerfile change, ably-interactive won't be on PATH and interactive mode will stop working.

This is already documented in the PR — just making sure it's not lost.


Verdict: Approve. The code is correct, the tests cover the key cases, and the coordination dependency is documented. No code changes requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants