From 50f0f8fc687e6382df3af6be09f75d966cfa7169 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Mon, 20 Apr 2026 21:08:18 +0200 Subject: [PATCH] feat(skills): detect re-published skills and offer upstream install When installing a skill whose SKILL.md contains github-repo metadata pointing to a different repository, the CLI detects it as a re-published skill and offers to redirect the install to the upstream source. In interactive mode, the user is prompted to choose between the re-publisher (default) and the upstream. In non-interactive mode, the install proceeds from the re-publisher with a notice. The --upstream flag skips the prompt and redirects to upstream directly, enabling non-interactive upstream installs in CI/scripts. If the user chooses upstream, the install restarts from that repo, resolving the latest version and discovering skills fresh. A skill_upstream_redirect telemetry event is emitted to track redirects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/cmd/skills/install/install.go | 133 ++++++++++++++- pkg/cmd/skills/install/install_test.go | 220 +++++++++++++++++++++++++ 2 files changed, 352 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/skills/install/install.go b/pkg/cmd/skills/install/install.go index d792501fd..a22249225 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 e4bd074bd..b7aa956c5 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) + } + }) + } +}