Skip to content

feat: migrate self-update logic to kaishin v0.2.3 #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yukimemi merged 1 commit into from
May 9, 2026
Merged

feat: migrate self-update logic to kaishin v0.2.3 #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .kata/applied.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
preset = "github.com/yukimemi/pj-presets:rust-cli"
applied_at = "2026-05-05T16:39:36.2177367Z"
applied_at = "2026-05-07T11:30:27.3001733Z"

[[templates]]
source = "github.com/yukimemi/pj-base"
Expand Down
38 changes: 35 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 2 additions & 14 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,10 @@ dirs = "6"
whoami = "2.1.2"
serde_json = "1.0.149"
owo-colors = "4.3.0"
kaishin = "0.2"
tokio = { version = "1", features = ["rt", "macros"] }
semver = "1.0.28"
tempfile = "3.27.0"
humantime = "2.3.0"
reqwest = { version = "0.13.3", default-features = false, features = [
"blocking",
"json",
"rustls",
] }
self_update = { version = "0.44", default-features = false, features = [
"reqwest",
"rustls",
"compression-flate2",
"archive-tar",
"archive-zip",
"compression-zip-deflate",
] }

[dev-dependencies]
pretty_assertions = "1.4.1"
94 changes: 37 additions & 57 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,15 @@ fn main() -> Result<()> {
enum AutoUpdateHandle {
/// A newer version was found in the local cache from a previous run.
CachedAvailable {
current: String,
latest: updater::LatestRelease,
checker: updater::Checker,
latest: kaishin::LatestRelease,
},
/// A background check is currently in progress.
Pending {
current: String,
rx: std::sync::mpsc::Receiver<Result<updater::LatestRelease>>,
checker: updater::Checker,
rx: std::sync::mpsc::Receiver<Result<kaishin::LatestRelease>>,
/// A newer version already known from the local cache.
cached_latest: Option<kaishin::LatestRelease>,
},
}

Expand All @@ -287,78 +289,56 @@ fn maybe_spawn_auto_update_check() -> Option<AutoUpdateHandle> {
return None;
}

let state = updater::load_check_state();
let now = std::time::SystemTime::now();
let current = env!("CARGO_PKG_VERSION").to_string();
let checker = updater::Checker::new().ok()?;
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If updater::Checker::new() is refactored to be infallible (by moving the runtime creation), this line can be simplified.

Suggested change
let checker = updater::Checker::new().ok()?;
let checker = updater::Checker::new();

let interval = loaded
.config
.ui
.update_check_interval
.as_deref()
.and_then(|s| kaishin::parse_interval(s).ok())
.unwrap_or_else(updater::default_interval);

let interval = match loaded.config.ui.update_check_interval.as_deref() {
Some(s) => match humantime::parse_duration(s) {
Ok(d) => d,
Err(e) => {
tracing::warn!(value = %s, error = %e, "invalid ui.update_check_interval; using default");
updater::default_interval()
}
},
None => updater::default_interval(),
};
let checker = checker.interval(interval);

if !updater::should_auto_check(state.as_ref(), interval, now) {
if let Some(state) = state {
if let Some(cached_tag) = state.last_known_latest.as_ref() {
if updater::is_update_available(&current, cached_tag).unwrap_or(false) {
return Some(AutoUpdateHandle::CachedAvailable {
current,
latest: updater::LatestRelease {
tag_name: cached_tag.clone(),
html_url: state.last_known_url.unwrap_or_default(),
},
});
}
}
if !checker.should_check() {
if let Some(latest) = checker.cached_update() {
return Some(AutoUpdateHandle::CachedAvailable { checker, latest });
}
return None;
}

let cached_latest = checker.cached_update();
let (tx, rx) = std::sync::mpsc::channel();
let checker_clone = updater::Checker::new().ok()?.interval(interval);
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This line creates a second Checker instance just to move it into the background thread. If updater::Checker is refactored to be lightweight and implement Clone, you could simply clone the existing instance. Even without Clone, recreating it is much cheaper if it doesn't initialize a tokio runtime.

Suggested change
let checker_clone = updater::Checker::new().ok()?.interval(interval);
let checker_clone = updater::Checker::new().interval(interval);

std::thread::spawn(move || {
let _ = tx.send(updater::check_latest_release());
let _ = tx.send(checker_clone.check_and_save());
});

Some(AutoUpdateHandle::Pending { current, rx })
Some(AutoUpdateHandle::Pending {
checker,
rx,
cached_latest,
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/// Waits for the background update check to complete (with a short timeout) and prints a banner if an update is available.
fn finalize_auto_update_check(handle: AutoUpdateHandle) {
match handle {
AutoUpdateHandle::CachedAvailable { current, latest } => {
eprintln!("\n{}", updater::format_update_banner(&current, &latest));
AutoUpdateHandle::CachedAvailable { checker, latest } => {
eprintln!("\n{}", checker.format_banner(&latest));
}
AutoUpdateHandle::Pending { current, rx } => {
AutoUpdateHandle::Pending {
checker,
rx,
cached_latest,
} => {
// Wait for 1 second.
let res = rx.recv_timeout(std::time::Duration::from_secs(1));
let now_unix = std::time::SystemTime::now()
.duration_since(std::time::SystemTime::UNIX_EPOCH)
.map(|d| d.as_secs())
.unwrap_or(0);

let mut state = updater::load_check_state().unwrap_or(updater::UpdateCheckState {
last_checked_unix: 0,
last_known_latest: None,
last_known_url: None,
});

state.last_checked_unix = now_unix;

if let Ok(Ok(latest)) = res {
state.last_known_latest = Some(latest.tag_name.clone());
state.last_known_url = Some(latest.html_url.clone());
let _ = updater::save_check_state(&state);
if updater::is_update_available(&current, &latest.tag_name).unwrap_or(false) {
eprintln!("\n{}", updater::format_update_banner(&current, &latest));
}
} else {
// Even on timeout or error, update the last_checked_unix to avoid constant checking.
let _ = updater::save_check_state(&state);
eprintln!("\n{}", checker.format_banner(&latest));
} else if let Some(latest) = cached_latest {
// Fallback to cached version on timeout or fetch error.
eprintln!("\n{}", checker.format_banner(&latest));
}
}
Comment on lines 336 to 343
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a regression in the update check state management. The previous implementation (lines 339-362 in the old version) ensured that the "last checked" timestamp was updated and saved even if the background check timed out or failed. This prevented the tool from attempting a check on every subsequent run in slow network conditions. The new implementation removes this logic, which may lead to repeated check attempts if the 1-second timeout is frequently hit.

}
Expand Down
Loading
Loading