feat(completion): add completion install subcommand and wire it into installer.sh / npm#61
Draft
pjcdawkins wants to merge 5 commits into
Draft
Conversation
Adds the build and publish tooling for distributing the Upsun CLI as an npm package, so users can run `npm install -g upsun` or `npx upsun`. Implements the optionalDependencies pattern used by esbuild, swc, biome, turbo, and similar tools. A thin wrapper package (`upsun`) declares four platform-specific packages as optionalDependencies; npm uses each package's `os` and `cpu` fields to install only the matching one. The wrapper's bin script resolves that package at runtime and execs the embedded binary, forwarding argv, stdio, exit code, and signals. No postinstall script and no runtime download. Packages produced per release: upsun wrapper @upsun/cli-linux-x64 @upsun/cli-linux-arm64 @upsun/cli-darwin universal binary, x64 and arm64 @upsun/cli-win32-x64 macOS uses the universal binary that GoReleaser already builds, so a single darwin package covers both Apple Silicon and Intel. publish.sh classifies each tarball by reading its package.json once (wrapper = "upsun", everything else is a platform package), publishes the platform packages, waits for them to become queryable on the public registry, then publishes the wrapper. The wait matters: npm publish returns success before the package is visible to npm view, and any user running npx in that window gets a broken install in their npx cache that does not self-heal. Layout: - npm/wrapper: shim package with bin/upsun.js - npm/platform-template: template for the per-platform packages - npm/scripts/build.sh: assembles tarballs from GoReleaser archives - npm/scripts/publish.sh: classifies, publishes, waits, then wrapper - Makefile targets: npm-pack, npm-publish, npm-clean Verified end-to-end against the live registry: build, publish, install via `npx upsun`, exec, argv passthrough, and exit-code propagation all work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review comments on PR #46: - Replace declare -A associative arrays in build.sh and publish.sh with case-statement helpers and parallel arrays, so the scripts run on macOS's default /bin/bash (3.2). - Stop swallowing chmod failures on Unix targets in build.sh; only the Windows binary, where the exec bit is meaningless, keeps || true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The deb/rpm/apk packages and the Homebrew formula already drop a
completion file, so users who install the CLI those ways get
autocompletion automatically. Users on installer.sh and npm have to
add `eval "$(upsun completion)"` to their shell rc by hand.
Adds an `upsun completion install` subcommand modelled after the
equivalents in fly, supabase, and gh: detect the user's shell from
$SHELL, render the completion script (via the legacy CLI's existing
Symfony-backed `completion <shell>` command), and write it to the
standard location for that shell:
bash, root: /etc/bash_completion.d/upsun
bash, user: ${XDG_DATA_HOME:-~/.local/share}/bash-completion/completions/upsun
zsh, root: /usr/local/share/zsh/site-functions/_upsun
zsh, user: ~/.zsh/completions/_upsun
Flags --shell, --path, and --print-path cover overrides and dry runs.
Writes go through internal/file.Write so the file appears atomically.
For zsh, the user-level path may not be in $fpath, so the post-install
output prints the snippet to add. PowerShell is out of scope: Symfony
Console doesn't emit a PowerShell completion template.
Refs CLI-139.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The raw install path (binary download) doesn't drop a completion file, unlike apt/yum/apk/homebrew. Now that `upsun completion install` exists, installer.sh can call it after the binary is in place. Skipped when: - INSTALL_METHOD is not "raw" (apt/yum/apk/homebrew already cover it) - $INSTALL_NO_COMPLETION is set (opt-out for scripted installs) - is_ci returns true (these paths are unattended by definition) - $SHELL is not bash or zsh (only shells we generate completions for) A failure inside `upsun completion install` is non-fatal — the binary is already installed, so we add a footer note pointing the user at the manual command rather than aborting the whole install. Refs CLI-139. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After `npm install -g upsun`, print a one-liner pointing the user at `upsun completion install`. The hint stays silent in three contexts where it would only be noise: - non-global installs (the wrapper as a transitive dependency in some user project), via the npm_config_global env var - non-TTY contexts (npx cache warmups, scripted installs) - CI, or when UPSUN_NO_COMPLETION_HINT is set The postinstall script never modifies any shell config — it only prints — so --ignore-scripts users lose the hint, not functionality. Refs CLI-139. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a first-class upsun completion install subcommand and integrates it into installation flows that previously didn’t set up shell completion (raw installer.sh path) or didn’t surface it (npm global installs).
Changes:
- Add
upsun completion install(bash/zsh) with shell detection, default path selection, atomic write, and post-install guidance. - Update
installer.sh(raw installs) to automatically run completion installation (non-fatal, skippable). - Add an npm
postinstallhint and document the completion install step.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
commands/completion.go |
Refactors completion generation and adds completion install implementation. |
commands/completion_test.go |
Adds tests for shell detection and default completion paths. |
installer.sh |
Adds optional post-install completion setup for raw installs + env skip flag. |
README.md |
Documents INSTALL_NO_COMPLETION installer env var. |
npm/wrapper/package.json.tmpl |
Adds a postinstall hook and includes the script in published files. |
npm/wrapper/bin/postinstall.js |
Prints a one-line completion install hint for global interactive installs. |
npm/wrapper/README.md |
Documents how to enable completion for npm installs. |
npm/scripts/build.sh |
Ensures postinstall.js is packaged into the wrapper tarball. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+32
| func TestDefaultCompletionPath(t *testing.T) { | ||
| if os.Geteuid() == 0 { | ||
| t.Skip("non-root user paths only") | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Same as the reply on completion.go: os.Geteuid() is implemented on Windows and returns -1, so this test compiles fine under GOOS=windows. Verified with GOOS=windows GOEXPERIMENT=jsonv2 go test -c ./commands/.
The Geteuid() == 0 skip is just to avoid running the non-root path assertions when the suite happens to be run as root on Linux. On Windows it's harmless: -1 != 0, so the test runs and the assertions still hold (the function builds and returns the same paths).
Comment on lines
+166
to
+170
| const ( | ||
| systemBashDir = "/etc/bash_completion.d" | ||
| systemZshDir = "/usr/local/share/zsh/site-functions" | ||
| ) | ||
| isRoot := os.Geteuid() == 0 |
Contributor
Author
There was a problem hiding this comment.
os.Geteuid() is implemented on Windows — it returns -1 (see syscall.Geteuid on Windows). Verified by building this package with GOOS=windows GOEXPERIMENT=jsonv2 go build ./... and compiling the test binary with go test -c; both succeed.
Returning -1 means the comparison Geteuid() == 0 is false on Windows, so we already fall through to the user-level path selection — i.e. Windows is already treated as non-root, which matches the recommendation in this comment.
pjcdawkins
force-pushed
the
cli-126-distribute-cli-via-npm
branch
from
568d34e to
0497a10
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new
upsun completion installsubcommand and wires it into the install methods that don't currently set up shell completion.The deb/rpm/apk packages and the Homebrew formula already drop a completion file. Users on the npm and raw install paths previously had to add
eval "$(upsun completion)"to their shell rc by hand.Changes
upsun completion install(new subcommand)$SHELL; overridable with--shellor a positional argument.completion <shell>plumbing, then writes it atomically to the standard location:/etc/bash_completion.d/upsun${XDG_DATA_HOME:-~/.local/share}/bash-completion/completions/upsun/usr/local/share/zsh/site-functions/_upsun~/.zsh/completions/_upsun--shell,--path,--print-path.$fpath, so the post-install output prints the snippet to add.installer.sh
upsun completion install.INSTALL_METHODis notraw(other paths already cover it), in CI, whenINSTALL_NO_COMPLETIONis set, or when$SHELLis not bash/zsh.npm wrapper
postinstallscript prints a one-line hint:To enable shell completion, run: upsun completion install.npm_config_global), non-TTY contexts, CI, and whenUPSUN_NO_COMPLETION_HINTis set.Out of scope
PowerShell / Scoop. Symfony Console doesn't emit a PowerShell completion template, so a Windows auto-install would require upstream work.