From 63262dce8b38243d6651d4620d0010c9075415da Mon Sep 17 00:00:00 2001 From: sammorrowdrums Date: Wed, 22 Apr 2026 22:07:30 +0200 Subject: [PATCH] feat(skills): support GHEC with data residency hosts Widen ValidateSupportedHost to accept tenancy hosts (*.ghe.com) alongside github.com. GHEC with data residency uses these domains, and all skill subcommands (search, install, preview, publish, update) now allow them. GitHub Enterprise Server remains unsupported and is explicitly rejected with a clear error message. Also fix the lockfile writer to use the actual host when constructing SourceURL instead of hardcoding github.com. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/skills/installer/installer.go | 4 +-- internal/skills/lockfile/lockfile.go | 5 ++-- internal/skills/lockfile/lockfile_test.go | 31 ++++++++++++++++++++--- internal/skills/source/source.go | 15 ++++++++--- internal/skills/source/source_test.go | 4 ++- pkg/cmd/skills/install/install_test.go | 2 +- pkg/cmd/skills/preview/preview_test.go | 2 +- pkg/cmd/skills/publish/publish.go | 2 +- pkg/cmd/skills/publish/publish_test.go | 2 +- pkg/cmd/skills/search/search_test.go | 2 +- pkg/cmd/skills/update/update_test.go | 2 +- 11 files changed, 52 insertions(+), 19 deletions(-) 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") }, }, {