Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions cli/azd/cmd/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ func (a *toolInstallAction) resolveHostOptions(

// Explicitly-named skill: when multiple hosts are detected we cannot
// safely guess which the user wants.
present := a.manager.AvailableSkillHosts(skill)
present := a.manager.AvailableSkillHosts(ctx, skill)
if len(present) > 1 {
// Interactive terminal: prompt the user to pick the host(s),
// after surfacing the --host hint so they learn the shortcut too.
Expand Down Expand Up @@ -734,7 +734,7 @@ func (a *toolInstallAction) resolveUnavailableHostPrompt(
return nil, false, nil
}

available := a.manager.AvailableSkillHosts(skill)
available := a.manager.AvailableSkillHosts(ctx, skill)
var unavailable []string
for _, host := range a.flags.hosts {
if !slices.Contains(available, host) {
Expand Down
10 changes: 5 additions & 5 deletions cli/azd/cmd/tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ type cmdMockInstaller struct {
uninstall func(
ctx context.Context, t *tool.ToolDefinition, opts ...tool.InstallOption,
) (*tool.InstallResult, error)
availableSkillHosts func(t *tool.ToolDefinition) []string
availableSkillHosts func(ctx context.Context, t *tool.ToolDefinition) []string
}

func (m *cmdMockInstaller) Install(
Expand All @@ -371,9 +371,9 @@ func (m *cmdMockInstaller) Upgrade(
return &tool.InstallResult{Tool: t, Success: true}, nil
}

func (m *cmdMockInstaller) AvailableSkillHosts(t *tool.ToolDefinition) []string {
func (m *cmdMockInstaller) AvailableSkillHosts(ctx context.Context, t *tool.ToolDefinition) []string {
if m.availableSkillHosts != nil {
return m.availableSkillHosts(t)
return m.availableSkillHosts(ctx, t)
}
return nil
}
Expand Down Expand Up @@ -663,7 +663,7 @@ func TestResolveHostOptions(t *testing.T) {

newAction := func(args []string, flags *toolInstallFlags, present []string) *toolInstallAction {
installer := &cmdMockInstaller{
availableSkillHosts: func(_ *tool.ToolDefinition) []string {
availableSkillHosts: func(_ context.Context, _ *tool.ToolDefinition) []string {
return present
},
}
Expand Down Expand Up @@ -944,7 +944,7 @@ func TestResolveHostOptions_Upgrade(t *testing.T) {

newAction := func(flags *toolUpgradeFlags, present []string) *toolUpgradeAction {
installer := &cmdMockInstaller{
availableSkillHosts: func(_ *tool.ToolDefinition) []string {
availableSkillHosts: func(_ context.Context, _ *tool.ToolDefinition) []string {
return present
},
}
Expand Down
144 changes: 116 additions & 28 deletions cli/azd/pkg/tool/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,11 @@ type Installer interface {
) (*InstallResult, error)

// AvailableSkillHosts returns the names of the tool's configured
// SkillHosts that are currently on PATH, in manifest order
// (preferred host first). It returns nil for non-skill tools or
// when none of the hosts are available.
AvailableSkillHosts(tool *ToolDefinition) []string
// SkillHosts that are currently usable (a functional CLI on PATH, per
// hostUsable), in manifest order (preferred host first). It returns nil
// for non-skill tools or when none of the hosts are usable. It probes
// the host binaries, so it takes a context.
AvailableSkillHosts(ctx context.Context, tool *ToolDefinition) []string

// Uninstall removes the given tool from the current platform. For
// skill tools the optional [WithHosts] option restricts removal to
Expand Down Expand Up @@ -173,6 +174,13 @@ type installer struct {
httpClient httpDoer
platformMu sync.Mutex
platform *Platform // lazily populated by ensurePlatform
// hostProbe memoizes hostUsable's version-probe result per host for the
// installer's lifetime (one process == one command), so the several
// host-resolution call sites do not re-spawn `--version` for the same
// host. Only on-PATH results are stored (see hostUsable). Guarded by
// hostProbeMu.
hostProbeMu sync.Mutex
hostProbe map[string]bool
// retryBackoff is the initial wait between post-install detection
// retries (doubled each attempt). Defaults to 1s; tests may shorten
// it to keep the suite fast.
Expand Down Expand Up @@ -241,16 +249,19 @@ func (i *installer) Upgrade(
}

// AvailableSkillHosts returns the names of the tool's configured
// SkillHosts that are currently on PATH, in manifest order (preferred
// host first). It returns nil for non-skill tools or when none of the
// hosts are available.
func (i *installer) AvailableSkillHosts(tool *ToolDefinition) []string {
// SkillHosts that are currently usable, in manifest order (preferred host
// first). A host counts only if [installer.hostUsable] confirms it is a
// functional CLI — not merely a same-named launcher stub on PATH — so the
// interactive host picker never offers a host the install path would later
// reject. It returns nil for non-skill tools or when none of the hosts are
// usable.
func (i *installer) AvailableSkillHosts(ctx context.Context, tool *ToolDefinition) []string {
if tool.Category != ToolCategorySkill {
return nil
}
var present []string
for _, host := range tool.SkillHosts {
if i.commandRunner.ToolInPath(host.Host) == nil {
if i.hostUsable(ctx, host) {
present = append(present, host.Host)
}
}
Expand Down Expand Up @@ -499,7 +510,7 @@ func (i *installer) resolveSkillUninstallTargets(
hosts []string,
) ([]SkillHost, error) {
if len(hosts) > 0 {
return i.explicitSkillHostTargets(tool, hosts)
return i.explicitSkillHostTargets(ctx, tool, hosts)
}

// Default / --host all: remove from every host the skill is
Expand Down Expand Up @@ -917,13 +928,14 @@ func (i *installer) resolveSkillTargets(
if allAvailable {
var onPath []SkillHost
for _, host := range tool.SkillHosts {
if i.commandRunner.ToolInPath(host.Host) == nil {
if i.hostUsable(ctx, host) {
onPath = append(onPath, host)
}
Comment thread
hemarina marked this conversation as resolved.
}
// No host CLI present at all — surface the install guidance.
// No usable host CLI present at all — surface the install
// guidance.
if len(onPath) == 0 {
host, err := i.pickSkillHost(tool)
host, err := i.pickSkillHost(ctx, tool)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1007,35 +1019,37 @@ func (i *installer) resolveSkillTargets(
),
}
}
host, err := i.pickSkillHost(tool)
host, err := i.pickSkillHost(ctx, tool)
if err != nil {
return nil, err
}
return []SkillHost{host}, nil
}

return i.explicitSkillHostTargets(tool, hosts)
return i.explicitSkillHostTargets(ctx, tool, hosts)
}

// explicitSkillHostTargets resolves an explicit list of requested host
// names to their [SkillHost] definitions. A requested host is usable only
// when it is a configured SkillHost that is also on PATH; both an unknown
// name and a known-but-not-on-PATH host fail with an error naming the
// when it is a configured SkillHost that is also a functional CLI on PATH
// (see [installer.hostUsable]); an unknown name, a host not on PATH, and a
// host present only as a launcher stub all fail with an error naming the
// supported hosts. Shared by the install/upgrade (resolveSkillTargets) and
// uninstall (resolveSkillUninstallTargets) paths so the host-availability
// rule lives in one place.
func (i *installer) explicitSkillHostTargets(
ctx context.Context,
tool *ToolDefinition,
hosts []string,
) ([]SkillHost, error) {
targets := make([]SkillHost, 0, len(hosts))
for _, name := range hosts {
// A requested host is usable only if it is a configured SkillHost
// that is also on PATH. "unknown name" and "known but not on PATH"
// both mean the host can't be used, so we point the user at the
// supported hosts.
// that is a functional CLI on PATH. "unknown name", "not on PATH"
// and "present but a launcher stub" all mean the host can't be
// used, so we point the user at the supported hosts.
host, ok := findSkillHost(tool, name)
if !ok || i.commandRunner.ToolInPath(name) != nil {
if !ok || !i.hostUsable(ctx, host) {
supported := make([]string, len(tool.SkillHosts))
for j, h := range tool.SkillHosts {
supported[j] = h.Host
Expand Down Expand Up @@ -1078,18 +1092,92 @@ func configuredSkillHostsFor(tool *ToolDefinition, installed []InstalledSkillHos
return targets
}

// pickSkillHost returns the first SkillHost whose binary is on PATH.
// When none of the configured hosts is available it returns an
// [errorhandler.ErrorWithSuggestion] (all four fields populated per the
// AGENTS.md completeness rule) that recommends installing GitHub
// Copilot CLI via `azd tool install github-copilot-cli` — a single
// command the user can copy-paste without leaving azd.
// hostUsable reports whether an agentic CLI host is a *functional*
// installation rather than merely a file named like the host on PATH.
//
// Some environments place a launcher stub on PATH that is not the
// real CLI — most notably the VS Code GitHub Copilot Chat extension, which
// drops a small `copilot` stub into its globalStorage and adds that folder
// to the integrated terminal's PATH. Invoking the stub only prints
// "Install GitHub Copilot CLI?" and exits 0 without doing anything, so it
// satisfies a bare exec.LookPath existence check yet cannot install the
// skill — which previously surfaced as the misleading
// "<skill> was installed via <host> but verification failed".
//
// hostUsable shells out to the host's version command (non-interactively,
// with an empty stdin so a stub that prompts reads EOF and exits instead of
// hanging) and treats the host as usable only when the output matches the
// host's BinaryVersionRegex. That regex is anchored to the host's own
// `--version` banner (e.g. "^GitHub Copilot CLI <ver>"), so an incidental
// version-shaped token elsewhere in a stub's output does not count. Hosts
// that do not configure a version probe (BinaryVersionArgs/BinaryVersionRegex)
// fall back to the existence check.
//
// hostUsable is consulted from several host-resolution call sites within one
// command, so the version-probe result is memoized per host (hostProbe). Only
// on-PATH results are cached: a host that is not on PATH is re-checked every
// time (the exec.LookPath check is cheap) so a host installed earlier in the
// same batch — e.g. `azd tool install github-copilot-cli azure-skills` — is
// still picked up at run time. The cache assumes an on-PATH host binary is not
// swapped in place mid-command, which azd never does (it installs hosts to
// their own package locations).
func (i *installer) hostUsable(ctx context.Context, host SkillHost) bool {
Comment thread
hemarina marked this conversation as resolved.
if i.commandRunner.ToolInPath(host.Host) != nil {
return false
}

i.hostProbeMu.Lock()
if i.hostProbe == nil {
i.hostProbe = map[string]bool{}
}
usable, ok := i.hostProbe[host.Host]
i.hostProbeMu.Unlock()
if ok {
return usable
}

usable = i.probeOnPathHost(ctx, host)

i.hostProbeMu.Lock()
i.hostProbe[host.Host] = usable
i.hostProbeMu.Unlock()
return usable
}

// probeOnPathHost runs the version probe for a host already confirmed to be on
// PATH and reports whether it is a functional CLI. It is the uncached half of
// hostUsable; see that method for the rationale behind the matching.
func (i *installer) probeOnPathHost(ctx context.Context, host SkillHost) bool {
if len(host.BinaryVersionArgs) == 0 || host.BinaryVersionRegex == "" {
return true
}
result, err := i.commandRunner.Run(
ctx,
exec.NewRunArgs(host.Host, host.BinaryVersionArgs...).
WithStdIn(strings.NewReader("")),
)
// A cancelled/timed-out probe is not evidence the host is a stub; do
// not penalize it here (context handling is the caller's concern).
if isContextErr(err) {
return true
}
return matchVersion(result.Stdout+"\n"+result.Stderr, host.BinaryVersionRegex) != ""
}

// pickSkillHost returns the first SkillHost that is a usable (functional)
// CLI — see [installer.hostUsable], which rejects launcher stubs that merely
// share the host's name on PATH. When none of the configured hosts is usable
// it returns an [errorhandler.ErrorWithSuggestion] (all four fields populated
// per the AGENTS.md completeness rule) that recommends installing GitHub
// Copilot CLI via `azd tool install github-copilot-cli` — a single command
// the user can copy-paste without leaving azd.
func (i *installer) pickSkillHost(
ctx context.Context,
tool *ToolDefinition,
) (SkillHost, error) {
var checked []string
for _, host := range tool.SkillHosts {
if err := i.commandRunner.ToolInPath(host.Host); err == nil {
if i.hostUsable(ctx, host) {
return host, nil
}
checked = append(checked, host.Host)
Expand Down
Loading
Loading