Identify which CLI an upgrade warning is about

[Dione-b]

What problem does your feature solve?
The upgrade warning displays incorrect version information because of multiple stellar installations on the same machine. An older binary prints the warning using its own version, which users then incorrectly compare against stellar --version from a different binary. Additionally, fast commands abort the background version fetch before it can complete.

What would you like to see?

• The upgrade warning updated to name the specific running executable.
• The doctor command updated to list every stellar/soroban executable on the system PATH along with its version.
• A full request timeout for the fetch, and a 2-second grace period for the background check to persist before the process exits.
• The shared cache recording which installation last wrote to it, with a doctor warning if it was a different install.

What alternatives are there?
Accepting the known limitations of this implementation: the grace period is skipped on error paths that call process::exit; it may add up to 2 seconds to the first command of the day; and recording the cache writer is strictly diagnostic (it does not affect whether the warning is shown).

fix: #2464

[Copilot]

🟡 Not ready to approve

Doctor can report false installation mismatches, omit single-install details, and hang on unresponsive executables.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Improves upgrade diagnostics for systems with multiple Stellar CLI installations.

Changes:

• Identifies the executable responsible for upgrade warnings and cache writes.
• Expands doctor diagnostics to inspect PATH installations.
• Adds request and shutdown timeouts for background upgrade checks.

File summaries

File	Description
cmd/soroban-cli/src/upgrade_check.rs	Adds executable identification, cache attribution, and fetch timeout.
cmd/soroban-cli/src/config/upgrade_check.rs	Persists the cache writer with backward compatibility.
cmd/soroban-cli/src/commands/doctor.rs	Reports installations and cache-writer mismatches.
cmd/soroban-cli/src/cli.rs	Gives background checks a completion grace period.

Review details

Suppressed comments (1)

cmd/soroban-cli/src/commands/doctor.rs:242

• The usual single-install case returns before printing the executable's path and version, so doctor does not actually list every PATH installation as promised. The zero-match case is also reported as “Only one.” Always enumerate nonempty results and handle zero separately.

    if installs.len() <= 1 {
        print.checkln("Only one Stellar CLI found on PATH".to_string());
        return;

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

Some installation listings and cache-writer diagnostics are incomplete or misleading.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

cmd/soroban-cli/src/upgrade_check.rs:140

• This assigns last_checked_by after a failed request without refreshing either cached version, but doctor later tells users the cache was “last refreshed by” this executable. That can falsely attribute version data to an install that never fetched it. Keep this assignment if the intended identity is the last writer/pacer, but update the field documentation and all doctor messages to say “last checked/written by”; alternatively, record this field only after successful refreshes.

                // A failed attempt still paces the next one, so record who
                // paced it -- otherwise the file credits whichever install
                // last succeeded, which may not be the one holding it back.
                stats.last_checked_by = Some(check_performed_by());

cmd/soroban-cli/src/commands/doctor.rs:264

• The single-install branch does not call list_installs, so doctor omits that executable's PATH location and version. This contradicts the PR's stated behavior of listing every discovered stellar/soroban executable with its version; the separate “Running executable” line is not necessarily the PATH entry and has no version.

        (1, _) => print.checkln("Only one Stellar CLI found on PATH".to_string()),

cmd/soroban-cli/src/commands/doctor.rs:277

• common_version == None also means one or more probes returned None, not necessarily that known versions differ. For example, two executables that cannot run are both listed as “unknown version” while this branch claims they reported different versions. Distinguish probe failures from genuinely distinct known versions so the diagnostic does not give a false cause.

        (count, None) => {
            print.warnln(format!(
                "Found {count} Stellar CLI executables on PATH reporting different versions; \
                 an outdated one can report a version that disagrees with `stellar --version`:"
            ));

• Files reviewed: 6/6 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

cmd/soroban-cli/src/commands/doctor.rs:266

• A single discovered executable is always shown with a success marker, even when its version probe failed and the list immediately labels it unknown version. This makes a broken or non-Stellar executable look healthy, whereas the same failed probe correctly warns when multiple entries exist. Match InstalledVersions::Agreed for the success case and warn for Unanswered.

        // One file, so nothing can disagree with it. Still list it: the running
        // executable is not necessarily the one `PATH` resolves by name, and the
        // line above carries no version.
        (1, _) => print.checkln("Only one Stellar CLI found on PATH:".to_string()),

cmd/soroban-cli/src/upgrade_check.rs:48

• Using the canonical target as the installation identity is not stable across symlink-managed upgrades. For example, Homebrew keeps /opt/homebrew/bin/stellar as the user-facing install path but retargets it from one versioned Cellar path to another; this cache value therefore changes after a normal in-place upgrade and doctor reports a different CLI. Preserve or separately record a stable invoked/install path for identity, while retaining the resolved path only as diagnostic detail.

    let path = std::env::current_exe().ok()?;
    let path = path.canonicalize().unwrap_or(path);

cmd/soroban-cli/src/commands/doctor.rs:54

• This read can race with the ordinary upgrade-check task spawned in cli.rs:81 before root.run(). If the cache is stale and that task completes or fails quickly, it writes this executable as last_checked_by before this load, hiding the different-install warning that this snapshot is meant to preserve. Skip the global background check for doctor, or capture/pass the previous writer before spawning it.

This issue also appears on line 263 of the same file.

        // Read this before `check_version`, which refreshes the cache and would
        // otherwise record this very run as the writer -- hiding the mismatch
        // the report exists to reveal.
        let previous_cache_writer = version_cache_writer();