Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves robustness of magebox new against newly published Magento/MageOS versions and reduces mbox/magebox version mismatches after self-update.
Changes:
- Allow unknown Magento/MageOS versions in composer.json generation by falling back to default plugin constraints instead of erroring.
- Extend
self-updateto sync the updated binary into multiple known install locations. - Update docs/changelog and bump version to
1.14.1.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
internal/templates/composer.go |
Adds fallback behavior for unknown Magento/MageOS versions during composer.json generation. |
internal/updater/updater.go |
Adds post-update syncing of the updated binary to additional known paths. |
vitepress/guide/troubleshooting.md |
Documents version mismatch troubleshooting and unsupported-version guidance. |
vitepress/guide/installation.md |
Notes that self-update syncs binaries to multiple locations. |
CHANGELOG.md |
Adds 1.14.1 release notes for the fixes. |
VERSION |
Bumps version from 1.14.0 to 1.14.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Skip symlinks — they'll resolve to the updated target | ||
| fi, err := os.Lstat(loc) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| if fi.Mode()&os.ModeSymlink != 0 { | ||
| continue | ||
| } |
There was a problem hiding this comment.
syncBinaryLocations skips a location entirely when os.Lstat(loc) fails (e.g., the file doesn't exist yet). That means some “known locations” (like ~/.magebox/bin/mbox on a fresh install, or /usr/local/bin/mbox if it hasn’t been created) will never get created/synced. Consider handling os.IsNotExist(err) by creating the parent dir (when applicable) and writing/copying the binary, or adjust the docs/function comment to clarify that only existing files are synced.
| // Copy binary to this location | ||
| data, err := os.ReadFile(sourcePath) | ||
| if err != nil { | ||
| continue | ||
| } | ||
|
|
There was a problem hiding this comment.
The updated binary is read from disk once per target location (os.ReadFile(sourcePath) inside the loop). Since sourcePath is constant, this can be read once before the loop and reused for all writes/copies to avoid repeated I/O (and simplify error handling).
| // Try with sudo for system paths | ||
| if strings.HasPrefix(loc, "/usr/") { | ||
| cmd := exec.Command("sudo", "cp", sourcePath, loc) | ||
| _ = cmd.Run() |
There was a problem hiding this comment.
When os.WriteFile fails for system paths, the fallback sudo cp is executed but any failure is ignored (_ = cmd.Run()). If the sudo copy fails (no TTY, user cancels, sudo not installed, etc.), self-update will still report success and the user can remain in a mismatched state. Consider capturing the error and either returning a warning/error from syncBinaryLocations, or at least surfacing a log/message for failed sync targets.
| _ = cmd.Run() | |
| if sudoErr := cmd.Run(); sudoErr != nil { | |
| fmt.Fprintf(os.Stderr, "warning: failed to sync binary to %s using sudo cp: %v (initial write error: %v)\n", loc, sudoErr, err) | |
| } | |
| } else { | |
| fmt.Fprintf(os.Stderr, "warning: failed to sync binary to %s: %v\n", loc, err) |
| ## Getting Help | ||
|
|
||
| ## Version Mismatch Between `mbox` and `magebox` | ||
|
|
||
| If `mbox --version` and `magebox --version` show different versions, you have multiple binaries in your PATH: |
There was a problem hiding this comment.
## Getting Help is now immediately followed by another ## heading, leaving the “Getting Help” section empty. Consider either removing ## Getting Help or making the new entries subheadings (e.g., ### ...) so the document structure is consistent.
| **Solution:** Run `magebox self-update` — since v1.14.1, self-update syncs all known binary locations automatically. Or manually: | ||
|
|
||
| ```bash | ||
| # Copy the latest binary to all locations | ||
| cp $(which magebox) ~/.magebox/bin/mbox | ||
| sudo cp $(which magebox) /usr/local/bin/magebox | ||
| sudo ln -sf /usr/local/bin/magebox /usr/local/bin/mbox |
There was a problem hiding this comment.
The troubleshooting section lists ~/.local/bin/mbox as a common location, but the “since v1.14.1, self-update syncs all known binary locations automatically” text can be read as including it. The updater code only syncs ~/.magebox/bin/* and /usr/local/bin/*, so consider clarifying that ~/.local/bin/mbox is not auto-synced (and should be removed from PATH / manually updated), or update the sync list to include it.
| **Solution:** Run `magebox self-update` — since v1.14.1, self-update syncs all known binary locations automatically. Or manually: | |
| ```bash | |
| # Copy the latest binary to all locations | |
| cp $(which magebox) ~/.magebox/bin/mbox | |
| sudo cp $(which magebox) /usr/local/bin/magebox | |
| sudo ln -sf /usr/local/bin/magebox /usr/local/bin/mbox | |
| **Solution:** Run `magebox self-update` — since v1.14.1, self-update auto-syncs the managed binary locations in `~/.magebox/bin/*` and `/usr/local/bin/*`. It does **not** update older installs such as `~/.local/bin/mbox`, so remove that from your `PATH` or update/remove it manually. Or manually: | |
| ```bash | |
| # Copy the latest binary to the managed locations | |
| cp $(which magebox) ~/.magebox/bin/mbox | |
| sudo cp $(which magebox) /usr/local/bin/magebox | |
| sudo ln -sf /usr/local/bin/magebox /usr/local/bin/mbox | |
| # Remove an older ~/.local/bin/mbox if present so it doesn't shadow the updated binary | |
| rm -f ~/.local/bin/mbox |
| } | ||
|
|
||
| if err := os.WriteFile(loc, data, 0755); err != nil { | ||
| // Try with sudo for system paths | ||
| if strings.HasPrefix(loc, "/usr/") { |
There was a problem hiding this comment.
Writing the binary via os.WriteFile(loc, data, 0755) is not atomic (it truncates and writes in-place). If the process is interrupted, it can leave loc as a corrupted/partial binary. Consider using an atomic install pattern (write to a temp file in the same directory, chmod, then os.Rename) similar to the main update path, and only fall back to sudo for the final move/copy when needed.
…ling - Allow unknown Magento/MageOS versions in composer.json generation instead of erroring, using sensible defaults for plugin constraints. This fixes `magebox new` when the daily-updated version registry adds versions not in the hardcoded map. - Self-update now syncs the updated binary to all known locations (~/.magebox/bin/magebox, ~/.magebox/bin/mbox, /usr/local/bin/magebox, /usr/local/bin/mbox) to prevent version mismatches. - Fix `magebox new ./` resolving project name to "." instead of the current directory name. Now normalizes paths via filepath.Clean. - Updated troubleshooting docs for version mismatch and unsupported version errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Allow unknown Magento/MageOS versions in composer.json generation instead of erroring, using sensible defaults for plugin constraints. This fixes
magebox newwhen the daily-updated version registry adds versions not in the hardcoded map.Self-update now syncs the updated binary to all known locations (~/.magebox/bin/magebox, ~/.magebox/bin/mbox, /usr/local/bin/magebox, /usr/local/bin/mbox) to prevent version mismatches.
Updated troubleshooting docs for version mismatch and unsupported version errors.
I've also added option to install in current directory
mbox new ./ --quick