Repository navigation
Alternative CLIs: absolute shim paths, update notices from the embedded config, and an installer step - #215
Merged
Merged
Conversation
The shim referred to the config file via "${HOME}" or "${XDG_CONFIG_HOME}"
(or "%AppData%" on Windows). Under cron or CI, where those differ or are
unset, it pointed at the wrong file and the CLI failed with "failed to load
config". The shim now contains the absolute path.
The shim ran the CLI by name, or with --absolute by os.Executable(), which
resolves symlinks on Linux (e.g. to a versioned Homebrew Cellar path). It now
records the unresolved path found on the PATH at install time, falling back
to os.Executable() if the CLI is not on the PATH, and runs the CLI by name if
that file has gone. The --absolute option is removed, as the path is now
always recorded.
Paths are quoted for the shell when needed, and the Windows batch file gets
the same fallback.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…otices With CLI_CONFIG_FILE (e.g. an alternative CLI installed by config:install), the update check and notice read wrapper.* and the application name, executable and slug from the loaded config. That config describes the alternative CLI, not the binary that runs it, so the notice could not give the right upgrade command, or was not shown at all. Config.Embedded() returns the binary's embedded config. It is now used for the release repository, the install method override, the Homebrew formula, the package marker slug, the Scoop command and the name in the notice. The environment variable prefix, the updates settings and the state file still come from the loaded config, so each alternative CLI keeps its own check interval and notification throttle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When CONFIG_URL is set, the installer runs "upsun config:install -y" with it after installing the CLI, as the installing user. This installs an alternative CLI (a config file and a shim that runs upsun with it). A failure stops the installer with an error. CONFIG_URL and vendor_name are in one block at the top of the script, so a vendor's installer build can set them by substitution. Without CONFIG_URL the installer behaves as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![upsun-dispatch[bot]](https://avatars.githubusercontent.com/in/4153575?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 1 warning · 🔵 2 minor points
🔍 Full review · 16 files reviewed
Verification
- The Windows shim doubles
%in paths insideset "VAR=..."and in the quoted target, and falls back totargetNamethroughgoto fallbackwhen the target is missing. - POSIX
shellQuoteleaves safe paths unquoted and single-quotes anything else, escaping embedded'as'\'', so$HOMEin a path stays literal. Embedded()returns the receiver when the raw bytes equal the embedded YAML, otherwise a cached parse of the embedded config, and falls back to the receiver if that parse fails.IsAutoUpdatingandnewInstallProbestill read the env var prefix from the loaded config, so<PREFIX>INSTALL_METHODkeeps working for the alt CLI.- installer.sh runs
config:install -ywith stdin from /dev/null, and passes the fulldir_binpath for the raw method, so adir_binoutside PATH still works.
The PR adds unit tests: TestGenerateExecutable (shim_test.go), TestFindExecutableTarget, an updated TestConfigInstallCmd and TestAlt, TestEmbedded, TestPrintUpdateMessageWithAltConfig and TestNewInstallProbeWithAltConfig. CI runs them through the make test-go step in .github/workflows/ci.yml. No test covers the installer.sh install_config step, and none runs the Windows shim.
Review details
- Commit: 28d6967
- Model: claude-opus-5-5
Review 1 of 10 for this pull request · View the full run
…--absolute When config:install ran through an existing alt CLI, the shim targeted that alt CLI's executable (from the loaded config), so the new CLI ran with the other alt CLI's config. The target name now comes from the embedded config, and a config for that name is refused. --absolute is back, to run the current executable rather than the one found on the PATH (e.g. a build that is not on the PATH). Tests take the expected embedded values from Config.Embedded(), so they pass with other embedded configs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
📋 PR Summary Makes alternative CLI configs sturdier. Changes
|
With CLI_CONFIG_FILE, the binary may be a private copy installed by another installer, in a location that install detection does not recognize. The generic link to the upsun/cli upgrade instructions would not update that copy, so the notice is now skipped when there is no upgrade command for the install method. Without CLI_CONFIG_FILE, the link is shown as before. printUpdateMessage now takes the install method, so tests do not depend on the memoized detection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
config:installshims now contain the absolute config path, so they work under cron and CI whereHOMEorXDG_CONFIG_HOMEdiffer. They run theupsunfound onPATHat install time, by its unresolved path (e.g./opt/homebrew/bin/upsun), and fall back to aPATHlookup if it has gone. The Windows.batshim does the same.--absolutenow writes the path of the running executable instead. When run through another alt CLI, the shim still targets the binary's own executable (from the embedded config).CLI_CONFIG_FILE, update checks and notices takewrapper.*and the application name, executable and slug from the binary's embedded config, as the binary is unchanged. The env var prefix,updatessettings and state file still come from the loaded config. The notice is only shown if there is an upgrade command for the install method: the binary may be a private copy (e.g. from an existing vendor installer), which the generic upgrade instructions would not update.installer.sh: optionalCONFIG_URL, installed withconfig:install -yafter the CLI.CONFIG_URLandvendor_nameare in one block at the top so a vendor build can set them.PATH(e.g. a legacy location) can be written withconfig:install --force --bin-dir <dir>; the duplicate check is only aboutPATHprecedence, so no change is needed.🤖 Generated with Claude Code