Skip to content

Expose install_bundled_cli and HAS_BUNDLED_CLI in the Rust SDK#1489

Merged
SteveSandersonMS merged 1 commit into
mainfrom
tclem/public-bundled-cli-install
May 29, 2026
Merged

Expose install_bundled_cli and HAS_BUNDLED_CLI in the Rust SDK#1489
SteveSandersonMS merged 1 commit into
mainfrom
tclem/public-bundled-cli-install

Conversation

@tclem
Copy link
Copy Markdown
Member

@tclem tclem commented May 29, 2026

github-app's health panel (and any caller that always sets CliProgram::Path(...)) needs the bundled CLI's path before a Client exists, but embeddedcli::path() is pub(crate). github-app has had to reinvent the cache-path convention in its own health module, making four copies (build.rs, embeddedcli.rs, resolve.rs, and the consumer) that have to stay byte-for-byte in sync or runtime resolution silently breaks.

This lifts the existing extraction path to a stable public surface without adding new logic:

pub const HAS_BUNDLED_CLI: bool;
pub fn install_bundled_cli() -> Option<PathBuf>;

install_bundled_cli() wraps embeddedcli::path() and returns the same path Client::start resolves to for CliProgram::Resolve with no COPILOT_CLI_PATH and no ClientOptions::bundled_cli_extract_dir override. Lazy on first call, idempotent thereafter, None when the bundled-cli feature is off or the target isn't supported by build.rs.

Notes for reviewers

  • No fallback to the dev-cache path. install_bundled_cli() returns None rather than chaining into the has_extracted_cli (bundled-cli off) branch. That's a separate question and callers can still reach it via CliProgram::Resolve. Consumer asked for this shape explicitly.
  • HAS_BUNDLED_CLI keys off cfg(has_bundled_cli), not the feature flag. So it stays accurate on unsupported targets even when the feature is on, and callers can branch on bundling presence without forcing extraction.
  • install_at stays pub(crate). No consumer needs the extract-dir variant today. If one shows up we can add install_bundled_cli_at(&Path) later.
  • resolve.rs is untouched. Client::start still goes through embeddedcli::path() directly; the public wrapper is just another caller of the same function.

Companion consumer change is github/github-app#5834, which replaces its hand-rolled find_bundled_copilot_path plus the duplicated sdk_cli_cache_root / sanitize_cli_version / COPILOT_CLI_EXTRACT_DIR_ENV helpers with this single SDK call.

  Generated via Copilot (Claude Opus 4.7) on behalf of @tclem

Lift embeddedcli::path() to a stable public surface so health checks,
diagnostics, and version probes can reach the bundled CLI without
spinning up a Client. Returns the same path Client::start resolves to
for CliProgram::Resolve with no COPILOT_CLI_PATH or extract_dir
override. Returns None when bundled-cli is off or the target is
unsupported (no fallback to the dev-cache path).

HAS_BUNDLED_CLI is a const so callers can branch on bundling presence
without forcing extraction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 29, 2026 03:08
@tclem tclem requested a review from a team as a code owner May 29, 2026 03:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Exposes a stable public surface (HAS_BUNDLED_CLI constant and install_bundled_cli() function) at the Rust SDK crate root, so consumers like github-app can reach the bundled Copilot CLI path without constructing a Client, eliminating duplicated cache-path logic in downstream crates.

Changes:

  • Add pub const HAS_BUNDLED_CLI (keyed on cfg(has_bundled_cli)) and pub fn install_bundled_cli() as a thin wrapper over the existing pub(crate) embeddedcli::path().
  • Document the new surface in the README under "Embedded CLI resolution".
  • Add integration tests covering both bundled (idempotent, matches resolver) and non-bundled (None, no dev-cache fallback) configurations.
Show a summary per file
File Description
rust/src/lib.rs Adds public HAS_BUNDLED_CLI const and install_bundled_cli() wrapper around the crate-private embedded CLI installer.
rust/README.md Documents the new "reaching the bundled binary without a Client" pattern with a runnable snippet.
rust/tests/cli_resolution_test.rs Adds three feature/cfg-gated tests: idempotent extraction, parity with Client::start resolver, and None when not bundled.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

@SteveSandersonMS SteveSandersonMS added this pull request to the merge queue May 29, 2026
Merged via the queue into main with commit 4463df4 May 29, 2026
21 checks passed
@SteveSandersonMS SteveSandersonMS deleted the tclem/public-bundled-cli-install branch May 29, 2026 09:01
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.

3 participants