Skip to content

feat(cli): check for newer GitHub release on every user-facing command#637

Merged
jamesadevine merged 3 commits into
mainfrom
copilot/add-update-prompt-to-compiler
May 19, 2026
Merged

feat(cli): check for newer GitHub release on every user-facing command#637
jamesadevine merged 3 commits into
mainfrom
copilot/add-update-prompt-to-compiler

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

Summary

Adds a non-blocking update check to ado-aw. On every user-facing command invocation the CLI spawns a concurrent background check against the GitHub Releases API (https://api.github.com/repos/githubnext/ado-aw/releases/latest) and prints a brief advisory to stderr when a newer version is available:

A newer version of ado-aw is available: v0.31.0 (you have v0.30.2).
Update at: https://github.com/githubnext/ado-aw/releases/latest

Changes

  • src/update_check.rs (new file) — check_for_update() async function that:
    • Fetches the latest release tag from GitHub API with a 5-second timeout
    • Strips the v prefix from the tag name and compares semver triples
    • Prints the advisory using a reconstructed maj.min.pat string (never the raw API response) to guard against terminal injection
    • Silently swallows all errors (logged at debug level) — a network hiccup never interrupts the user
  • src/main.rs — registers the new module and spawns check_for_update() concurrently via tokio::spawn before command dispatch; the handle is awaited just before main returns so the advisory (if any) always prints after the command finishes. The check is skipped for:
    • Pipeline-internal commands (execute, mcp, mcp-http) — run inside network-isolated sandboxes
    • CI environments (CI env var set) — avoids unnecessary outbound calls on every CI run

Test plan

  • 6 unit tests covering: newer patch, newer minor, newer major, same version, older version, and v-prefix stripping
  • All existing tests continue to pass; build and clippy are clean

Copilot AI and others added 2 commits May 19, 2026 08:24
@jamesadevine
Copy link
Copy Markdown
Collaborator

/rust-review

@jamesadevine jamesadevine marked this pull request as ready for review May 19, 2026 09:31
@jamesadevine
Copy link
Copy Markdown
Collaborator

/rust-review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

Rust PR Reviewer completed successfully!

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: One latency bug needs fixing before merge; two smaller concerns worth addressing.

Findings

🐛 Bugs / Logic Issues

  • src/main.rs:785 — The update check is awaited before command dispatch. On a slow network (or at the 5-second timeout), every user-facing command is silently delayed by up to 5 seconds. The fix is to spawn the check concurrently and let it race the actual command:

    let update_handle = if !is_pipeline_internal {
        Some(tokio::spawn(update_check::check_for_update()))
    } else {
        None
    };
    
    // ... match command { ... } dispatch ...
    
    if let Some(handle) = update_handle {
        let _ = handle.await;
    }

    This way the advisory prints (if needed) right after the command finishes rather than blocking it. The only scenario where output ordering matters—extremely fast commands where the update message might appear before the command's own output has flushed—is an acceptable trade-off, and you can mitigate it by moving the await to just before Ok(()).

🔒 Security Concerns

  • src/update_check.rs:28latest is the raw tag_name from the GitHub API, stripped of a leading v. It is printed verbatim in eprintln!. If the tag ever contained ANSI escape sequences (e.g. 0.31.0\x1b[31mRED), terminal injection would follow. Since parse() already validates that the version string is a numeric semver triple (returning false from is_newer otherwise), the simplest guard is to only print the reconstructed version from the parsed tuple rather than the raw string:

    if let (true, Some((maj, min, pat))) = (is_newer(latest, CURRENT_VERSION), parse_version(latest)) {
        eprintln!("A newer version of ado-aw is available: v{maj}.{min}.{pat} ...");
    }

    Low-severity given this repo's tag names are under team control, but cheap to fix.

⚠️ Suggestions

  • src/main.rs:784 — The check fires in CI environments (GitHub Actions sets CI=true). This is harmless but adds an unnecessary outbound network call on every CI run. A quick guard:
    if !is_pipeline_internal && std::env::var_os("CI").is_none() {
        ...
    }

✅ What Looks Good

  • Errors are silently absorbed (log::debug!), so a network failure never breaks the user's workflow — exactly the right pattern for a best-effort advisory.
  • The 5-second timeout is a sensible ceiling.
  • is_newer correctly handles pre-release suffixes via the split(|c| !c.is_ascii_digit()) trick, and the unit tests cover all six relevant cases.
  • Skipping execute / mcp / mcp-http for the network-isolated pipeline sandbox is the right call.

Generated by Rust PR Reviewer for issue #637 · ● 238.4K ·

@jamesadevine
Copy link
Copy Markdown
Collaborator

@copilot implement feedback and suggestions

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 19, 2026

@copilot implement feedback and suggestions

All three findings addressed in b6cbea1:

  • Latency bug — update check is now spawned with tokio::spawn before command dispatch and awaited just before Ok(()), so it runs concurrently with the command rather than blocking it.
  • Terminal injection — we now print the version reconstructed from the parsed (maj, min, pat) tuple instead of the raw API tag_name, so malformed/escaped content in the API response can never reach the terminal.
  • CI skip — check is suppressed when CI env var is set, avoiding the unnecessary outbound call on every CI run.

@jamesadevine jamesadevine merged commit 7d197f1 into main May 19, 2026
15 checks passed
@jamesadevine jamesadevine deleted the copilot/add-update-prompt-to-compiler branch May 19, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants