feat: upgrade command - #5
Conversation
🤖 Augment PR SummarySummary: Adds a new Changes:
Technical Notes: Refuses to run on 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| func latestReleaseTag(client *http.Client, repo string) (string, error) { | ||
| url := fmt.Sprintf("https://api.github.com/repos/%s/releases/latest", repo) | ||
| resp, err := client.Get(url) |
There was a problem hiding this comment.
internal/cmd/upgrade.go:170: The GitHub API requires a User-Agent header on requests; using client.Get directly can be rejected (commonly as 403), which would make upgrade fail even when networking is otherwise fine. Consider ensuring these requests include a User-Agent so the endpoint is reliably callable.
Severity: high
Other Locations
internal/cmd/upgrade.go:192internal/cmd/upgrade.go:224
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
a8eefb3 to
c97716b
Compare
|
augment review |
| // and always returns the newest non-pre-release tag, so a plain | ||
| // inequality is enough here -- no need to pull in a semver library | ||
| // just to answer "is there something to install". | ||
| if latestTag == version { |
There was a problem hiding this comment.
internal/cmd/upgrade.go:91: Treating any latestTag != version as an available upgrade can be misleading for pre-release builds (e.g. v1.2.0-beta), since /releases/latest returns the latest non-pre-release and may be older than the current version. In that case this would present a downgrade as an upgrade ("is available"), which could surprise users.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // couldn't remove its own running file. It's harmless clutter, but | ||
| // this is the one command that would ever create it, so it's also | ||
| // the natural place to sweep it up on a later run. | ||
| _ = os.Remove(exe + ".old") |
There was a problem hiding this comment.
internal/cmd/upgrade.go:68: _ = os.Remove(exe + ".old") runs on all platforms even though the .old file is only created by the Windows replacement path. This could delete a user-created <binary>.old file without confirmation.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| fmt.Printf("versola-cli %s is available (you have %s).\n", latestTag, version) | ||
| if !upgradeAssumeYes && !confirm("Update now?") { |
There was a problem hiding this comment.
internal/cmd/upgrade.go:102: confirm() reads from stdin even when it’s non-interactive, so versola upgrade can block in scripted/CI contexts unless --yes is provided. That makes automation behavior sensitive to how stdin is wired up.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| return release.TagName, nil | ||
| } | ||
|
|
||
| // downloadToFile streams url into path and also returns the full body, so |
There was a problem hiding this comment.
internal/cmd/upgrade.go:212-214: The downloadToFile doc comment says it "streams" into the file, but the implementation reads the full response into memory via io.ReadAll and only then writes it. This can mislead readers about memory usage/behavior for larger assets.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ### `upgrade` | ||
|
|
||
| Checks the latest versola-cli release on GitHub and, if it's different | ||
| from this binary's version, downloads it — verified against that |
There was a problem hiding this comment.
README.md:190-193: The docs state the download is verified against the release’s checksums.txt, but the implementation explicitly proceeds without verification on 404/other fetch errors or when the asset isn’t present in checksums.txt (with warnings). This wording may overstate the verification guarantee.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
1 participant
No description provided.