Skip to content

Commit 98d728c

Browse files
authored
fix(gl): point doctor's version check at Gitlawb/node, not the frozen Gitlawb/releases repo (#198)
* fix(gl): point doctor's version check at Gitlawb/node, not the frozen Gitlawb/releases repo check_version queried https://api.github.com/repos/gitlawb/releases/releases/latest. Gitlawb/releases is a separate repo that stopped receiving tags after v0.3.8 (2026-03-30). Releases have shipped directly on Gitlawb/node since v0.4.0, via release-please, and install.sh already treats Gitlawb/node as the canonical release source. The old query target meant gl doctor's version check could never again detect a real update. Points the check at Gitlawb/node instead, matching install.sh's own default, and makes the GitHub API base injectable so the fix is covered by a test against a mocked server rather than the real GitHub API. Closes #197 * fix(gl): check HTTP status before parsing the release response; strengthen up-to-date test Addresses CodeRabbit review on #198: - check_version now checks resp.status().is_success() before parsing the body, so a GitHub rate-limit or other API error surfaces as its actual HTTP status instead of a generic "could not parse latest tag". - test_check_version_up_to_date now also asserts the detail message, so the test can't silently pass on an unmatched mock request (mockito's fallback for an unhandled request is a 501, which check_version also maps to Ok). * test(gl): add unit test for non-2xx check_version response * fix(gl): remove trailing blank line at EOF in doctor.rs cargo fmt --all -- --check fails on the extra terminal blank line, and the fmt + clippy workflow runs that check as a required gate.
1 parent b6877a6 commit 98d728c

1 file changed

Lines changed: 61 additions & 4 deletions

File tree

‎crates/gl/src/doctor.rs‎

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::path::PathBuf;
1616
use crate::http::NodeClient;
1717

1818
const PUBLIC_NODE: &str = "https://node.gitlawb.com";
19+
const GITHUB_API_BASE: &str = "https://api.github.com";
1920

2021
#[derive(Args)]
2122
pub struct DoctorArgs {
@@ -263,7 +264,7 @@ pub async fn run(args: DoctorArgs) -> Result<()> {
263264

264265
// ── 7. Version up to date ─────────────────────────────────────────────
265266
let current = env!("CARGO_PKG_VERSION");
266-
checks.push(check_version(current).await);
267+
checks.push(check_version(current, GITHUB_API_BASE).await);
267268

268269
// ── Render ────────────────────────────────────────────────────────────
269270
for check in &checks {
@@ -319,8 +320,9 @@ fn which_in_path(name: &str) -> bool {
319320
.unwrap_or(false)
320321
}
321322

322-
/// Fetch the latest release tag from gitlawb/releases and compare to current version.
323-
async fn check_version(current: &'static str) -> Check {
323+
/// Fetch the latest release tag from Gitlawb/node (the actual release repo — see install.sh's
324+
/// `GITLAWB_RELEASE_REPO` default) and compare to current version.
325+
async fn check_version(current: &'static str, github_api_base: &str) -> Check {
324326
let client = match reqwest::Client::builder()
325327
.user_agent(format!("gl/{current}"))
326328
.timeout(std::time::Duration::from_secs(5))
@@ -331,14 +333,23 @@ async fn check_version(current: &'static str) -> Check {
331333
};
332334

333335
let resp = match client
334-
.get("https://api.github.com/repos/gitlawb/releases/releases/latest")
336+
.get(format!(
337+
"{github_api_base}/repos/Gitlawb/node/releases/latest"
338+
))
335339
.send()
336340
.await
337341
{
338342
Ok(r) => r,
339343
Err(_) => return Check::pass("version", format!("v{current} (offline — could not check)")),
340344
};
341345

346+
if !resp.status().is_success() {
347+
return Check::pass(
348+
"version",
349+
format!("v{current} (GitHub API returned HTTP {})", resp.status()),
350+
);
351+
}
352+
342353
let body: serde_json::Value = match resp.json().await {
343354
Ok(v) => v,
344355
Err(_) => return Check::pass("version", format!("v{current} (invalid response)")),
@@ -417,4 +428,50 @@ mod tests {
417428
fn test_which_in_path_missing_binary() {
418429
assert!(!which_in_path("this-binary-does-not-exist-gl-test-12345"));
419430
}
431+
432+
#[tokio::test]
433+
async fn test_check_version_queries_gitlawb_node_releases() {
434+
let mut server = mockito::Server::new_async().await;
435+
let _m = server
436+
.mock("GET", "/repos/Gitlawb/node/releases/latest")
437+
.with_status(200)
438+
.with_header("content-type", "application/json")
439+
.with_body(r#"{"tag_name":"v9.9.9"}"#)
440+
.create_async()
441+
.await;
442+
443+
let check = check_version("0.1.0", &server.url()).await;
444+
assert!(matches!(check.state, CheckState::Warn));
445+
assert!(check.detail.contains("v9.9.9"));
446+
}
447+
448+
#[tokio::test]
449+
async fn test_check_version_up_to_date() {
450+
let mut server = mockito::Server::new_async().await;
451+
let _m = server
452+
.mock("GET", "/repos/Gitlawb/node/releases/latest")
453+
.with_status(200)
454+
.with_header("content-type", "application/json")
455+
.with_body(r#"{"tag_name":"v0.1.0"}"#)
456+
.create_async()
457+
.await;
458+
459+
let check = check_version("0.1.0", &server.url()).await;
460+
assert!(matches!(check.state, CheckState::Ok));
461+
assert!(check.detail.contains("up to date"));
462+
}
463+
464+
#[tokio::test]
465+
async fn test_check_version_http_error() {
466+
let mut server = mockito::Server::new_async().await;
467+
let _m = server
468+
.mock("GET", "/repos/Gitlawb/node/releases/latest")
469+
.with_status(403)
470+
.create_async()
471+
.await;
472+
473+
let check = check_version("0.1.0", &server.url()).await;
474+
assert!(matches!(check.state, CheckState::Ok));
475+
assert!(check.detail.contains("GitHub API returned HTTP 403"));
476+
}
420477
}

0 commit comments

Comments
 (0)