diff --git a/acceptance/testdata/telemetry/no-telemetry-for-completion.txtar b/acceptance/testdata/telemetry/no-telemetry-for-completion.txtar index ffde6e6059f..20139ce5f41 100644 --- a/acceptance/testdata/telemetry/no-telemetry-for-completion.txtar +++ b/acceptance/testdata/telemetry/no-telemetry-for-completion.txtar @@ -4,4 +4,4 @@ env GH_TELEMETRY=log env GH_TELEMETRY_SAMPLE_RATE=100 exec gh completion -s bash -! stderr 'Telemetry payload:' +stderr 'Telemetry payload: none' diff --git a/acceptance/testdata/telemetry/no-telemetry-for-extension.txtar b/acceptance/testdata/telemetry/no-telemetry-for-extension.txtar index fcb820e0c2a..19f3d69ccaf 100644 --- a/acceptance/testdata/telemetry/no-telemetry-for-extension.txtar +++ b/acceptance/testdata/telemetry/no-telemetry-for-extension.txtar @@ -22,7 +22,7 @@ cd $WORK # Run the extension and verify no telemetry is logged exec gh hello stdout 'hello from extension' -! stderr 'Telemetry payload:' +stderr 'Telemetry payload: none' -- gh-hello.sh -- #!/usr/bin/env bash diff --git a/acceptance/testdata/telemetry/no-telemetry-for-ghes-user.txtar b/acceptance/testdata/telemetry/no-telemetry-for-ghes-user.txtar index 0fe6f4bb210..f04fabf364e 100644 --- a/acceptance/testdata/telemetry/no-telemetry-for-ghes-user.txtar +++ b/acceptance/testdata/telemetry/no-telemetry-for-ghes-user.txtar @@ -5,4 +5,4 @@ env GH_TELEMETRY_SAMPLE_RATE=100 env GH_ENTERPRISE_TOKEN=fake-enterprise-token exec gh version -! stderr 'Telemetry payload:' +stderr 'Telemetry payload: none' diff --git a/acceptance/testdata/telemetry/no-telemetry-for-send-telemetry.txtar b/acceptance/testdata/telemetry/no-telemetry-for-send-telemetry.txtar index 7f9d0457aa9..28436aaae58 100644 --- a/acceptance/testdata/telemetry/no-telemetry-for-send-telemetry.txtar +++ b/acceptance/testdata/telemetry/no-telemetry-for-send-telemetry.txtar @@ -8,7 +8,7 @@ env GH_TELEMETRY_ENDPOINT_URL=http://localhost:1 # It will fail to connect but that's fine — we only care about telemetry logging. stdin payload.json ! exec gh send-telemetry -! stderr 'Telemetry payload:' +stderr 'Telemetry payload: none' -- payload.json -- {"events":[{"type":"test","dimensions":{},"measures":{}}]} diff --git a/internal/ghcmd/cmd.go b/internal/ghcmd/cmd.go index 34806f87449..9512e4b55bb 100644 --- a/internal/ghcmd/cmd.go +++ b/internal/ghcmd/cmd.go @@ -82,19 +82,31 @@ func Main() exitCode { case cfgErr != nil: // Without a valid on-disk config we can't honour user telemetry preferences, so disable it to be safe. telemetryService = &telemetry.NoOpService{} - case os.Getenv("GH_PRIVATE_ENABLE_TELEMETRY") == "" || mightBeGHESUser(cfg): - telemetryService = &telemetry.NoOpService{} default: telemetryState := telemetry.ParseTelemetryState(cfg.Telemetry().Value) + telemetryDisabled := os.Getenv("GH_PRIVATE_ENABLE_TELEMETRY") == "" || mightBeGHESUser(cfg) + switch telemetryState { case telemetry.Disabled: telemetryService = &telemetry.NoOpService{} case telemetry.Logged: + // Always construct the real service in log mode so that the log + // flusher runs and surfaces an explicit "Telemetry payload: none" + // marker when no events will be sent. This gives the user an + // observable signal that telemetry is wired up even when their + // context (e.g. GHES) causes events to be dropped. telemetryService = telemetry.NewService( telemetry.LogFlusher(ioStreams.ErrOut, ioStreams.ColorEnabled()), telemetry.WithAdditionalCommonDimensions(additionalCommonDimensions), ) + if telemetryDisabled { + telemetryService.Disable() + } case telemetry.Enabled: + if telemetryDisabled { + telemetryService = &telemetry.NoOpService{} + break + } sampleRate := 1 if v, err := strconv.Atoi(os.Getenv("GH_TELEMETRY_SAMPLE_RATE")); err == nil && v >= 0 && v <= 100 { sampleRate = v diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 67fb8b7626e..e5dcbad9a49 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -3,6 +3,7 @@ package telemetry import ( "bytes" + "encoding/binary" "encoding/json" "errors" "fmt" @@ -166,17 +167,24 @@ func WithSampleRate(rate int) telemetryServiceOption { } // LogFlusher returns a flush function that writes telemetry payloads to the provided log writer. This is used for the "log" telemetry mode, which is intended for debugging and development. +// When there are no events to report (for example the command opted out of telemetry, the user is on GHES, or no events were recorded), a "Telemetry payload: none" marker is written so that the absence of events is observable. var LogFlusher = func(log io.Writer, colorEnabled bool) func(payload SendTelemetryPayload) { return func(payload SendTelemetryPayload) { + header := "Telemetry payload:" + if colorEnabled { + header = ansi.Color(header, "cyan+b") + } + + if len(payload.Events) == 0 { + fmt.Fprintf(log, "%s none\n", header) + return + } + payloadBytes, err := json.Marshal(payload) if err != nil { return } - header := "Telemetry payload:" - if colorEnabled { - header = ansi.Color(header, "cyan+b") - } fmt.Fprintf(log, "%s\n", header) if colorEnabled { @@ -190,8 +198,12 @@ var LogFlusher = func(log io.Writer, colorEnabled bool) func(payload SendTelemet } // GitHubFlusher returns a flush function that sends telemetry payloads to a child `gh send-telemetry` process. This is used for the "enabled" telemetry mode. +// Empty payloads are dropped without spawning a subprocess. var GitHubFlusher = func(executable string) func(payload SendTelemetryPayload) { return func(payload SendTelemetryPayload) { + if len(payload.Events) == 0 { + return + } SpawnSendTelemetry(executable, payload) } } @@ -221,7 +233,7 @@ func NewService(flusher func(SendTelemetryPayload), opts ...telemetryServiceOpti maps.Copy(commonDimensions, telemetryServiceOpts.additionalDimensions) hash := uuid.NewSHA1(uuid.Nil, []byte(invocationID)) - sampleBucket := hash[0] % 100 + sampleBucket := byte(binary.BigEndian.Uint32(hash[:4]) % 100) s := &service{ flush: flusher, @@ -278,28 +290,29 @@ func (s *service) Flush() { s.mu.Lock() defer s.mu.Unlock() - if s.disabled { - return - } - if s.previouslyCalled { return } s.previouslyCalled = true - if len(s.events) == 0 { + if s.sampleRate > 0 && s.sampleRate < 100 && int(s.sampleBucket) >= s.sampleRate { return } - if s.sampleRate > 0 && s.sampleRate < 100 && int(s.sampleBucket) >= s.sampleRate { - return + // When the service has been disabled mid-invocation (e.g. an enterprise host + // was contacted), discard any recorded events. We still call the flusher + // with an empty payload so that the log-mode flusher can surface the + // absence of telemetry rather than leaving the user staring at silence. + events := s.events + if s.disabled { + events = nil } payload := SendTelemetryPayload{ - Events: make([]PayloadEvent, len(s.events)), + Events: make([]PayloadEvent, len(events)), } - for i, recorded := range s.events { + for i, recorded := range events { dimensions := map[string]string{ "timestamp": recorded.recordedAt.UTC().Format("2006-01-02T15:04:05.000Z"), } diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index 207d611eeca..a796afd677d 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -351,6 +351,23 @@ func TestNewServiceLogModeWithColorLogsToWriter(t *testing.T) { assert.Contains(t, output, "\033[", "expected ANSI escape sequences when color is enabled") } +func TestLogFlusherWritesNoneMarkerForEmptyPayload(t *testing.T) { + t.Run("no color", func(t *testing.T) { + var buf bytes.Buffer + LogFlusher(&buf, false)(SendTelemetryPayload{}) + assert.Equal(t, "Telemetry payload: none\n", buf.String()) + }) + + t.Run("with color", func(t *testing.T) { + var buf bytes.Buffer + LogFlusher(&buf, true)(SendTelemetryPayload{}) + output := buf.String() + assert.Contains(t, output, "Telemetry payload:") + assert.Contains(t, output, "none") + assert.Contains(t, output, "\x1b") // ANSI escape char for color codes + }) +} + func TestServiceDeviceIDFallback(t *testing.T) { t.Cleanup(stubDeviceIDError(errors.New("no device id"))) @@ -365,14 +382,19 @@ func TestServiceDeviceIDFallback(t *testing.T) { } func TestServiceFlush(t *testing.T) { - t.Run("does nothing when no events recorded", func(t *testing.T) { + t.Run("calls flusher with empty payload when no events recorded", func(t *testing.T) { t.Cleanup(stubDeviceID("test-device")) + var captured SendTelemetryPayload called := false - svc := newService(func(SendTelemetryPayload) { called = true }, nil) + svc := newService(func(p SendTelemetryPayload) { + called = true + captured = p + }, nil) svc.Flush() - assert.False(t, called, "flusher should not be called with no events") + assert.True(t, called, "flusher should be called even with no events so log mode can surface the absence") + assert.Empty(t, captured.Events, "payload should have no events") }) t.Run("flushes events with merged dimensions", func(t *testing.T) { @@ -599,24 +621,33 @@ func TestWithAdditionalCommonDimensions(t *testing.T) { } func TestServiceDisable(t *testing.T) { - t.Run("prevents flush from sending events", func(t *testing.T) { + t.Run("drops recorded events from flushed payload", func(t *testing.T) { t.Cleanup(stubDeviceID("test-device")) + var captured SendTelemetryPayload called := false - svc := newService(func(SendTelemetryPayload) { called = true }, nil) + svc := newService(func(p SendTelemetryPayload) { + called = true + captured = p + }, nil) svc.Record(ghtelemetry.Event{Type: "test"}) svc.Disable() svc.Flush() - assert.False(t, called, "flusher should not be called after Disable()") + assert.True(t, called, "flusher should still be called so log mode can surface the absence of events") + assert.Empty(t, captured.Events, "recorded events should be dropped after Disable()") }) - t.Run("prevents flush even with multiple recorded events", func(t *testing.T) { + t.Run("drops events even with multiple recorded events", func(t *testing.T) { t.Cleanup(stubDeviceID("test-device")) + var captured SendTelemetryPayload called := false - svc := newService(func(SendTelemetryPayload) { called = true }, nil) + svc := newService(func(p SendTelemetryPayload) { + called = true + captured = p + }, nil) svc.Record(ghtelemetry.Event{Type: "event1"}) svc.Record(ghtelemetry.Event{Type: "event2"}) @@ -624,20 +655,26 @@ func TestServiceDisable(t *testing.T) { svc.Disable() svc.Flush() - assert.False(t, called, "flusher should not be called after Disable()") + assert.True(t, called, "flusher should still be called") + assert.Empty(t, captured.Events, "recorded events should be dropped after Disable()") }) t.Run("can be called before any events are recorded", func(t *testing.T) { t.Cleanup(stubDeviceID("test-device")) + var captured SendTelemetryPayload called := false - svc := newService(func(SendTelemetryPayload) { called = true }, nil) + svc := newService(func(p SendTelemetryPayload) { + called = true + captured = p + }, nil) svc.Disable() svc.Record(ghtelemetry.Event{Type: "test"}) svc.Flush() - assert.False(t, called, "flusher should not be called when disabled before recording") + assert.True(t, called, "flusher should still be called") + assert.Empty(t, captured.Events, "events recorded after Disable() should be dropped") }) } diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 3002a351294..0becbf5c81b 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -127,6 +127,16 @@ var HelpTopics = []helpTopic{ a textual progress indicator. `, "`"), }, + { + name: "telemetry", + short: "Information about telemetry in gh", + long: heredoc.Doc(` + gh collects telemetry to help us understand how the CLI is being used and to improve it. + + To learn more about what data is collected, how it is used, and how to opt out, see: + + `), + }, { name: "reference", short: "A comprehensive reference of all gh commands", diff --git a/pkg/cmd/skills/install/install.go b/pkg/cmd/skills/install/install.go index d792501fd27..a22249225da 100644 --- a/pkg/cmd/skills/install/install.go +++ b/pkg/cmd/skills/install/install.go @@ -1,6 +1,7 @@ package install import ( + "encoding/base64" "errors" "fmt" "io" @@ -57,6 +58,7 @@ type InstallOptions struct { Force bool FromLocal bool // treat SkillSource as a local directory path AllowHiddenDirs bool // include skills in dot-prefixed directories + Upstream bool // install from upstream when re-published skill detected repo ghrepo.Interface // set when SkillSource is a GitHub repository localPath string // set when FromLocal is true @@ -193,6 +195,10 @@ func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru return err } + if err := cmdutil.MutuallyExclusive("--from-local and --upstream cannot be used together", opts.FromLocal, opts.Upstream); err != nil { + return err + } + if opts.Pin != "" && opts.SkillName != "" && strings.Contains(opts.SkillName, "@") { return cmdutil.FlagErrorf("cannot use --pin with an inline @version in the skill name") } @@ -212,6 +218,7 @@ func NewCmdInstall(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, ru cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Overwrite existing skills without prompting") cmd.Flags().BoolVar(&opts.FromLocal, "from-local", false, "Treat the argument as a local directory path instead of a repository") cmd.Flags().BoolVar(&opts.AllowHiddenDirs, "allow-hidden-dirs", false, "Include skills in hidden directories (e.g. .claude/skills/, .agents/skills/)") + cmd.Flags().BoolVar(&opts.Upstream, "upstream", false, "Install from the upstream source when a re-published skill is detected") cmdutil.DisableAuthCheckFlag(cmd.Flags().Lookup("from-local")) return cmd @@ -248,13 +255,17 @@ func installRun(opts *InstallOptions) error { // 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. + // Capture repo fields now to avoid a data race if opts.repo is + // swapped during an upstream redirect. type visResult struct { vis discovery.RepoVisibility err error } visCh := make(chan visResult, 1) + visOwner := opts.repo.RepoOwner() + visRepo := opts.repo.RepoName() go func() { - vis, err := discovery.FetchRepoVisibility(apiClient, hostname, opts.repo.RepoOwner(), opts.repo.RepoName()) + vis, err := discovery.FetchRepoVisibility(apiClient, hostname, visOwner, visRepo) visCh <- visResult{vis: vis, err: err} }() @@ -293,6 +304,43 @@ func installRun(opts *InstallOptions) error { } } + // Track upstream provenance detection result for telemetry. + upstreamSource := "none" + + // Check if the selected skill was re-published from an upstream source. + // The re-publisher's SKILL.md will have github-repo metadata pointing + // to the original source repo. If detected, offer to install directly + // from upstream instead. + if len(selectedSkills) == 1 && selectedSkills[0].BlobSHA != "" { + upstreamRepo, detected, err := checkUpstreamProvenance(opts, apiClient, hostname, selectedSkills[0], resolved.SHA) + if err != nil { + return err + } + if upstreamRepo != nil { + redirectDims := map[string]string{} + select { + case r := <-visCh: + if r.err == nil && r.vis == discovery.RepoVisibilityPublic { + redirectDims["from_owner"] = visOwner + redirectDims["from_repo"] = visRepo + } + case <-time.After(visibilityWaitTimeout): + } + opts.Telemetry.Record(ghtelemetry.Event{ + Type: "skill_upstream_redirect", + Dimensions: redirectDims, + }) + opts.repo = upstreamRepo + opts.SkillSource = ghrepo.FullName(upstreamRepo) + opts.version = "" + opts.Pin = "" + return installRun(opts) + } + if detected { + upstreamSource = "republisher" + } + } + printPreInstallDisclaimer(opts.IO.ErrOut, cs) selectedHosts, err := resolveHosts(opts, canPrompt) @@ -355,6 +403,7 @@ func installRun(opts *InstallOptions) error { dims := map[string]string{ "agent_hosts": mapAgentHostsToIDs(selectedHosts), "skill_host_type": ghinstance.CategorizeHost(opts.repo.RepoHost()), + "upstream_source": upstreamSource, } select { case r := <-visCh: @@ -1169,3 +1218,85 @@ func filterHiddenDirSkills(opts *InstallOptions, allSkills []discovery.Skill) ([ return standard, nil } + +// checkUpstreamProvenance fetches the skill's SKILL.md via the contents API +// to check if it contains github-repo metadata pointing to a different +// repository, indicating the skill was re-published from an upstream source. +// In interactive mode, the user is asked whether to install from the +// re-publisher or redirect to the upstream. Non-interactive mode always +// installs from the re-publisher. +// Returns (repo to redirect to, whether upstream was detected, error). +func checkUpstreamProvenance(opts *InstallOptions, client *api.Client, hostname string, skill discovery.Skill, commitSHA string) (ghrepo.Interface, bool, error) { + apiPath := fmt.Sprintf("repos/%s/%s/contents/%s?ref=%s", + opts.repo.RepoOwner(), opts.repo.RepoName(), + skill.Path+"/SKILL.md", commitSHA) + var fileResp struct { + Content string `json:"content"` + Encoding string `json:"encoding"` + } + if err := client.REST(hostname, "GET", apiPath, nil, &fileResp); err != nil { + return nil, false, nil //nolint:nilerr // best-effort check; failing to fetch is not fatal + } + if fileResp.Encoding != "base64" { + return nil, false, nil + } + decoded, decodeErr := io.ReadAll(base64.NewDecoder(base64.StdEncoding, strings.NewReader(fileResp.Content))) + if decodeErr != nil { + return nil, false, nil //nolint:nilerr // best-effort; decode failure is not fatal + } + content := string(decoded) + + result, parseErr := frontmatter.Parse(content) + if parseErr != nil || result.Metadata.Meta == nil { + //nolint:nilerr // unparseable frontmatter means no upstream to detect + return nil, false, nil + } + + existingRepo, _ := result.Metadata.Meta["github-repo"].(string) + if existingRepo == "" { + return nil, false, nil + } + + currentRepoURL := source.BuildRepoURL(hostname, opts.repo.RepoOwner(), opts.repo.RepoName()) + if existingRepo == currentRepoURL { + return nil, false, nil + } + + upstreamRepo, parseErr := source.ParseRepoURL(existingRepo) + if parseErr != nil { + //nolint:nilerr // invalid repo URL means we can't redirect; install normally + return nil, false, nil + } + + cs := opts.IO.ColorScheme() + upstreamLabel := ghrepo.FullName(upstreamRepo) + repoSource := ghrepo.FullName(opts.repo) + + fmt.Fprintf(opts.IO.ErrOut, "%s This skill was originally published in %s\n", cs.WarningIcon(), upstreamLabel) + + if opts.Upstream { + fmt.Fprintf(opts.IO.ErrOut, "Redirecting install to %s...\n", upstreamLabel) + return upstreamRepo, true, nil + } + + if !opts.IO.CanPrompt() { + fmt.Fprintf(opts.IO.ErrOut, " Installing from %s (use --upstream or interactive mode to choose upstream)\n", repoSource) + return nil, true, nil + } + + choices := []string{ + fmt.Sprintf("%s (re-publisher, recommended)", repoSource), + fmt.Sprintf("%s (upstream)", upstreamLabel), + } + idx, err := opts.Prompter.Select("Install from:", "", choices) + if err != nil { + return nil, true, err + } + + if idx == 1 { + fmt.Fprintf(opts.IO.ErrOut, "Redirecting install to %s...\n", upstreamLabel) + return upstreamRepo, true, nil + } + + return nil, true, nil +} diff --git a/pkg/cmd/skills/install/install_test.go b/pkg/cmd/skills/install/install_test.go index e4bd074bd5f..b7aa956c5a9 100644 --- a/pkg/cmd/skills/install/install_test.go +++ b/pkg/cmd/skills/install/install_test.go @@ -125,6 +125,16 @@ func TestNewCmdInstall(t *testing.T) { cli: "monalisa/skills-repo --allow-hidden-dirs", wantOpts: InstallOptions{SkillSource: "monalisa/skills-repo", Scope: "project", AllowHiddenDirs: true}, }, + { + name: "upstream flag", + cli: "monalisa/skills-repo git-commit --upstream", + wantOpts: InstallOptions{SkillSource: "monalisa/skills-repo", SkillName: "git-commit", Scope: "project", Upstream: true}, + }, + { + name: "from-local with --upstream is mutually exclusive", + cli: "--from-local ./local-dir --upstream", + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -2487,3 +2497,213 @@ func TestInstallRun_TelemetryMultipleSkills(t *testing.T) { assert.Contains(t, names, "code-review") assert.Contains(t, names, "git-commit") } + +var republishedContent = heredoc.Doc(` + --- + name: git-commit + description: Writes commits + metadata: + github-repo: https://github.com/monalisa/original-skills + github-tree-sha: upstreamTreeSHA + github-path: skills/git-commit + --- + # Git Commit +`) + +func stubContentsAPI(reg *httpmock.Registry, owner, repo, path, content string) { + encoded := base64.StdEncoding.EncodeToString([]byte(content)) + reg.Register( + httpmock.REST("GET", fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, path)), + httpmock.StringResponse(fmt.Sprintf(`{"content": %q, "encoding": "base64"}`, encoded)), + ) +} + +func TestInstallRun_UpstreamDetection(t *testing.T) { + tests := []struct { + name string + isTTY bool + stubs func(*httpmock.Registry) + opts func(t *testing.T, ios *iostreams.IOStreams, reg *httpmock.Registry) *InstallOptions + wantErr string + wantStdout string + wantStderr string + }{ + { + name: "detects re-published skill and user picks re-publisher", + isTTY: true, + stubs: func(reg *httpmock.Registry) { + stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123") + stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123", + singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA")) + stubContentsAPI(reg, "monalisa", "skills-repo", + "skills/git-commit/SKILL.md", republishedContent) + stubInstallFiles(reg, "monalisa", "skills-repo", + "treeSHA", "blobSHA", republishedContent) + }, + opts: func(t *testing.T, ios *iostreams.IOStreams, reg *httpmock.Registry) *InstallOptions { + return &InstallOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + GitClient: &git.Client{RepoDir: t.TempDir()}, + Prompter: &prompter.PrompterMock{ + SelectFunc: func(_ string, _ string, choices []string) (int, error) { + require.Len(t, choices, 2) + assert.Contains(t, choices[0], "monalisa/skills-repo") + assert.Contains(t, choices[1], "monalisa/original-skills") + return 0, nil + }, + }, + Telemetry: &telemetry.NoOpService{}, + SkillSource: "monalisa/skills-repo", + SkillName: "git-commit", + Agent: "github-copilot", + Scope: "project", + ScopeChanged: true, + Dir: t.TempDir(), + } + }, + wantStderr: "originally published in monalisa/original-skills", + wantStdout: "Installed git-commit", + }, + { + name: "detects re-published skill and user picks upstream", + isTTY: true, + stubs: func(reg *httpmock.Registry) { + stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123") + stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123", + singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA")) + stubContentsAPI(reg, "monalisa", "skills-repo", + "skills/git-commit/SKILL.md", republishedContent) + stubResolveVersion(reg, "monalisa", "original-skills", "v2.0.0", "upstream456") + stubDiscoverTree(reg, "monalisa", "original-skills", "upstream456", + singleSkillTreeJSON("git-commit", "upTreeSHA", "upBlobSHA")) + stubContentsAPI(reg, "monalisa", "original-skills", + "skills/git-commit/SKILL.md", gitCommitContent) + stubInstallFiles(reg, "monalisa", "original-skills", + "upTreeSHA", "upBlobSHA", gitCommitContent) + }, + opts: func(t *testing.T, ios *iostreams.IOStreams, reg *httpmock.Registry) *InstallOptions { + return &InstallOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + GitClient: &git.Client{RepoDir: t.TempDir()}, + Prompter: &prompter.PrompterMock{ + SelectFunc: func(_ string, _ string, choices []string) (int, error) { + require.Len(t, choices, 2) + assert.Contains(t, choices[0], "monalisa/skills-repo") + assert.Contains(t, choices[1], "monalisa/original-skills") + return 1, nil + }, + }, + Telemetry: &telemetry.NoOpService{}, + SkillSource: "monalisa/skills-repo", + SkillName: "git-commit", + Agent: "github-copilot", + Scope: "project", + ScopeChanged: true, + Dir: t.TempDir(), + } + }, + wantStderr: "Redirecting install to monalisa/original-skills", + wantStdout: "Installed git-commit", + }, + { + name: "non-interactive defaults to re-publisher with notice", + isTTY: false, + stubs: func(reg *httpmock.Registry) { + stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123") + stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123", + singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA")) + stubContentsAPI(reg, "monalisa", "skills-repo", + "skills/git-commit/SKILL.md", republishedContent) + stubInstallFiles(reg, "monalisa", "skills-repo", + "treeSHA", "blobSHA", republishedContent) + }, + opts: func(t *testing.T, ios *iostreams.IOStreams, reg *httpmock.Registry) *InstallOptions { + return &InstallOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + GitClient: &git.Client{RepoDir: t.TempDir()}, + Telemetry: &telemetry.NoOpService{}, + SkillSource: "monalisa/skills-repo", + SkillName: "git-commit", + Agent: "github-copilot", + Scope: "project", + ScopeChanged: true, + Dir: t.TempDir(), + } + }, + wantStderr: "use --upstream", + wantStdout: "Installed git-commit", + }, + { + name: "non-interactive with --upstream redirects to upstream", + isTTY: false, + stubs: func(reg *httpmock.Registry) { + stubResolveVersion(reg, "monalisa", "skills-repo", "v1.0.0", "abc123") + stubDiscoverTree(reg, "monalisa", "skills-repo", "abc123", + singleSkillTreeJSON("git-commit", "treeSHA", "blobSHA")) + stubContentsAPI(reg, "monalisa", "skills-repo", + "skills/git-commit/SKILL.md", republishedContent) + stubResolveVersion(reg, "monalisa", "original-skills", "v2.0.0", "upstream456") + stubDiscoverTree(reg, "monalisa", "original-skills", "upstream456", + singleSkillTreeJSON("git-commit", "upTreeSHA", "upBlobSHA")) + stubContentsAPI(reg, "monalisa", "original-skills", + "skills/git-commit/SKILL.md", gitCommitContent) + stubInstallFiles(reg, "monalisa", "original-skills", + "upTreeSHA", "upBlobSHA", gitCommitContent) + }, + opts: func(t *testing.T, ios *iostreams.IOStreams, reg *httpmock.Registry) *InstallOptions { + return &InstallOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }, + GitClient: &git.Client{RepoDir: t.TempDir()}, + Telemetry: &telemetry.NoOpService{}, + SkillSource: "monalisa/skills-repo", + SkillName: "git-commit", + Agent: "github-copilot", + Scope: "project", + ScopeChanged: true, + Dir: t.TempDir(), + Upstream: true, + } + }, + wantStderr: "Redirecting install to monalisa/original-skills", + wantStdout: "Installed git-commit", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + tt.stubs(reg) + + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + t.Setenv("USERPROFILE", homeDir) + + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.isTTY) + ios.SetStdinTTY(tt.isTTY) + ios.SetStderrTTY(tt.isTTY) + opts := tt.opts(t, ios, reg) + + err := installRun(opts) + + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + + require.NoError(t, err) + if tt.wantStdout != "" { + assert.Contains(t, stdout.String(), tt.wantStdout) + } + if tt.wantStderr != "" { + assert.Contains(t, stderr.String(), tt.wantStderr) + } + }) + } +} diff --git a/skills/gh-skill/SKILL.md b/skills/gh-skill/SKILL.md new file mode 100644 index 00000000000..e0db2feaed4 --- /dev/null +++ b/skills/gh-skill/SKILL.md @@ -0,0 +1,116 @@ +--- +name: gh-skill +description: Manage agent skills with gh skill. Use this skill to discover, preview, install, update, and publish Agent Skills so an agent can self-manage the skills available in its environment. +--- + +# Managing skills with `gh skill` + +`gh skill` installs, previews, searches, updates, and publishes +[Agent Skills](https://agentskills.io). An agent can use it to keep its +own skill set in sync with one or more GitHub repositories. + +The command is also aliased as `gh skills`. Prefer the canonical singular +`gh skill` in scripts and docs. + +## Search + +```bash +gh skill search # free-text search +gh skill search --owner # restrict to one owner +gh skill search --limit 20 --page 2 +gh skill search --json skillName,repo,description +``` + +## Preview before installing + +```bash +gh skill preview / +gh skill preview / @v1.2.0 # pin a version +``` + +## Install + +```bash +gh skill install / +gh skill install / @v1.2.0 +gh skill install / skills// # exact path, fastest +gh skill install ./local-skills-repo --from-local +``` + +`/` and `` are both required. + +Useful flags: + +- `--agent ` - target host (e.g. `github-copilot`, `claude-code`, + `cursor`, `codex`, `gemini-cli`). Repeat for multiple. Default is + `github-copilot` when non-interactive. You should know what agent you are, + so set this appropriately to install for yourself. +- `--scope project|user` - `project` (default) writes inside the current + git repo; `user` writes to the home directory and applies everywhere. +- `--pin ` - pin to a tag, branch, or commit SHA. Mutually exclusive + with `--from-local` and with inline `@version` syntax. +- `--allow-hidden-dirs` - also discover skills under dot-directories such + as `.claude/skills/`. Don't use this unless you need to, it comes with risks. +- `--force` - overwrite an existing install. + +## Update + +```bash +gh skill update --all # update every installed skill +gh skill update # update one +gh skill update --force +gh skill update --unpin # drop the pin and move to latest +``` + +## Publish + +Publishing turns a repo into a discoverable skill source. Skills are +discovered with these conventions: + +- `skills//SKILL.md` +- `skills///SKILL.md` +- `/SKILL.md` (root-level) +- `plugins//skills//SKILL.md` + +Each `SKILL.md` needs YAML frontmatter: + +```yaml +--- +name: my-skill # must equal the directory name +description: One sentence... # required, recommended <= 1024 chars +license: MIT # optional but recommended +--- +``` + +### Validate, then publish + +```bash +gh skill publish --dry-run # validate only, no release +gh skill publish --dry-run ./path/to/repo # validate a specific dir +gh skill publish --fix # auto-strip install metadata +gh skill publish --tag v1.0.0 # non-interactive publish +gh skill publish # interactive publish flow +``` + +`--fix` and `--dry-run` are mutually exclusive. `--fix` only rewrites +install-injected `metadata.github-*` keys and does not publish; commit +the result and re-run `publish`. + +The publish flow will: + +1. Add the `agent-skills` topic to the repo (so search can find it). +2. Use `--tag` (or prompt for one in a TTY). +3. Auto-push any unpushed commits. +4. Create a GitHub release with auto-generated notes. + +Always pass `--tag` so it doesn't fall through to the interactive flow. + +## Self-management pattern for agents + +A reasonable loop: + +1. `gh skill search --json skillName,repo,namespace` +2. `gh skill preview ` to inspect the `SKILL.md`. +3. `gh skill install --agent --pin ` for a + reproducible install. +4. Periodically `gh skill update --all` to refresh. \ No newline at end of file diff --git a/skills/gh/SKILL.md b/skills/gh/SKILL.md new file mode 100644 index 00000000000..34cce8bc2d8 --- /dev/null +++ b/skills/gh/SKILL.md @@ -0,0 +1,81 @@ +--- +name: gh +description: Patterns for invoking the GitHub CLI (gh) from agents. Covers structured output, pagination, repo targeting, search vs list, gh api fallback. +--- + +# Reference + +## Interactivity policy + +`gh` already does the right thing in non-TTY contexts: it skips the pager, +strips ANSI color, and errors out fast with a helpful message instead of +prompting (e.g. `must provide --title and --body when not running interactively`). +You don't need to defensively set `GH_PAGER` or pass `--no-pager` (no such +flag exists). + +## Parsing JSON + +Human output from `gh` is column-formatted. If you want structured data: + +- Add `--json field1,field2,...` for structured output. +- Run a command with `--json` and **no field list** to print the full set of + available fields, then pick what you need. +- Use `--jq ''` for filtering without piping through a separate `jq`. +- Use `--template ''` (alongside `--json`) when you want shaped + text output. Note that `--template`/`-T` collides with a body-template flag + on a few commands (e.g. `gh pr create -T`, `gh issue create -T`); always + check `--help` before assuming which one you're hitting. + +## Pagination and silent truncation + +List commands cap results. + +- `gh issue list`, `gh pr list`, `gh search ...`: pass `-L N` (`--limit N`). + The default is usually 30. +- `gh issue list` / `gh pr list` do not expose aggregate totals like + `totalCount` via `--json`. If you need a true total, use `gh api graphql` + to query `totalCount`; otherwise, treat `-L` as the cap for the current call. +- For raw API calls use `gh api --paginate `. Combine with + `--jq` and (optionally) `--slurp` to assemble one array. + +## Repo targeting + +`gh` infers the repo from the cwd's git remotes. + +Pass `--repo OWNER/REPO` (`-R`) to override the resolved CWD repo. + +## Search vs list + +- `gh search issues|prs|code|repos|commits|users` uses GitHub's search + index and accepts the full search syntax (`is:open`, `author:`, + `label:`, `repo:owner/name`, `in:title`, ...). Pass the entire query as + one quoted string, the same way you would for `--search`: + `gh search issues "is:open author:foo repo:cli/cli"`. Prefer it for + anything cross-repo or filtered by author/label. +- `gh issue list --search "..."` and `gh pr list --search "..."` accept + the same syntax but are scoped to one repo. + +## Fall back to `gh api` for anything `--json` doesn't expose + +Sometimes useful data isn't on the typed commands. Examples: + +- Review-thread comments on a PR: `gh api repos/{owner}/{repo}/pulls/{n}/comments` + (the `--comments` flag on `gh pr view` shows issue-level comments only). +- Arbitrary GraphQL: `gh api graphql -f query='...' -F var=value`. +- REST shortcuts: `gh api repos/{owner}/{repo}/...` - note the + `{owner}/{repo}` placeholder is filled in for you when run from a repo + with detected remotes; pass them literally if you want determinism. + +## Authentication + +- `gh auth status` prints the active host(s), user, and which env var (if + any) is being honored. +- `gh auth status --json` is supported. + +## Other notes + +- `gh pr checkout ` switches branches. Use `gh pr diff ` or + `gh pr view ` if you only need to read. +- `NO_COLOR`, `CLICOLOR_FORCE`, and `GH_FORCE_TTY` are honored. Set + `GH_FORCE_TTY=1` if you want TTY-style output (colors, tables, the + pager, interactivity) inside an agent harness; leave it unset unless needed.