Merge pull request #13264 from SamMorrowDrums/sammorrowdrums/skill-ghec-data-residency

feat(skills): support GHEC with data residency hosts
This commit is contained in:
Sam Morrow 2026-04-24 11:45:28 +02:00 committed by GitHub
commit 2c1f5b2f72
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 52 additions and 19 deletions

View file

@ -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))
}
}

View file

@ -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,

View file

@ -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")

View file

@ -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 {

View file

@ -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")
}

View file

@ -1173,7 +1173,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",

View file

@ -413,7 +413,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) {

View file

@ -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 {

View file

@ -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) {

View file

@ -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) {

View file

@ -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")
},
},
{