feat(update): add check-only release diagnostics#2291
Conversation
Add `codewhale update --check` so users can compare the installed version with the latest release without downloading or replacing binaries. Surface the same release check in `codewhale doctor`, and share release lookup, mirror handling, timeout, and version comparison logic between update and doctor.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extracts shared release discovery, version comparison, and update check logic into a new workspace crate called codewhale-release. It also introduces a new --check flag to the CLI's update command and integrates update checks into the TUI's doctor command. The reviewer feedback focuses on making the new library crate more robust by passing the current binary version as an argument instead of relying on the compile-time env!("CARGO_PKG_VERSION") of the library itself. Additionally, the reviewer suggests removing a redundant error context layer in the HTTP response handling.
| url: &str, | ||
| description: &str, | ||
| ) -> Result<String> { | ||
| let body = body.with_context(|| format!("failed to read {description} response from {url}"))?; |
There was a problem hiding this comment.
The body parameter passed to release_response_body is already constructed with .with_context(...) in both fetch_release_json_blocking and fetch_release_json_async. Calling .with_context(...) again here with the exact same message is redundant and results in duplicate context layers in the error chain (e.g., failed to read release info response from ... appearing twice).
We can simplify this by directly propagating the error using ?.
| let body = body.with_context(|| format!("failed to read {description} response from {url}"))?; | |
| let body = body?; |
| pub fn resolve_release_query(channel: ReleaseChannel) -> ReleaseQuery { | ||
| let version = update_version_from_env().unwrap_or_else(|| env!("CARGO_PKG_VERSION").into()); | ||
| if let Some(base_url) = release_base_url_from_env(&version) { | ||
| return ReleaseQuery::Mirror { base_url, version }; | ||
| } |
There was a problem hiding this comment.
Using env!("CARGO_PKG_VERSION") inside a shared library crate (codewhale-release) evaluates to the version of the library itself at compile time, rather than the version of the running binary (codewhale-cli or codewhale-tui). While they currently share the same version in the workspace, this is a fragile pattern that can lead to subtle bugs if the library and binary versions ever diverge or are packaged/built separately.
To make the library robust and reusable, we should pass the current_version as an argument to resolve_release_query (and consequently to latest_release_tag_async and latest_release_tag_blocking).
| pub fn resolve_release_query(channel: ReleaseChannel) -> ReleaseQuery { | |
| let version = update_version_from_env().unwrap_or_else(|| env!("CARGO_PKG_VERSION").into()); | |
| if let Some(base_url) = release_base_url_from_env(&version) { | |
| return ReleaseQuery::Mirror { base_url, version }; | |
| } | |
| pub fn resolve_release_query(channel: ReleaseChannel, current_version: &str) -> ReleaseQuery { | |
| let version = update_version_from_env().unwrap_or_else(|| current_version.to_string()); | |
| if let Some(base_url) = release_base_url_from_env(&version) { | |
| return ReleaseQuery::Mirror { base_url, version }; | |
| } |
| pub async fn latest_release_tag_async(channel: ReleaseChannel) -> Result<String> { | ||
| match resolve_release_query(channel) { | ||
| ReleaseQuery::Mirror { version, .. } => Ok(format!("v{}", version.trim_start_matches('v'))), | ||
| ReleaseQuery::GitHubLatest { url } => { | ||
| let body = fetch_release_json_async(url, "latest release").await?; | ||
| latest_tag_from_release_json(&body) | ||
| } | ||
| ReleaseQuery::GitHubReleaseList { url } => { | ||
| let body = fetch_release_json_async(url, "release list").await?; | ||
| latest_beta_tag_from_release_list_json(&body) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn latest_release_tag_blocking(channel: ReleaseChannel) -> Result<String> { | ||
| match resolve_release_query(channel) { | ||
| ReleaseQuery::Mirror { version, .. } => Ok(format!("v{}", version.trim_start_matches('v'))), | ||
| ReleaseQuery::GitHubLatest { url } => { | ||
| let body = fetch_release_json_blocking(url, "latest release")?; | ||
| latest_tag_from_release_json(&body) | ||
| } | ||
| ReleaseQuery::GitHubReleaseList { url } => { | ||
| let body = fetch_release_json_blocking(url, "release list")?; | ||
| latest_beta_tag_from_release_list_json(&body) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Update latest_release_tag_async and latest_release_tag_blocking to accept the current_version parameter and pass it down to resolve_release_query to avoid relying on the library's compile-time env!("CARGO_PKG_VERSION").
pub async fn latest_release_tag_async(channel: ReleaseChannel, current_version: &str) -> Result<String> {
match resolve_release_query(channel, current_version) {
ReleaseQuery::Mirror { version, .. } => Ok(format!("v{}", version.trim_start_matches('v'))),
ReleaseQuery::GitHubLatest { url } => {
let body = fetch_release_json_async(url, "latest release").await?;
latest_tag_from_release_json(&body)
}
ReleaseQuery::GitHubReleaseList { url } => {
let body = fetch_release_json_async(url, "release list").await?;
latest_beta_tag_from_release_list_json(&body)
}
}
}
pub fn latest_release_tag_blocking(channel: ReleaseChannel, current_version: &str) -> Result<String> {
match resolve_release_query(channel, current_version) {
ReleaseQuery::Mirror { version, .. } => Ok(format!("v{}", version.trim_start_matches('v'))),
ReleaseQuery::GitHubLatest { url } => {
let body = fetch_release_json_blocking(url, "latest release")?;
latest_tag_from_release_json(&body)
}
ReleaseQuery::GitHubReleaseList { url } => {
let body = fetch_release_json_blocking(url, "release list")?;
latest_beta_tag_from_release_list_json(&body)
}
}
}| let latest_tag = | ||
| latest_release_tag_blocking(channel).with_context(update_network_fallback_hint)?; |
There was a problem hiding this comment.
Pass the current_version parameter to latest_release_tag_blocking to align with the updated signature and avoid relying on the library's compile-time version.
| let latest_tag = | |
| latest_release_tag_blocking(channel).with_context(update_network_fallback_hint)?; | |
| let latest_tag = | |
| latest_release_tag_blocking(channel, current_version).with_context(update_network_fallback_hint)?; |
| match codewhale_release::latest_release_tag_async(codewhale_release::ReleaseChannel::Stable) | ||
| .await |
There was a problem hiding this comment.
Pass the current_version parameter to latest_release_tag_async to align with the updated signature and avoid relying on the library's compile-time version.
| match codewhale_release::latest_release_tag_async(codewhale_release::ReleaseChannel::Stable) | |
| .await | |
| match codewhale_release::latest_release_tag_async(codewhale_release::ReleaseChannel::Stable, current_version) | |
| .await |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn release_response_body( | ||
| status: reqwest::StatusCode, | ||
| body: Result<String>, | ||
| url: &str, | ||
| description: &str, | ||
| ) -> Result<String> { | ||
| let body = body.with_context(|| format!("failed to read {description} response from {url}"))?; |
There was a problem hiding this comment.
body result already has context applied at the call-site in both fetch_release_json_blocking and fetch_release_json_async before being passed in, so release_response_body wraps it a second time with the identical message. Under anyhow's error chain this will print "failed to read {description} response from {url}" twice on any body-read failure.
| fn release_response_body( | |
| status: reqwest::StatusCode, | |
| body: Result<String>, | |
| url: &str, | |
| description: &str, | |
| ) -> Result<String> { | |
| let body = body.with_context(|| format!("failed to read {description} response from {url}"))?; | |
| fn release_response_body( | |
| status: reqwest::StatusCode, | |
| body: Result<String>, | |
| url: &str, | |
| description: &str, | |
| ) -> Result<String> { | |
| let body = body?; |
| match compare_release_versions(current_version, &latest_tag)? { | ||
| Ordering::Greater => { | ||
| println!("Current build is newer than the latest published release."); | ||
| } | ||
| Ordering::Less | Ordering::Equal => { | ||
| println!("Already up to date."); | ||
| } | ||
| } |
There was a problem hiding this comment.
Beta and a mirror is configured, latest_release_tag_blocking returns the plain mirror version (e.g., v0.8.47) without a beta pre-release identifier. update_is_needed then returns false (because latest_is_beta is false), but compare_release_versions can still return Ordering::Less, producing the incorrect message "Already up to date." for a user who is actually behind. The regular run_update path handles this by printing a mirror+beta warning unconditionally; check_only has no equivalent guard.
|
hi @Hmbown please help to take a look when you have time. thanks. |
1 participant
Summary
Adds a non-destructive release check for CodeWhale.
This follows up on #1836 with a fresh implementation on current
main. The old PR added a usefulupdate --checkflow, but it became stale and conflicted with newer CodeWhale changes. This version keeps the change scoped to update/doctor behavior and avoids carrying over unrelated old diffs.This PR adds:
codewhale update --checkto report the current binary version and latest release without downloading or replacing binariesUpdatessection incodewhale doctorTesting
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-featurescargo test --workspace --all-featuresChecklist
Greptile Summary
This PR extracts shared release-discovery and version-comparison logic into a new
codewhale-releasecrate and builds two consumer features on top of it: acodewhale update --checkflag that reports the current/latest version without downloading anything, and an "Updates" section incodewhale doctorthat performs the same check asynchronously.crates/release/src/lib.rs— new crate withfetch_release_json_blocking/_async,latest_release_tag_*,update_is_needed,compare_release_versions, and all URL/env constants previously scattered acrossupdate.rs.crates/cli/src/update.rs—run_updategains acheck_onlybranch; most of the old boilerplate is removed and delegated to the release crate.crates/tui/src/main.rs—run_doctorgains an async "Updates" block usinglatest_release_tag_asyncwith graceful error handling and colored output.Confidence Score: 4/5
The change is well-scoped and the new crate cleanly centralises logic that was previously duplicated; the two issues found are confined to error-message formatting and an edge-case output label in the beta+mirror path.
The double
.with_context()wrapping inrelease_response_bodywill emit a repeated error message on any body-read failure, and theOrdering::Lessarm of the check-only fallback prints "Already up to date." when the user is actually behind (beta channel + mirror). Both are isolated to diagnostic output paths with no data-loss or security risk.crates/release/src/lib.rs (error context duplication) and crates/cli/src/update.rs (misleading check-only output for beta+mirror)
Important Files Changed
.with_context()inrelease_response_bodyduplicates error message on body-read failure.check_onlypath using shared release helpers; theOrdering::Lessarm of the fallback match incorrectly prints "Already up to date." when the beta+mirror edge case causesupdate_is_neededto return false despite being behind.codewhale doctorusing the new release crate; hardcodedReleaseChannel::Stableis intentional and error paths are handled gracefully.--checkflag toUpdateArgsand threads it through torun_update; CLI parsing tests updated correctly.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["codewhale update --check"] --> B["run_update(beta, check_only=true)"] B --> C["latest_release_tag_blocking(channel)"] C --> D{"resolve_release_query"} D -->|"Mirror"| E["return v{version} from env"] D -->|"GitHubLatest"| F["fetch_release_json_blocking"] D -->|"GitHubReleaseList"| G["fetch_release_json_blocking"] F --> H["latest_tag_from_release_json"] G --> I["latest_beta_tag_from_release_list_json"] E & H & I --> J{"update_is_needed"} J -->|"true"| K["print: Update available"] J -->|"false"| L{"compare_release_versions"} L -->|"Greater"| M["print: Current build is newer"] L -->|"Less or Equal"| N["print: Already up to date"] O["codewhale doctor"] --> P["latest_release_tag_async(Stable)"] P --> DReviews (1): Last reviewed commit: "feat(update): add check-only release dia..." | Re-trigger Greptile