diff --git a/builder/git/gitserver/gitaly/file.go b/builder/git/gitserver/gitaly/file.go index 58c650185..8017a1fea 100644 --- a/builder/git/gitserver/gitaly/file.go +++ b/builder/git/gitserver/gitaly/file.go @@ -43,6 +43,13 @@ func (c *Client) GetRepoFileRaw(ctx context.Context, req gitserver.GetRepoInfoBy RelativePath: relativePath, } + if req.GitAlternateObjectDirectoriesRelative != nil { + repository.GitAlternateObjectDirectories = req.GitAlternateObjectDirectoriesRelative + } + if req.GitObjectDirectoryRelative != "" { + repository.GitObjectDirectory = req.GitObjectDirectoryRelative + } + req.Path = strings.TrimPrefix(req.Path, "/") if req.Ref == "" { diff --git a/builder/git/gitserver/types.go b/builder/git/gitserver/types.go index e6ed26ebe..329548817 100644 --- a/builder/git/gitserver/types.go +++ b/builder/git/gitserver/types.go @@ -170,7 +170,9 @@ type GetRepoInfoByPathReq struct { RepoType types.RepositoryType `json:"repo_type"` File bool `json:"file"` // limit file size, don't return file content if file size is greater than MaxFileSize - MaxFileSize int64 `json:"max_file_size"` + MaxFileSize int64 `json:"max_file_size"` + GitObjectDirectoryRelative string `json:"git_object_directory_relative"` + GitAlternateObjectDirectoriesRelative []string `json:"git_alternate_object_directories_relative"` } type GetRepoAllFilesReq struct { diff --git a/component/checker/skill_file_checker.go b/component/checker/skill_file_checker.go new file mode 100644 index 000000000..117854860 --- /dev/null +++ b/component/checker/skill_file_checker.go @@ -0,0 +1,82 @@ +package checker + +import ( + "context" + "errors" + "fmt" + "regexp" + "strings" + + "opencsg.com/csghub-server/builder/git" + "opencsg.com/csghub-server/builder/git/gitserver" + "opencsg.com/csghub-server/builder/store/database" + "opencsg.com/csghub-server/common/config" + "opencsg.com/csghub-server/common/types" +) + +type SkillFileChecker struct { + repoStore database.RepoStore + gitServer gitserver.GitServer + config *config.Config +} + +func NewSkillFileChecker(config *config.Config) (GitCallbackChecker, error) { + git, err := git.NewGitServer(config) + if err != nil { + return nil, fmt.Errorf("failed to create git server: %w", err) + } + return &SkillFileChecker{ + repoStore: database.NewRepoStore(), + gitServer: git, + config: config, + }, nil +} + +func (c *SkillFileChecker) Check(ctx context.Context, req types.GitalyAllowedReq) (bool, error) { + var ref string + repoType, namespace, name := req.GetRepoTypeNamespaceAndName() + + // Only check skill repositories + if repoType != types.SkillRepo { + return true, nil + } + + repo, err := c.repoStore.FindByPath(ctx, repoType, namespace, name) + if err != nil { + return false, fmt.Errorf("failed to find repo, err: %v", err) + } + if repo == nil { + return false, errors.New("repo not found") + } + changes := strings.Split(req.Changes, " ") + if len(changes) > 1 { + ref = changes[1] + } + + // Check if SKILL.md exists and has required metadata in the new content + skillsContent, err := c.gitServer.GetRepoFileRaw(ctx, gitserver.GetRepoInfoByPathReq{ + Namespace: namespace, + Name: name, + RepoType: repoType, + GitObjectDirectoryRelative: req.GitEnv.GitObjectDirectoryRelative, + GitAlternateObjectDirectoriesRelative: req.GitEnv.GitAlternateObjectDirectoriesRelative, + Path: "SKILL.md", + Ref: ref, + }) + if err != nil { + // SKILL.md not found in new content, reject push + return false, fmt.Errorf("skill repository must have a SKILL.md file with name and description metadata") + } + + // Check if SKILL.md is in the correct YAML format with name and description + pattern := `^---\s*\nname:\s*.+\s*\ndescription:\s*.+\s*---$` + matched, err := regexp.MatchString(pattern, skillsContent) + if err != nil { + return false, fmt.Errorf("failed to check SKILL.md format: %w", err) + } + if !matched { + return false, fmt.Errorf("SKILL.md must be in YAML format with name and description fields") + } + + return true, nil +} diff --git a/component/internal.go b/component/internal.go index 176ae5cca..a13227550 100644 --- a/component/internal.go +++ b/component/internal.go @@ -66,8 +66,12 @@ func NewInternalComponent(config *config.Config) (InternalComponent, error) { if err != nil { return nil, fmt.Errorf("failed to create lfs exists checker: %w", err) } + skillFileChecker, err := checker.NewSkillFileChecker(config) + if err != nil { + return nil, fmt.Errorf("failed to create skill file checker: %w", err) + } - c.callbackCheckers = append(c.callbackCheckers, fileSizeChecker, lfsExistsChecker) + c.callbackCheckers = append(c.callbackCheckers, fileSizeChecker, lfsExistsChecker, skillFileChecker) c.gitServer = git return c, nil } diff --git a/component/skill.go b/component/skill.go index 749400a17..4be83fad6 100644 --- a/component/skill.go +++ b/component/skill.go @@ -192,8 +192,12 @@ func (c *skillComponentImpl) setupCreateRequest(req *types.CreateSkillReq) { req.Readme = generateReadmeData(req.License) } -// initializeCommitFiles initializes commit files with README and .gitattributes +// initializeCommitFiles initializes commit files with README, .gitattributes, and SKILL.md func (c *skillComponentImpl) initializeCommitFiles(req *types.CreateSkillReq) []types.CommitFile { + skillsContent := fmt.Sprintf(`--- +name: %s +description: %s +---`, req.Name, req.Description) return []types.CommitFile{ { Content: req.Readme, @@ -203,6 +207,10 @@ func (c *skillComponentImpl) initializeCommitFiles(req *types.CreateSkillReq) [] Content: skillGitattributesContent, Path: types.GitattributesFileName, }, + { + Content: skillsContent, + Path: "SKILL.md", + }, } } diff --git a/component/skill_test.go b/component/skill_test.go index 2f769c052..a42826ce4 100644 --- a/component/skill_test.go +++ b/component/skill_test.go @@ -49,6 +49,10 @@ func TestSkillComponent_Create(t *testing.T) { crq.Readme = generateReadmeData(req.License) crq.RepoType = types.SkillRepo crq.DefaultBranch = "main" + skillsContent := fmt.Sprintf(`--- +name: %s +description: %s +---`, crq.Name, crq.Description) crq.CommitFiles = []types.CommitFile{ { Content: crq.Readme, @@ -58,6 +62,10 @@ func TestSkillComponent_Create(t *testing.T) { Content: skillGitattributesContent, Path: types.GitattributesFileName, }, + { + Content: skillsContent, + Path: "SKILL.md", + }, } var wg sync.WaitGroup @@ -384,6 +392,10 @@ func TestSkillComponent_CreateWithGitURL(t *testing.T) { crq.Readme = generateReadmeData(req.License) crq.RepoType = types.SkillRepo crq.DefaultBranch = "main" + skillsContent := fmt.Sprintf(`--- +name: %s +description: %s +---`, crq.Name, crq.Description) crq.CommitFiles = []types.CommitFile{ { Content: crq.Readme, @@ -393,6 +405,10 @@ func TestSkillComponent_CreateWithGitURL(t *testing.T) { Content: skillGitattributesContent, Path: types.GitattributesFileName, }, + { + Content: skillsContent, + Path: "SKILL.md", + }, } var wg sync.WaitGroup @@ -497,7 +513,11 @@ func TestSkillComponent_CreateWithBatchCommit(t *testing.T) { // In the actual code, the Create method creates commitFiles with README and .gitattributes // Then appends req.CommitFiles to it, and sets req.CommitFiles = commitFiles // So we need to create a CommitFilesReq with all these files - // First, create the initial files (README and .gitattributes) + // First, create the initial files (README, .gitattributes, and SKILL.md) + skillsContent := fmt.Sprintf(`--- +name: %s +description: %s +---`, crq.Name, crq.Description) initialFiles := []types.CommitFile{ { Content: crq.Readme, @@ -507,6 +527,10 @@ func TestSkillComponent_CreateWithBatchCommit(t *testing.T) { Content: skillGitattributesContent, Path: types.GitattributesFileName, }, + { + Content: skillsContent, + Path: "SKILL.md", + }, } // Add the additional files from the request @@ -524,7 +548,7 @@ func TestSkillComponent_CreateWithBatchCommit(t *testing.T) { } // In the actual code, the Create method sets req.CommitFiles = commitFiles - // where commitFiles is README + .gitattributes + req.CommitFiles + // where commitFiles is README + .gitattributes + SKILL.md + req.CommitFiles // So we need to update crq.CommitFiles to match what will be passed to CreateRepo crq.CommitFiles = allFiles @@ -532,12 +556,12 @@ func TestSkillComponent_CreateWithBatchCommit(t *testing.T) { nil, dbrepo, commitFilesReq, nil, ) - // Expect two calls to CommitFiles (one for first 50 files, one for remaining 2 files) + // Expect two calls to CommitFiles (one for first 50 files, one for remaining 3 files) cc.mocks.gitServer.EXPECT().CommitFiles(ctx, mock.MatchedBy(func(req gitserver.CommitFilesReq) bool { return len(req.Files) == 50 })).Return(nil) cc.mocks.gitServer.EXPECT().CommitFiles(ctx, mock.MatchedBy(func(req gitserver.CommitFilesReq) bool { - return len(req.Files) == 2 + return len(req.Files) == 3 })).Return(nil) cc.mocks.stores.SkillMock().EXPECT().CreateAndUpdateRepoPath(ctx, database.Skill{ Repository: dbrepo,