STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add skills specific telemetry

Browse source 
* Add skills specific telemetry

* Remove VisibilityFuture, inline goroutine at call sites

The VisibilityFuture/FetchRepoVisibilityAsync/Wait wrapper was an
unidiomatic async abstraction built for a single pattern used in
exactly two call sites. In Go the channel is already the future;
wrapping it in a struct with a Wait(timeout) method adds no value.

Delete the abstraction and inline a local visResult struct,
buffered channel, goroutine, and select at each call site. Behavior
is preserved exactly: err -> "unknown", timeout -> "unknown",
success+public -> include skill_names.

FetchRepoVisibility (synchronous) is kept as-is.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix nonsense copilot tests

* Update telemetry tests for public-only dims and search event removal

Production telemetry emission changed:
- preview: skill_owner/skill_repo/skill_name (renamed from skill_names)
  are now emitted only when repo_visibility=public.
- install: skill_owner/skill_repo/skill_names are now emitted only
  when repo_visibility=public.
- search: the initial skill_search event was removed entirely; the
  skill_search_install event no longer carries query/owner dims.

Update tests to match: rename skill_names -> skill_name in preview,
make owner/repo assertions conditional on public visibility in both
preview and install, and reduce the search test to a single event
with explicit Empty assertions for the removed query/owner dims so a
privacy regression cannot pass silently.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Test CategorizeHost and switch telemetry to skill_host_type

Add TestCategorizeHost covering all four classification branches
(github.com, ghes, tenancy, uncategorized) with cases verified
against the real ghauth implementation rather than guessed.

Update install and preview unit tests to assert the new
skill_host_type dimension name, and fix a typo in the preview
acceptance txtar (skill_hos_type -> skill_host_type).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Shrink visibility wait and test unknown visibility

The 2s visibilityWaitTimeout was wildly overprovisioned: by the time
telemetry emission reaches the select, the command has already done
several serial GitHub REST calls (and for install, a git sparse-checkout
plus possibly interactive prompts), so the one-call visibility fetch
has almost always completed. Drop the timeout to 200ms — a short safety
net for a stalled REST call, not a wait budget for a healthy one.

Also adds a table-driven case to TestFetchRepoVisibility covering an
unknown/future visibility value from the API, addressing @babakks'
review nitpick.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
William Martin 2026-04-17 19:58:59 +02:00 • committed by GitHub
parent 451d399eac
commit 998b6212b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 896 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

13
acceptance/testdata/skills/skills-install.txtar vendored
View file

@ -18,3 +18,16 @@ stdout 'Installed git-commit'
# Verify the skill was written to the custom directory
exists $WORK/custom-skills/git-commit/SKILL.md
grep 'github-repo' $WORK/custom-skills/git-commit/SKILL.md
# Telemetry: skill_install event records agent hosts, repo identifiers,
# and (for a public repo) the installed skill name.
env GH_PRIVATE_ENABLE_TELEMETRY=1
env GH_TELEMETRY=log
env GH_TELEMETRY_SAMPLE_RATE=100
exec gh skill install github/awesome-copilot git-commit --scope user --force --agent github-copilot
stderr 'Telemetry payload:'
stderr '"type": "skill_install"'
stderr '"agent_hosts": "github-copilot"'
stderr '"skill_host_type": "github.com"'
stderr '"skill_owner": "github"'
stderr '"skill_repo": "awesome-copilot"'

12
acceptance/testdata/skills/skills-preview.txtar vendored
View file

@ -7,3 +7,15 @@ stdout 'git-commit/'
# Preview a skill that doesn't exist should error
! exec gh skill preview github/awesome-copilot nonexistent-skill-xyz
stderr 'not found'
# Telemetry: skill_preview event records repo identifiers and, for a
# public repo, the skill name.
env GH_PRIVATE_ENABLE_TELEMETRY=1
env GH_TELEMETRY=log
env GH_TELEMETRY_SAMPLE_RATE=100
exec gh skill preview github/awesome-copilot git-commit
stderr 'Telemetry payload:'
stderr '"type": "skill_preview"'
stderr '"skill_host_type": "github.com"'
stderr '"skill_owner": "github"'
stderr '"skill_repo": "awesome-copilot"'

4
acceptance/testdata/skills/skills-publish-dry-run.txtar vendored
View file

@ -6,10 +6,6 @@ stderr 'no skills found in'
exec gh skill publish --dry-run $WORK/test-repo
stdout 'hello-world'
# Validate alias should work identically
exec gh skill validate --dry-run $WORK/test-repo
stdout 'hello-world'
# Publish dry-run with --tag
exec gh skill publish --dry-run --tag v1.0.0 $WORK/test-repo
stdout 'hello-world'

2
acceptance/testdata/skills/skills-search-page.txtar vendored
View file

@ -1,3 +1,3 @@
# Pagination returns results on page 2
exec gh skill search copilot --page 2
exec gh skill search --owner github copilot --page 2
stdout 'copilot'

4
acceptance/testdata/skills/skills-search.txtar vendored
View file

@ -1,5 +1,5 @@
# Search for skills matching a query
exec gh skill search copilot
exec gh skill search --owner github copilot
stdout 'copilot'
# Search with JSON output
@ -9,4 +9,4 @@ stdout '"repo"'
# Search with a short query should error
! exec gh skill search a
stderr 'at least'
stderr 'at least'

16
internal/ghinstance/host.go
View file

@ -96,3 +96,19 @@ func HostPrefix(hostname string) string {
}
return fmt.Sprintf("https://%s/", hostname)
}
func CategorizeHost(host string) string {
if host == defaultHostname {
return "github.com"
}
if ghauth.IsEnterprise(host) {
return "ghes"
}
if ghauth.IsTenancy(host) {
return "tenancy"
}
return "uncategorized"
}

54
internal/ghinstance/host_test.go
View file

@ -157,3 +157,57 @@ func TestRESTPrefix(t *testing.T) {
})
}
}
func TestCategorizeHost(t *testing.T) {
tests := []struct {
name string
host string
want string
}{
{
name: "github.com returns github.com",
host: "github.com",
want: "github.com",
},
{
name: "classic GHES hostname returns ghes",
host: "ghe.io",
want: "ghes",
},
{
name: "arbitrary enterprise hostname returns ghes",
host: "enterprise.example.com",
want: "ghes",
},
{
name: "tenant subdomain of ghe.com returns tenancy",
host: "tenant.ghe.com",
want: "tenancy",
},
{
name: "api subdomain under tenant returns tenancy",
host: "api.tenant.ghe.com",
want: "tenancy",
},
{
name: "bare ghe.com returns ghes",
host: "ghe.com",
want: "ghes",
},
{
name: "github.localhost returns uncategorized",
host: "github.localhost",
want: "uncategorized",
},
{
name: "github.com subdomain returns uncategorized",
host: "garage.github.com",
want: "uncategorized",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, CategorizeHost(tt.host))
})
}
}

71
internal/skills/discovery/discovery.go
View file

@ -126,18 +126,37 @@ type treeResponse struct {
Truncated bool `json:"truncated"`
}
type blobResponse struct {
SHA string `json:"sha"`
Content string `json:"content"`
Encoding string `json:"encoding"`
type RepoVisibility string
const (
RepoVisibilityPublic RepoVisibility = "public"
RepoVisibilityPrivate RepoVisibility = "private"
RepoVisibilityInternal RepoVisibility = "internal"
)
func parseRepoVisibility(s string) (RepoVisibility, error) {
switch s {
case "public":
return RepoVisibilityPublic, nil
case "private":
return RepoVisibilityPrivate, nil
case "internal":
return RepoVisibilityInternal, nil
default:
return "", fmt.Errorf("unknown repository visibility: %q", s)
}
}
type releaseResponse struct {
TagName string `json:"tag_name"`
}
type repoResponse struct {
DefaultBranch string `json:"default_branch"`
// FetchRepoVisibility returns the repository visibility: "public", "private", or "internal".
func FetchRepoVisibility(client *api.Client, host, owner, repo string) (RepoVisibility, error) {
apiPath := fmt.Sprintf("repos/%s/%s", url.PathEscape(owner), url.PathEscape(repo))
var resp struct {
Visibility string `json:"visibility"`
}
if err := client.REST(host, "GET", apiPath, nil, &resp); err != nil {
return "", err
}
return parseRepoVisibility(resp.Visibility)
}
// ResolveRef determines the git ref to use for a given owner/repo.
@ -266,8 +285,10 @@ func (e *noReleasesError) Error() string { return e.reason }
func resolveLatestRelease(client *api.Client, host, owner, repo string) (*ResolvedRef, error) {
apiPath := fmt.Sprintf("repos/%s/%s/releases/latest", url.PathEscape(owner), url.PathEscape(repo))
var release releaseResponse
if err := client.REST(host, "GET", apiPath, nil, &release); err != nil {
var resp struct {
TagName string `json:"tag_name"`
}
if err := client.REST(host, "GET", apiPath, nil, &resp); err != nil {
// A 404 means the repository has no releases. This is the
// only case where falling back to the default branch is safe.
// Any other HTTP error (403, 500, …) or network failure is
@ -278,19 +299,21 @@ func resolveLatestRelease(client *api.Client, host, owner, repo string) (*Resolv
}
return nil, fmt.Errorf("could not fetch latest release: %w", err)
}
if release.TagName == "" {
if resp.TagName == "" {
return nil, &noReleasesError{reason: "latest release has no tag"}
}
return resolveTagRef(client, host, owner, repo, release.TagName)
return resolveTagRef(client, host, owner, repo, resp.TagName)
}
func resolveDefaultBranch(client *api.Client, host, owner, repo string) (*ResolvedRef, error) {
apiPath := fmt.Sprintf("repos/%s/%s", url.PathEscape(owner), url.PathEscape(repo))
var repoResp repoResponse
if err := client.REST(host, "GET", apiPath, nil, &repoResp); err != nil {
var resp struct {
DefaultBranch string `json:"default_branch"`
}
if err := client.REST(host, "GET", apiPath, nil, &resp); err != nil {
return nil, fmt.Errorf("could not determine default branch: %w", err)
}
branch := repoResp.DefaultBranch
branch := resp.DefaultBranch
if branch == "" {
return nil, fmt.Errorf("could not determine default branch for %s/%s", owner, repo)
}
@ -657,18 +680,22 @@ func walkTree(client *api.Client, host, owner, repo, sha, prefix string, depth i
// FetchBlob retrieves the content of a blob by SHA.
func FetchBlob(client *api.Client, host, owner, repo, sha string) (string, error) {
apiPath := fmt.Sprintf("repos/%s/%s/git/blobs/%s", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha))
var blob blobResponse
if err := client.REST(host, "GET", apiPath, nil, &blob); err != nil {
var resp struct {
SHA string `json:"sha"`
Content string `json:"content"`
Encoding string `json:"encoding"`
}
if err := client.REST(host, "GET", apiPath, nil, &resp); err != nil {
return "", fmt.Errorf("could not fetch blob: %w", err)
}
if blob.Encoding != "base64" {
return "", fmt.Errorf("unexpected blob encoding: %s", blob.Encoding)
if resp.Encoding != "base64" {
return "", fmt.Errorf("unexpected blob encoding: %s", resp.Encoding)
}
// GitHub API returns base64 with embedded newlines; use the StdEncoding
// decoder via a reader to handle them transparently.
decoded, err := io.ReadAll(base64.NewDecoder(base64.StdEncoding, strings.NewReader(blob.Content)))
decoded, err := io.ReadAll(base64.NewDecoder(base64.StdEncoding, strings.NewReader(resp.Content)))
if err != nil {
return "", fmt.Errorf("could not decode blob content: %w", err)
}

80
internal/skills/discovery/discovery_test.go
View file

@ -561,6 +561,86 @@ func TestFetchBlob(t *testing.T) {
}
}
func TestFetchRepoVisibility(t *testing.T) {
tests := []struct {
name string
stubs func(*httpmock.Registry)
want RepoVisibility
wantErr string
}{
{
name: "public repo",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": "public",
}))
},
want: RepoVisibilityPublic,
},
{
name: "private repo",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": "private",
}))
},
want: RepoVisibilityPrivate,
},
{
name: "internal repo",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": "internal",
}))
},
want: RepoVisibilityInternal,
},
{
name: "unknown visibility",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": "cool-visibility",
}))
},
wantErr: `unknown repository visibility: "cool-visibility"`,
},
{
name: "API error",
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.StatusStringResponse(500, "server error"))
},
wantErr: "HTTP 500",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.stubs(reg)
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
got, err := FetchRepoVisibility(client, "github.com", "monalisa", "octocat-skills")
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}
func TestDiscoverSkills(t *testing.T) {
tests := []struct {
name string

20
internal/telemetry/fake.go
View file

@ -13,3 +13,23 @@ func (r *EventRecorderSpy) Record(event ghtelemetry.Event) {
func (r *EventRecorderSpy) Disable() {}
func (r *EventRecorderSpy) Flush() {}
// CommandRecorderSpy is a test double for ghtelemetry.CommandRecorder.
// It captures recorded events and the most recent SetSampleRate call so tests can
// assert on the sampling behavior commands attempt to configure.
type CommandRecorderSpy struct {
Events []ghtelemetry.Event
LastSampleRate int
}
func (r *CommandRecorderSpy) Record(event ghtelemetry.Event) {
r.Events = append(r.Events, event)
}
func (r *CommandRecorderSpy) Disable() {}
func (r *CommandRecorderSpy) SetSampleRate(rate int) {
r.LastSampleRate = rate
}
func (r *CommandRecorderSpy) Flush() {}

2
pkg/cmd/root/root.go
View file

@ -149,7 +149,7 @@ func NewCmdRoot(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, versi
cmd.AddCommand(codespaceCmd.NewCmdCodespace(f))
cmd.AddCommand(projectCmd.NewCmdProject(f))
cmd.AddCommand(previewCmd.NewCmdPreview(f))
cmd.AddCommand(skillsCmd.NewCmdSkills(f))
cmd.AddCommand(skillsCmd.NewCmdSkills(f, telemetry))
// Root commands with standalone functionality and no subcommands
cmd.AddCommand(copilotCmd.NewCmdCopilot(f, nil))

68
pkg/cmd/skills/install/install.go
View file

@ -8,11 +8,14 @@ import (
"os"
"path/filepath"
"strings"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/gh/ghtelemetry"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills/discovery"
@ -38,6 +41,7 @@ const (
// InstallOptions holds all dependencies and user-provided flags for the install command.
type InstallOptions struct {
IO *iostreams.IOStreams
Telemetry ghtelemetry.EventRecorder
HttpClient func() (*http.Client, error)
Prompter prompter.Prompter
GitClient *git.Client
@ -59,9 +63,10 @@ type InstallOptions struct {
}
// NewCmdInstall creates the "skills install" command.
func NewCmdInstall(f *cmdutil.Factory, runF func(*InstallOptions) error) *cobra.Command {
func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF func(*InstallOptions) error) *cobra.Command {
opts := &InstallOptions{
IO: f.IOStreams,
Telemetry: telemetry,
Prompter: f.Prompter,
GitClient: f.GitClient,
Remotes: f.Remotes,
@ -232,6 +237,19 @@ func installRun(opts *InstallOptions) error {
return err
}
// Kick off the visibility fetch in parallel with the install work so
// the extra API roundtrip doesn't add latency on the critical path.
// The result is consumed when the telemetry event is emitted below.
type visResult struct {
vis discovery.RepoVisibility
err error
}
visCh := make(chan visResult, 1)
go func() {
vis, err := discovery.FetchRepoVisibility(apiClient, hostname, opts.repo.RepoOwner(), opts.repo.RepoName())
visCh <- visResult{vis: vis, err: err}
}()
resolved, err := resolveVersion(opts, apiClient, hostname)
if err != nil {
return err
@ -325,9 +343,57 @@ func installRun(opts *InstallOptions) error {
}
}
dims := map[string]string{
"agent_hosts": mapAgentHostsToIDs(selectedHosts),
"skill_host_type": ghinstance.CategorizeHost(opts.repo.RepoHost()),
}
select {
case r := <-visCh:
if r.err == nil {
dims["repo_visibility"] = string(r.vis)
if r.vis == discovery.RepoVisibilityPublic {
dims["skill_owner"] = opts.repo.RepoOwner()
dims["skill_repo"] = opts.repo.RepoName()
dims["skill_names"] = mapSkillsToNames(selectedSkills)
}
} else {
dims["repo_visibility"] = "unknown"
}
case <-time.After(visibilityWaitTimeout):
dims["repo_visibility"] = "unknown"
}
opts.Telemetry.Record(ghtelemetry.Event{
Type: "skill_install",
Dimensions: dims,
})
return nil
}
// visibilityWaitTimeout is how long to wait at telemetry-emit time for
// the in-flight repo visibility fetch before giving up and emitting
// repo_visibility="unknown". By this point the command has already done
// several serial API calls and (for install) a git sparse-checkout, so
// the fetch has almost always completed; this budget is a short safety
// net for the case where that single REST call has stalled.
const visibilityWaitTimeout = 200 * time.Millisecond
func mapSkillsToNames(skills []discovery.Skill) string {
names := make([]string, len(skills))
for i, s := range skills {
names[i] = s.DisplayName()
}
return strings.Join(names, ",")
}
func mapAgentHostsToIDs(hosts []*registry.AgentHost) string {
agentHostIDs := make([]string, len(hosts))
for i, h := range hosts {
agentHostIDs[i] = h.ID
}
return strings.Join(agentHostIDs, ",")
}
// runLocalInstall handles installation from a local directory path.
func runLocalInstall(opts *InstallOptions) error {
cs := opts.IO.ColorScheme()

188
pkg/cmd/skills/install/install_test.go
View file

@ -16,6 +16,7 @@ import (
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/telemetry"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -128,7 +129,7 @@ func TestNewCmdInstall(t *testing.T) {
}
var gotOpts *InstallOptions
cmd := NewCmdInstall(f, func(opts *InstallOptions) error {
cmd := NewCmdInstall(f, &telemetry.NoOpService{}, func(opts *InstallOptions) error {
gotOpts = opts
return nil
})
@ -167,7 +168,7 @@ func TestNewCmdInstall(t *testing.T) {
t.Run("command metadata", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{IOStreams: ios, Prompter: &prompter.PrompterMock{}, GitClient: &git.Client{}}
cmd := NewCmdInstall(f, nil)
cmd := NewCmdInstall(f, &telemetry.NoOpService{}, nil)
assert.Equal(t, "install <repository> [<skill[@version]>] [flags]", cmd.Use)
assert.NotEmpty(t, cmd.Short)
@ -1348,6 +1349,9 @@ func TestInstallRun(t *testing.T) {
ios.SetStdinTTY(tt.isTTY)
ios.SetStderrTTY(tt.isTTY)
opts := tt.opts(ios, reg)
if opts.Telemetry == nil {
opts.Telemetry = &telemetry.NoOpService{}
}
err := installRun(opts)
@ -1414,6 +1418,7 @@ func TestInstallRun_DeduplicatesSharedProjectDirAcrossHosts(t *testing.T) {
SkillSource: "monalisa/octocat-skills",
SkillName: "git-commit",
Force: true,
Telemetry: &telemetry.NoOpService{},
})
require.NoError(t, err)
assert.Equal(t, 1, strings.Count(stdout.String(), "Installed git-commit"))
@ -1980,3 +1985,182 @@ func Test_selectSkillsWithSelector_noDisclaimer(t *testing.T) {
require.NoError(t, err)
assert.NotContains(t, stderr.String(), "not verified by GitHub")
}
func TestInstallRun_TelemetryVisibility(t *testing.T) {
tests := []struct {
name string
visibility string
visibilityErr bool
wantSkillNames string
}{
{
name: "public repo includes skill names",
visibility: "public",
wantSkillNames: "git-commit",
},
{
name: "private repo excludes skill names",
visibility: "private",
},
{
name: "internal repo excludes skill names",
visibility: "internal",
},
{
name: "API error omits visibility and skill names",
visibilityErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
stubResolveVersion(reg, "monalisa", "octocat-skills", "v1.0.0", "abc123")
stubDiscoverTree(reg, "monalisa", "octocat-skills", "abc123",
singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA"))
stubInstallFiles(reg, "monalisa", "octocat-skills", "treeSHA", "blobSHA", gitCommitContent)
if tt.visibilityErr {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.StatusStringResponse(500, "server error"),
)
} else {
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": tt.visibility,
}),
)
}
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
recorder := &telemetry.EventRecorderSpy{}
err := installRun(&InstallOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
GitClient: &git.Client{RepoDir: t.TempDir()},
Prompter: &prompter.PrompterMock{},
SkillSource: "monalisa/octocat-skills",
SkillName: "git-commit",
Agent: "github-copilot",
Scope: "project",
ScopeChanged: true,
Dir: t.TempDir(),
Force: true,
Telemetry: recorder,
})
require.NoError(t, err)
require.Len(t, recorder.Events, 1)
event := recorder.Events[0]
assert.Equal(t, "skill_install", event.Type)
assert.NotEmpty(t, event.Dimensions["agent_hosts"], "agent_hosts should always be present")
// skill_host_type is always recorded (categorized, no raw hostname for enterprise/tenancy).
assert.Equal(t, "github.com", event.Dimensions["skill_host_type"])
if tt.visibilityErr {
assert.Equal(t, "unknown", event.Dimensions["repo_visibility"],
"visibility fetch errors should emit repo_visibility=\"unknown\" so the fallback is distinguishable from a successful fetch")
} else {
assert.Equal(t, tt.visibility, event.Dimensions["repo_visibility"])
}
// Owner, repo, and skill names are only included when the repo
// is public; for private/internal/unknown they are omitted to
// avoid leaking identifiers of non-public repositories.
if tt.wantSkillNames != "" {
assert.Equal(t, "monalisa", event.Dimensions["skill_owner"])
assert.Equal(t, "octocat-skills", event.Dimensions["skill_repo"])
assert.Equal(t, tt.wantSkillNames, event.Dimensions["skill_names"])
} else {
assert.Empty(t, event.Dimensions["skill_owner"])
assert.Empty(t, event.Dimensions["skill_repo"])
assert.Empty(t, event.Dimensions["skill_names"])
}
})
}
}
func TestInstallRun_TelemetryMultipleSkills(t *testing.T) {
codeReviewContent := heredoc.Doc(`
---
name: code-review
description: Reviews code
---
# Code Review
`)
reg := &httpmock.Registry{}
defer reg.Verify(t)
stubResolveVersion(reg, "monalisa", "octocat-skills", "v1.0.0", "abc123")
treeJSON := `{"path": "skills/git-commit", "type": "tree", "sha": "treeGC"}, ` +
`{"path": "skills/git-commit/SKILL.md", "type": "blob", "sha": "blobGC"}, ` +
`{"path": "skills/code-review", "type": "tree", "sha": "treeCR"}, ` +
`{"path": "skills/code-review/SKILL.md", "type": "blob", "sha": "blobCR"}`
stubDiscoverTree(reg, "monalisa", "octocat-skills", "abc123", treeJSON)
// Blob stubs for FetchDescriptionsConcurrent during interactive selection
encGC := base64.StdEncoding.EncodeToString([]byte(gitCommitContent))
encCR := base64.StdEncoding.EncodeToString([]byte(codeReviewContent))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/blobs/blobGC"),
httpmock.StringResponse(fmt.Sprintf(`{"sha": "blobGC", "content": %q, "encoding": "base64"}`, encGC)))
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills/git/blobs/blobCR"),
httpmock.StringResponse(fmt.Sprintf(`{"sha": "blobCR", "content": %q, "encoding": "base64"}`, encCR)))
stubInstallFiles(reg, "monalisa", "octocat-skills", "treeGC", "blobGC", gitCommitContent)
stubInstallFiles(reg, "monalisa", "octocat-skills", "treeCR", "blobCR", codeReviewContent)
reg.Register(
httpmock.REST("GET", "repos/monalisa/octocat-skills"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": "public",
}),
)
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
pm := &prompter.PrompterMock{
MultiSelectWithSearchFunc: func(_, _ string, _, _ []string, _ func(string) prompter.MultiSelectSearchResult) ([]string, error) {
return []string{allSkillsKey}, nil
},
}
recorder := &telemetry.EventRecorderSpy{}
err := installRun(&InstallOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
GitClient: &git.Client{RepoDir: t.TempDir()},
Prompter: pm,
SkillSource: "monalisa/octocat-skills",
Agent: "github-copilot",
Scope: "project",
ScopeChanged: true,
Dir: t.TempDir(),
Telemetry: recorder,
})
require.NoError(t, err)
require.Len(t, recorder.Events, 1)
event := recorder.Events[0]
assert.Equal(t, "skill_install", event.Type)
assert.Equal(t, "public", event.Dimensions["repo_visibility"])
// Verify comma-separated skill names (alphabetical order from DiscoverSkills)
names := strings.Split(event.Dimensions["skill_names"], ",")
assert.Len(t, names, 2)
assert.Contains(t, names, "code-review")
assert.Contains(t, names, "git-commit")
}

67
pkg/cmd/skills/preview/preview.go
View file

@ -7,9 +7,12 @@ import (
"path"
"sort"
"strings"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh/ghtelemetry"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills/discovery"
@ -23,6 +26,7 @@ import (
type PreviewOptions struct {
IO *iostreams.IOStreams
Telemetry ghtelemetry.EventRecorder
HttpClient func() (*http.Client, error)
Prompter prompter.Prompter
ExecutablePath string
@ -36,9 +40,10 @@ type PreviewOptions struct {
}
// NewCmdPreview creates the "skills preview" command.
func NewCmdPreview(f *cmdutil.Factory, runF func(*PreviewOptions) error) *cobra.Command {
func NewCmdPreview(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF func(*PreviewOptions) error) *cobra.Command {
opts := &PreviewOptions{
IO: f.IOStreams,
Telemetry: telemetry,
HttpClient: f.HttpClient,
Prompter: f.Prompter,
ExecutablePath: f.ExecutablePath,
@ -125,6 +130,19 @@ func previewRun(opts *PreviewOptions) error {
}
apiClient := api.NewClientFromHTTP(httpClient)
// Kick off the visibility fetch in parallel with the preview work so
// the extra API roundtrip doesn't add latency on the critical path.
// The result is consumed when the telemetry event is emitted below.
type visResult struct {
vis discovery.RepoVisibility
err error
}
visCh := make(chan visResult, 1)
go func() {
vis, err := discovery.FetchRepoVisibility(apiClient, hostname, owner, repoName)
visCh <- visResult{vis: vis, err: err}
}()
opts.IO.StartProgressIndicatorWithLabel(fmt.Sprintf("Resolving %s/%s", owner, repoName))
resolved, err := discovery.ResolveRef(apiClient, hostname, owner, repoName, opts.Version)
opts.IO.StopProgressIndicator()
@ -177,17 +195,50 @@ func previewRun(opts *PreviewOptions) error {
// Non-interactive or skill has only SKILL.md: dump through pager
if !canPrompt || len(extraFiles) == 0 {
return renderAllFiles(opts, cs, skill, files, rendered, extraFiles, apiClient, hostname, owner, repoName)
renderAllFiles(opts, cs, skill, files, rendered, extraFiles, apiClient, hostname, owner, repoName)
} else {
// Interactive with multiple files: show tree, then file picker
renderInteractive(opts, cs, skill, files, rendered, extraFiles, apiClient, hostname, owner, repoName)
}
// Interactive with multiple files: show tree, then file picker
return renderInteractive(opts, cs, skill, files, rendered, extraFiles, apiClient, hostname, owner, repoName)
dims := map[string]string{
"skill_host_type": ghinstance.CategorizeHost(opts.repo.RepoHost()),
}
select {
case r := <-visCh:
if r.err == nil {
dims["repo_visibility"] = string(r.vis)
if r.vis == discovery.RepoVisibilityPublic {
dims["skill_owner"] = opts.repo.RepoOwner()
dims["skill_repo"] = opts.repo.RepoName()
dims["skill_name"] = skill.DisplayName()
}
} else {
dims["repo_visibility"] = "unknown"
}
case <-time.After(visibilityWaitTimeout):
dims["repo_visibility"] = "unknown"
}
opts.Telemetry.Record(ghtelemetry.Event{
Type: "skill_preview",
Dimensions: dims,
})
return nil
}
// visibilityWaitTimeout is how long to wait at telemetry-emit time for
// the in-flight repo visibility fetch before giving up and emitting
// repo_visibility="unknown". By this point the command has already done
// several serial API calls and rendering work, so the fetch has almost
// always completed; this budget is a short safety net for the case
// where that single REST call has stalled.
const visibilityWaitTimeout = 200 * time.Millisecond
// renderAllFiles dumps the tree, SKILL.md, and all extra files through the pager.
func renderAllFiles(opts *PreviewOptions, cs *iostreams.ColorScheme, skill discovery.Skill,
files []discovery.SkillFile, rendered string, extraFiles []discovery.SkillFile,
apiClient *api.Client, hostname, owner, repo string) error {
apiClient *api.Client, hostname, owner, repo string) {
opts.IO.DetectTerminalTheme()
if err := opts.IO.StartPager(); err != nil {
@ -232,14 +283,12 @@ func renderAllFiles(opts *PreviewOptions, cs *iostreams.ColorScheme, skill disco
fmt.Fprintln(out)
}
}
return nil
}
// renderInteractive shows the file tree, then a picker to browse individual files.
func renderInteractive(opts *PreviewOptions, cs *iostreams.ColorScheme, skill discovery.Skill,
files []discovery.SkillFile, renderedSkillMD string, extraFiles []discovery.SkillFile,
apiClient *api.Client, hostname, owner, repo string) error {
apiClient *api.Client, hostname, owner, repo string) {
// Show the file tree to stderr so it persists above the prompt
fmt.Fprintf(opts.IO.ErrOut, "\n%s\n", cs.Bold(skill.DisplayName()+"/"))
@ -265,7 +314,7 @@ func renderInteractive(opts *PreviewOptions, cs *iostreams.ColorScheme, skill di
idx, err := opts.Prompter.Select("View a file (Esc to exit):", "", choices)
if err != nil {
return nil //nolint:nilerr // Prompter returns error on Esc/Ctrl-C; treat as graceful exit
return // Prompter returns error on Esc/Ctrl-C; treat as graceful exit
}
var content string

223
pkg/cmd/skills/preview/preview_test.go
View file

@ -11,6 +11,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/telemetry"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -74,7 +75,7 @@ func TestNewCmdPreview(t *testing.T) {
}
var gotOpts *PreviewOptions
cmd := NewCmdPreview(f, func(opts *PreviewOptions) error {
cmd := NewCmdPreview(f, &telemetry.NoOpService{}, func(opts *PreviewOptions) error {
gotOpts = opts
return nil
})
@ -332,6 +333,7 @@ func TestPreviewRun(t *testing.T) {
tt.opts.IO = ios
tt.opts.Prompter = &prompter.PrompterMock{}
tt.opts.Telemetry = &telemetry.NoOpService{}
err := previewRun(tt.opts)
@ -354,6 +356,7 @@ func TestPreviewRun_UnsupportedHost(t *testing.T) {
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{}, nil },
repo: ghrepo.NewWithHost("github", "awesome-copilot", "acme.ghes.com"),
Telemetry: &telemetry.NoOpService{},
})
require.ErrorContains(t, err, "supports only github.com")
}
@ -415,6 +418,7 @@ func TestPreviewRun_Interactive(t *testing.T) {
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: pm,
repo: ghrepo.New("owner", "repo"),
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -510,6 +514,7 @@ func TestPreviewRun_ShowsFileTree(t *testing.T) {
Prompter: pm,
repo: ghrepo.New("owner", "repo"),
SkillName: "my-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -593,6 +598,7 @@ func TestPreviewRun_ShowsFileTree(t *testing.T) {
renderCalls++
return fmt.Sprintf("rendered:%s", filePath)
},
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -618,6 +624,7 @@ func TestPreviewRun_ShowsFileTree(t *testing.T) {
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("owner", "repo"),
SkillName: "my-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -724,6 +731,7 @@ func TestPreviewRun_RenderLimits(t *testing.T) {
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("monalisa", "skills-repo"),
SkillName: "my-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -761,6 +769,7 @@ func TestPreviewRun_RenderLimits(t *testing.T) {
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("monalisa", "skills-repo"),
SkillName: "my-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -793,6 +802,7 @@ func TestPreviewRun_RenderLimits(t *testing.T) {
Prompter: &prompter.PrompterMock{},
repo: ghrepo.New("monalisa", "skills-repo"),
SkillName: "my-skill",
Telemetry: &telemetry.NoOpService{},
}
err := previewRun(opts)
@ -802,3 +812,214 @@ func TestPreviewRun_RenderLimits(t *testing.T) {
assert.Contains(t, out, "could not fetch file")
})
}
func TestPreviewRun_InteractiveTelemetryCapturesSelectedSkillName(t *testing.T) {
skillContent := "# Selected Skill\n\nContent here."
encodedContent := base64.StdEncoding.EncodeToString([]byte(skillContent))
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/abc123"),
httpmock.StringResponse(`{
"sha": "abc123",
"truncated": false,
"tree": [
{"path": "skills/alpha", "type": "tree", "sha": "tree-a"},
{"path": "skills/alpha/SKILL.md", "type": "blob", "sha": "blob-a"},
{"path": "skills/beta", "type": "tree", "sha": "tree-b"},
{"path": "skills/beta/SKILL.md", "type": "blob", "sha": "blob-b"}
]
}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/tree-b"),
httpmock.StringResponse(`{
"tree": [
{"path": "SKILL.md", "type": "blob", "sha": "blob-b", "size": 40}
]
}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/blobs/blob-b"),
httpmock.StringResponse(`{"sha": "blob-b", "content": "`+encodedContent+`", "encoding": "base64"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": "public",
}),
)
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
pm := &prompter.PrompterMock{
SelectFunc: func(prompt string, defaultValue string, options []string) (int, error) {
return 1, nil // select "beta"
},
}
recorder := &telemetry.EventRecorderSpy{}
opts := &PreviewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: pm,
Telemetry: recorder,
repo: ghrepo.New("owner", "repo"),
// SkillName intentionally left empty to simulate interactive selection
}
err := previewRun(opts)
require.NoError(t, err)
// Verify the telemetry event captured the interactively-selected skill name, not empty string
require.Len(t, recorder.Events, 1)
event := recorder.Events[0]
assert.Equal(t, "skill_preview", event.Type)
assert.Equal(t, "beta", event.Dimensions["skill_name"], "telemetry should capture the selected skill name, not the empty opts.SkillName")
}
func TestPreviewRun_TelemetryVisibility(t *testing.T) {
skillContent := heredoc.Doc(`
---
name: my-skill
description: test
---
# My Skill
Body.
`)
encodedContent := base64.StdEncoding.EncodeToString([]byte(skillContent))
tests := []struct {
name string
visibility string
visibilityErr bool
wantSkillNames string
}{
{
name: "public repo includes skill names",
visibility: "public",
wantSkillNames: "my-skill",
},
{
name: "private repo excludes skill names",
visibility: "private",
},
{
name: "internal repo excludes skill names",
visibility: "internal",
},
{
name: "API error omits visibility and skill names",
visibilityErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/releases/latest"),
httpmock.StringResponse(`{"tag_name": "v1.0.0"}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/ref/tags/v1.0.0"),
httpmock.StringResponse(`{"object": {"sha": "abc123", "type": "commit"}}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/abc123"),
httpmock.StringResponse(`{
"sha": "abc123",
"truncated": false,
"tree": [
{"path": "skills/my-skill", "type": "tree", "sha": "treeSHA"},
{"path": "skills/my-skill/SKILL.md", "type": "blob", "sha": "blobSKILL"}
]
}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/trees/treeSHA"),
httpmock.StringResponse(`{
"tree": [
{"path": "SKILL.md", "type": "blob", "sha": "blobSKILL", "size": 50}
]
}`),
)
reg.Register(
httpmock.REST("GET", "repos/owner/repo/git/blobs/blobSKILL"),
httpmock.StringResponse(`{"sha": "blobSKILL", "content": "`+encodedContent+`", "encoding": "base64"}`),
)
if tt.visibilityErr {
reg.Register(
httpmock.REST("GET", "repos/owner/repo"),
httpmock.StatusStringResponse(500, "server error"),
)
} else {
reg.Register(
httpmock.REST("GET", "repos/owner/repo"),
httpmock.JSONResponse(map[string]interface{}{
"visibility": tt.visibility,
}),
)
}
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(false)
ios.SetStdinTTY(false)
recorder := &telemetry.EventRecorderSpy{}
opts := &PreviewOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Prompter: &prompter.PrompterMock{},
Telemetry: recorder,
repo: ghrepo.New("owner", "repo"),
SkillName: "my-skill",
}
err := previewRun(opts)
require.NoError(t, err)
require.Len(t, recorder.Events, 1)
event := recorder.Events[0]
assert.Equal(t, "skill_preview", event.Type)
// skill_host_type is always recorded (categorized, no raw hostname for enterprise/tenancy).
assert.Equal(t, "github.com", event.Dimensions["skill_host_type"])
if tt.visibilityErr {
assert.Equal(t, "unknown", event.Dimensions["repo_visibility"],
"visibility fetch errors should emit repo_visibility=\"unknown\" so the fallback is distinguishable from a successful fetch")
} else {
assert.Equal(t, tt.visibility, event.Dimensions["repo_visibility"])
}
// Owner, repo, and skill name are only included when the repo
// is public; for private/internal/unknown they are omitted to
// avoid leaking identifiers of non-public repositories.
if tt.wantSkillNames != "" {
assert.Equal(t, "owner", event.Dimensions["skill_owner"])
assert.Equal(t, "repo", event.Dimensions["skill_repo"])
assert.Equal(t, tt.wantSkillNames, event.Dimensions["skill_name"])
} else {
assert.Empty(t, event.Dimensions["skill_owner"])
assert.Empty(t, event.Dimensions["skill_repo"])
assert.Empty(t, event.Dimensions["skill_name"])
}
})
}
}

12
pkg/cmd/skills/search/search.go
View file

@ -15,6 +15,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/gh/ghtelemetry"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/skills/discovery"
"github.com/cli/cli/v2/internal/skills/frontmatter"
@ -48,6 +49,7 @@ var SkillSearchFields = []string{
type SearchOptions struct {
IO *iostreams.IOStreams
Telemetry ghtelemetry.EventRecorder
HttpClient func() (*http.Client, error)
Config func() (gh.Config, error)
Prompter prompter.Prompter
@ -62,9 +64,10 @@ type SearchOptions struct {
}
// NewCmdSearch creates the "skills search" command.
func NewCmdSearch(f *cmdutil.Factory, runF func(*SearchOptions) error) *cobra.Command {
func NewCmdSearch(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF func(*SearchOptions) error) *cobra.Command {
opts := &SearchOptions{
IO: f.IOStreams,
Telemetry: telemetry,
HttpClient: f.HttpClient,
Config: f.Config,
Prompter: f.Prompter,
@ -552,6 +555,13 @@ func promptInstall(opts *SearchOptions, skills []skillResult) error {
return nil
}
opts.Telemetry.Record(ghtelemetry.Event{
Type: "skill_search_install",
Measures: ghtelemetry.Measures{
"install_count": int64(len(indices)),
},
})
// Prompt for target agent host (once for all selected skills)
hostNames := registry.AgentNames()
hostIdx, err := opts.Prompter.Select("Select target agent:", "", hostNames)

77
pkg/cmd/skills/search/search_test.go
View file

@ -8,6 +8,8 @@ import (
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/telemetry"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
@ -102,7 +104,7 @@ func TestNewCmdSearch(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
f := &cmdutil.Factory{}
var gotOpts *SearchOptions
cmd := NewCmdSearch(f, func(opts *SearchOptions) error {
cmd := NewCmdSearch(f, &telemetry.NoOpService{}, func(opts *SearchOptions) error {
gotOpts = opts
return nil
})
@ -372,6 +374,7 @@ func TestSearchRun(t *testing.T) {
ios.SetStdoutTTY(tt.tty)
ios.SetStderrTTY(tt.tty)
tt.opts.IO = ios
tt.opts.Telemetry = &telemetry.NoOpService{}
defer reg.Verify(t)
err := searchRun(tt.opts)
@ -576,3 +579,75 @@ func TestDeduplicateByName_Namespaced(t *testing.T) {
assert.NotEqual(t, "org/repo6", s.Repo)
}
}
// TestSearchRun_TelemetryRecordsInstallFromResults verifies that when a
// user searches, picks one or more results interactively, and proceeds to
// install them, the search command records a telemetry event capturing
// that the search led to an install attempt. This is the key signal for
// measuring the value of search results: of the searches that ran, how
// many converted to an install?
func TestSearchRun_TelemetryRecordsInstallFromResults(t *testing.T) {
codeResponse := `{"total_count": 1, "incomplete_results": false, "items": [
{"name": "SKILL.md", "path": "skills/terraform/SKILL.md", "sha": "abc123",
"repository": {"full_name": "org/repo"}}
]}`
reg := &httpmock.Registry{}
defer reg.Verify(t)
// Keyword search fires path + owner + primary (3 requests).
for range 3 {
reg.Register(
httpmock.REST("GET", "search/code"),
httpmock.StringResponse(codeResponse),
)
}
ios, _, _, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStderrTTY(true)
ios.SetStdinTTY(true)
pm := &prompter.PrompterMock{
MultiSelectFunc: func(prompt string, defaults []string, options []string) ([]int, error) {
// Select the single result.
return []int{0}, nil
},
SelectFunc: func(prompt, defaultValue string, options []string) (int, error) {
// First Select: target agent (0). Second Select: scope (0).
return 0, nil
},
}
recorder := &telemetry.EventRecorderSpy{}
err := searchRun(&SearchOptions{
IO: ios,
HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil },
Config: func() (gh.Config, error) { return config.NewBlankConfig(), nil },
Prompter: pm,
Telemetry: recorder,
ExecutablePath: "/nonexistent/gh", // install subprocess will fail; failures are logged, not fatal.
Query: "terraform",
Page: 1,
Limit: defaultLimit,
})
require.NoError(t, err)
// The search command no longer records a separate skill_search event;
// only the follow-up skill_search_install event fires when the user
// proceeds to install from the results.
require.Len(t, recorder.Events, 1)
installEvent := recorder.Events[0]
assert.Equal(t, "skill_search_install", installEvent.Type,
"an install triggered from search results should be recorded as a distinct event")
assert.Equal(t, int64(1), installEvent.Measures["install_count"],
"install_count captures how many results the user chose to install")
// The skill_search_install event must not carry the query or owner:
// these were intentionally removed so that installs from search are
// not linked back to the search terms at the telemetry layer.
assert.Empty(t, installEvent.Dimensions["query"],
"skill_search_install must not record the search query")
assert.Empty(t, installEvent.Dimensions["owner"],
"skill_search_install must not record the search owner filter")
}

13
pkg/cmd/skills/skills.go
View file

@ -2,6 +2,7 @@ package skills
import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/gh/ghtelemetry"
"github.com/cli/cli/v2/pkg/cmd/skills/install"
"github.com/cli/cli/v2/pkg/cmd/skills/preview"
"github.com/cli/cli/v2/pkg/cmd/skills/publish"
@ -12,7 +13,7 @@ import (
)
// NewCmdSkills returns the top-level "skill" command.
func NewCmdSkills(f *cmdutil.Factory) *cobra.Command {
func NewCmdSkills(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder) *cobra.Command {
cmd := &cobra.Command{
Use: "skill <command>",
Short: "Install and manage agent skills (preview)",
@ -40,12 +41,16 @@ func NewCmdSkills(f *cmdutil.Factory) *cobra.Command {
# Validate skills for publishing
$ gh skill publish --dry-run
`),
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
telemetry.SetSampleRate(ghtelemetry.SAMPLE_ALL)
return nil
},
}
cmd.AddCommand(install.NewCmdInstall(f, nil))
cmd.AddCommand(preview.NewCmdPreview(f, nil))
cmd.AddCommand(install.NewCmdInstall(f, telemetry, nil))
cmd.AddCommand(preview.NewCmdPreview(f, telemetry, nil))
cmd.AddCommand(publish.NewCmdPublish(f, nil))
cmd.AddCommand(search.NewCmdSearch(f, nil))
cmd.AddCommand(search.NewCmdSearch(f, telemetry, nil))
cmd.AddCommand(update.NewCmdUpdate(f, nil))
return cmd

19
pkg/cmd/skills/skills_test.go Normal file
View file

@ -0,0 +1,19 @@
package skills_test
import (
"testing"
"github.com/cli/cli/v2/internal/gh/ghtelemetry"
"github.com/cli/cli/v2/internal/telemetry"
"github.com/cli/cli/v2/pkg/cmd/skills"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/stretchr/testify/require"
)
func TestSkillCommandsAreSampledAt100(t *testing.T) {
spy := &telemetry.CommandRecorderSpy{}
factory := &cmdutil.Factory{}
cmd := skills.NewCmdSkills(factory, spy)
cmd.PersistentPreRunE(nil, []string{})
require.Equal(t, ghtelemetry.SAMPLE_ALL, spy.LastSampleRate)
}