diff --git a/internal/skills/installer/installer.go b/internal/skills/installer/installer.go index e27d35f5b..6072b98ca 100644 --- a/internal/skills/installer/installer.go +++ b/internal/skills/installer/installer.go @@ -76,7 +76,7 @@ func Install(opts *Options) (*Result, error) { return nil, fmt.Errorf("failed to install skill %q: %w", skill.InstallName(), err) } var warnings []string - if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { + if err := lockfile.RecordInstall(opts.Host, skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err)) } return &Result{Installed: []string{skill.InstallName()}, Dir: targetDir, Warnings: warnings}, nil @@ -129,7 +129,7 @@ func Install(opts *Options) (*Result, error) { } installed = append(installed, r.name) skill := opts.Skills[i] - if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { + if err := lockfile.RecordInstall(opts.Host, skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err)) } } diff --git a/internal/skills/lockfile/lockfile.go b/internal/skills/lockfile/lockfile.go index 42d2abb34..2e6697234 100644 --- a/internal/skills/lockfile/lockfile.go +++ b/internal/skills/lockfile/lockfile.go @@ -10,6 +10,7 @@ import ( "time" "github.com/cli/cli/v2/internal/flock" + "github.com/cli/cli/v2/internal/ghinstance" ) const ( @@ -93,7 +94,7 @@ func writeTo(f *os.File, lf *file) error { // RecordInstall adds or updates a skill entry in the lock file. // It uses a file-based lock to prevent concurrent read-modify-write races // when multiple install processes run simultaneously. -func RecordInstall(skillName, owner, repo, skillPath, treeSHA, pinnedRef string) error { +func RecordInstall(host, skillName, owner, repo, skillPath, treeSHA, pinnedRef string) error { lockPath, err := lockfilePath() if err != nil { return err @@ -124,7 +125,7 @@ func RecordInstall(skillName, owner, repo, skillPath, treeSHA, pinnedRef string) f.Skills[skillName] = entry{ Source: owner + "/" + repo, SourceType: "github", - SourceURL: "https://github.com/" + owner + "/" + repo + ".git", + SourceURL: ghinstance.HostPrefix(host) + owner + "/" + repo + ".git", SkillPath: skillPath, SkillFolderHash: treeSHA, InstalledAt: installedAt, diff --git a/internal/skills/lockfile/lockfile_test.go b/internal/skills/lockfile/lockfile_test.go index d68e9a8f1..7a040a550 100644 --- a/internal/skills/lockfile/lockfile_test.go +++ b/internal/skills/lockfile/lockfile_test.go @@ -24,6 +24,7 @@ func TestRecordInstall(t *testing.T) { tests := []struct { name string setup func(t *testing.T) + host string skill string owner string repo string @@ -35,6 +36,7 @@ func TestRecordInstall(t *testing.T) { }{ { name: "fresh install creates lockfile", + host: "github.com", skill: "code-review", owner: "monalisa", repo: "octocat-skills", @@ -55,8 +57,25 @@ func TestRecordInstall(t *testing.T) { assert.Empty(t, e.PinnedRef) }, }, + { + name: "tenancy host uses correct URL", + host: "mycompany.ghe.com", + skill: "code-review", + owner: "monalisa", + repo: "octocat-skills", + skillPath: "skills/code-review/SKILL.md", + treeSHA: "abc123", + verify: func(t *testing.T, lockPath string) { + t.Helper() + f := readTestLockfile(t, lockPath) + require.Contains(t, f.Skills, "code-review") + e := f.Skills["code-review"] + assert.Equal(t, "https://mycompany.ghe.com/monalisa/octocat-skills.git", e.SourceURL) + }, + }, { name: "install with pinned ref", + host: "github.com", skill: "pr-summary", owner: "hubot", repo: "skills-repo", @@ -73,8 +92,9 @@ func TestRecordInstall(t *testing.T) { name: "multiple skills coexist", setup: func(t *testing.T) { t.Helper() - require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "sha1", "")) + require.NoError(t, RecordInstall("github.com", "code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "sha1", "")) }, + host: "github.com", skill: "issue-triage", owner: "monalisa", repo: "octocat-skills", @@ -107,6 +127,7 @@ func TestRecordInstall(t *testing.T) { require.NoError(t, err) t.Cleanup(unlock) }, + host: "github.com", skill: "code-review", owner: "monalisa", repo: "octocat-skills", @@ -123,6 +144,7 @@ func TestRecordInstall(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Dir(lockPath), 0o755)) require.NoError(t, os.WriteFile(lockPath, []byte("{invalid json"), 0o644)) }, + host: "github.com", skill: "code-review", owner: "monalisa", repo: "octocat-skills", @@ -145,6 +167,7 @@ func TestRecordInstall(t *testing.T) { data, _ := json.Marshal(file{Version: 999, Skills: map[string]entry{"old-skill": {}}}) require.NoError(t, os.WriteFile(lockPath, data, 0o644)) }, + host: "github.com", skill: "code-review", owner: "monalisa", repo: "octocat-skills", @@ -166,7 +189,7 @@ func TestRecordInstall(t *testing.T) { tt.setup(t) } - err := RecordInstall(tt.skill, tt.owner, tt.repo, tt.skillPath, tt.treeSHA, tt.pinnedRef) + err := RecordInstall(tt.host, tt.skill, tt.owner, tt.repo, tt.skillPath, tt.treeSHA, tt.pinnedRef) if tt.wantErr { require.Error(t, err) return @@ -181,10 +204,10 @@ func TestRecordInstall(t *testing.T) { t.Run("update preserves InstalledAt and updates treeSHA", func(t *testing.T) { lockPath := setupTestHome(t) - require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "old-sha", "")) + require.NoError(t, RecordInstall("github.com", "code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "old-sha", "")) firstInstalledAt := readTestLockfile(t, lockPath).Skills["code-review"].InstalledAt - require.NoError(t, RecordInstall("code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "new-sha", "")) + require.NoError(t, RecordInstall("github.com", "code-review", "monalisa", "octocat-skills", "skills/code-review/SKILL.md", "new-sha", "")) entry := readTestLockfile(t, lockPath).Skills["code-review"] assert.Equal(t, "new-sha", entry.SkillFolderHash, "treeSHA should be updated") diff --git a/internal/skills/source/source.go b/internal/skills/source/source.go index 5e8f52888..ff0e5e9d7 100644 --- a/internal/skills/source/source.go +++ b/internal/skills/source/source.go @@ -4,6 +4,8 @@ import ( "fmt" "strings" + ghauth "github.com/cli/go-gh/v2/pkg/auth" + "github.com/cli/cli/v2/internal/ghrepo" ) @@ -48,16 +50,21 @@ func ParseMetadataRepo(meta map[string]interface{}) (ghrepo.Interface, bool, err return repo, true, nil } -// ValidateSupportedHost rejects hosts that are not supported in public preview. +// ValidateSupportedHost rejects hosts that are not supported. +// Supported hosts are github.com and GHEC with data residency (*.ghe.com). +// GitHub Enterprise Server is not currently supported. func ValidateSupportedHost(host string) error { host = normalizeHost(host) if host == "" { return fmt.Errorf("could not determine repository host") } - if host != SupportedHost { - return fmt.Errorf("GitHub Skills currently supports only %s as a host; got %s", SupportedHost, host) + if host == SupportedHost || ghauth.IsTenancy(host) { + return nil } - return nil + if ghauth.IsEnterprise(host) { + return fmt.Errorf("GitHub Skills does not currently support GitHub Enterprise Server; got %s", host) + } + return fmt.Errorf("unsupported host for GitHub Skills: %s", host) } func normalizeHost(host string) string { diff --git a/internal/skills/source/source_test.go b/internal/skills/source/source_test.go index f797591b4..9c2457d3f 100644 --- a/internal/skills/source/source_test.go +++ b/internal/skills/source/source_test.go @@ -72,5 +72,7 @@ func TestParseMetadataRepo(t *testing.T) { func TestValidateSupportedHost(t *testing.T) { require.NoError(t, ValidateSupportedHost("github.com")) - require.ErrorContains(t, ValidateSupportedHost("acme.ghes.com"), "supports only github.com") + require.NoError(t, ValidateSupportedHost("mycompany.ghe.com"), "GHEC data residency tenancy hosts should be accepted") + require.ErrorContains(t, ValidateSupportedHost("acme.ghes.com"), "does not currently support GitHub Enterprise Server") + require.ErrorContains(t, ValidateSupportedHost("github.localhost"), "unsupported host") } diff --git a/pkg/cmd/skills/install/install_test.go b/pkg/cmd/skills/install/install_test.go index b7aa956c5..f6202b977 100644 --- a/pkg/cmd/skills/install/install_test.go +++ b/pkg/cmd/skills/install/install_test.go @@ -1177,7 +1177,7 @@ func TestInstallRun(t *testing.T) { SkillName: "git-commit", } }, - wantErr: "supports only github.com", + wantErr: "does not currently support GitHub Enterprise Server", }, { name: "select all skills in interactive prompt", diff --git a/pkg/cmd/skills/preview/preview_test.go b/pkg/cmd/skills/preview/preview_test.go index be3c86117..7d02ff2ee 100644 --- a/pkg/cmd/skills/preview/preview_test.go +++ b/pkg/cmd/skills/preview/preview_test.go @@ -403,7 +403,7 @@ func TestPreviewRun_UnsupportedHost(t *testing.T) { repo: ghrepo.NewWithHost("github", "awesome-copilot", "acme.ghes.com"), Telemetry: &telemetry.NoOpService{}, }) - require.ErrorContains(t, err, "supports only github.com") + require.ErrorContains(t, err, "does not currently support GitHub Enterprise Server") } func TestPreviewRun_Interactive(t *testing.T) { diff --git a/pkg/cmd/skills/publish/publish.go b/pkg/cmd/skills/publish/publish.go index 3a27fc5a2..c53ea0b6b 100644 --- a/pkg/cmd/skills/publish/publish.go +++ b/pkg/cmd/skills/publish/publish.go @@ -968,7 +968,7 @@ func detectGitHubRemote(gitClient *git.Client, dir string) (*gitHubRemote, error } // parseGitHubURL extracts owner/repo from a GitHub remote URL. -// Only GitHub.com URLs are recognized. +// Only github.com and GHEC data residency (*.ghe.com) URLs are recognized. func parseGitHubURL(rawURL string) (ghrepo.Interface, error) { u, err := git.ParseURL(rawURL) if err != nil { diff --git a/pkg/cmd/skills/publish/publish_test.go b/pkg/cmd/skills/publish/publish_test.go index a4f48dfe6..757cc5126 100644 --- a/pkg/cmd/skills/publish/publish_test.go +++ b/pkg/cmd/skills/publish/publish_test.go @@ -171,7 +171,7 @@ func TestPublishRun_UnsupportedHost(t *testing.T) { HttpClient: func() (*http.Client, error) { return nil, nil }, host: "acme.ghes.com", }) - require.ErrorContains(t, err, "supports only github.com") + require.ErrorContains(t, err, "does not currently support GitHub Enterprise Server") } func TestPublishRun(t *testing.T) { diff --git a/pkg/cmd/skills/search/search_test.go b/pkg/cmd/skills/search/search_test.go index 763ca2124..cf66ba4ac 100644 --- a/pkg/cmd/skills/search/search_test.go +++ b/pkg/cmd/skills/search/search_test.go @@ -33,7 +33,7 @@ func TestSearchRun_UnsupportedHost(t *testing.T) { HttpClient: func() (*http.Client, error) { return &http.Client{}, nil }, Config: func() (gh.Config, error) { return cfg, nil }, }) - require.ErrorContains(t, err, "supports only github.com") + require.ErrorContains(t, err, "does not currently support GitHub Enterprise Server") } func TestNewCmdSearch(t *testing.T) { diff --git a/pkg/cmd/skills/update/update_test.go b/pkg/cmd/skills/update/update_test.go index 86fdcaa80..5d0208043 100644 --- a/pkg/cmd/skills/update/update_test.go +++ b/pkg/cmd/skills/update/update_test.go @@ -171,7 +171,7 @@ func TestScanInstalledSkills(t *testing.T) { require.NoError(t, err) require.Len(t, skills, 1) require.Error(t, skills[0].metadataErr) - assert.Contains(t, skills[0].metadataErr.Error(), "supports only github.com") + assert.Contains(t, skills[0].metadataErr.Error(), "does not currently support GitHub Enterprise Server") }, }, {