Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cli/add_package_manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ func parseRepositoryPackageSpec(spec string) (*RepoSpec, bool, error) {
if len(slashParts) < 2 || slashParts[0] == "" || slashParts[1] == "" {
return nil, false, nil
}
if !parser.IsValidGitHubIdentifier(slashParts[0]) || !parser.IsValidGitHubIdentifier(slashParts[1]) {
if !parser.IsValidGitHubIdentifier(slashParts[0]) || !parser.IsValidGitHubRepositoryName(slashParts[1]) {
return nil, false, nil
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/cli/add_package_manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,12 @@ func TestParseRepositoryPackageSpec(t *testing.T) {
wantRepoSlug: "owner/repo",
wantVersion: "release/2026.05.27-rc_1",
},
{
name: "repo only package with long hyphenated repo name",
spec: "owner/this-repository-name-is-significantly-longer-than-thirty-nine",
wantOK: true,
wantRepoSlug: "owner/this-repository-name-is-significantly-longer-than-thirty-nine",
},
{
name: "nested package path",
spec: "owner/repo/packages/repo-assist",
Expand Down
9 changes: 9 additions & 0 deletions pkg/cli/add_wildcard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ func TestParseWorkflowSpecWithWildcard(t *testing.T) {
expectedVer: "feature/github-agentic-workflows",
expectedPath: "agentic-workflows/pr-review.md",
},
{
name: "direct_workflow_path_with_long_hyphenated_repo_name",
spec: "owner/long-repository-name-with-many-hyphens-in-it/agentic-workflows/business-deviation-tracker.md",
expectWildcard: false,
expectError: false,
expectedRepo: "owner/long-repository-name-with-many-hyphens-in-it",
expectedVer: "",
expectedPath: "agentic-workflows/business-deviation-tracker.md",
},
{
name: "invalid_spec_too_few_parts",
spec: "owner/*",
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func parseGitHubURL(spec string) (*WorkflowSpec, error) {
}

// Validate owner and repo
if !parser.IsValidGitHubIdentifier(owner) || !parser.IsValidGitHubIdentifier(repo) {
if !parser.IsValidGitHubIdentifier(owner) || !parser.IsValidGitHubRepositoryName(repo) {
return nil, fmt.Errorf("invalid GitHub URL: '%s/%s' does not look like a valid GitHub repository", owner, repo)
}
Comment on lines 208 to 211

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in e31d8ee. pkg/cli/spec_github_url_test.go now includes a parseGitHubURL regression case with a long hyphenated repo segment (>39 chars).


Expand Down Expand Up @@ -371,7 +371,7 @@ func parseWorkflowSpec(spec string) (*WorkflowSpec, error) {
}

// Basic validation that owner and repo look like GitHub identifiers
if !parser.IsValidGitHubIdentifier(owner) || !parser.IsValidGitHubIdentifier(repo) {
if !parser.IsValidGitHubIdentifier(owner) || !parser.IsValidGitHubRepositoryName(repo) {
return nil, fmt.Errorf("invalid workflow specification: '%s/%s' does not look like a valid GitHub repository", owner, repo)
}

Expand Down
9 changes: 9 additions & 0 deletions pkg/cli/spec_github_url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ func TestParseGitHubURL(t *testing.T) {
wantVersion: "v2.0.0",
wantErr: false,
},
{
name: "blob URL with long hyphenated repo name",
url: "https://github.com/owner/this-repository-name-is-significantly-longer-than-thirty-nine/blob/main/workflows/release.md",
wantRepo: "owner/this-repository-name-is-significantly-longer-than-thirty-nine",
wantWorkflowPath: "workflows/release.md",
wantWorkflowName: "release",
wantVersion: "main",
wantErr: false,
},
{
name: "invalid - non-github domain",
url: "https://gitlab.com/owner/repo/blob/main/workflows/test.md",
Expand Down
3 changes: 2 additions & 1 deletion pkg/parser/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ The package is designed for use both in the main CLI binary and in WebAssembly c
| `ParseRunURLExtended` | `func(input string) (*GitHubURLComponents, error)` | Parses a workflow run URL (extended formats) |
| `ParsePRURL` | `func(prURL string) (owner, repo string, prNumber int, err error)` | Parses a pull request URL |
| `ParseRepoFileURL` | `func(fileURL string) (owner, repo, ref, filePath string, err error)` | Parses a repository file URL |
| `IsValidGitHubIdentifier` | `func(s string) bool` | Validates a GitHub username/org/repo name |
| `IsValidGitHubIdentifier` | `func(s string) bool` | Validates a GitHub username/org identifier |
| `IsValidGitHubRepositoryName` | `func(s string) bool` | Validates a GitHub repository name |
| `GetGitHubHost` | `func() string` | Returns the GitHub host (supports GHES via `GH_HOST`) |
| `GetGitHubHostForRepo` | `func(owner, repo string) string` | Returns the GitHub host for a specific repo |
| `GetGitHubToken` | `func() (string, error)` | Returns the GitHub auth token from the environment |
Expand Down
24 changes: 19 additions & 5 deletions pkg/parser/github_urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,15 @@ func ParseRepoFileURL(fileURL string) (owner, repo, ref, filePath string, err er
}
}

// IsValidGitHubIdentifier checks if a string is a valid GitHub username or repository name
func IsValidGitHubIdentifier(s string) bool {
// GitHub identifiers can contain alphanumeric characters, hyphens, and underscores
// They cannot start or end with a hyphen and must be 1-39 characters long
if len(s) == 0 || len(s) > 39 {
const (
maxGitHubOwnerIdentifierLength = 39
maxGitHubRepositoryNameLength = 100
)

func isValidGitHubNameWithMaxLength(s string, maxLength int) bool {
// GitHub identifiers can contain alphanumeric characters, hyphens, and underscores.
// They cannot start or end with a hyphen.
if len(s) == 0 || len(s) > maxLength {
return false
}
if s[0] == '-' || s[len(s)-1] == '-' {
Expand All @@ -369,3 +373,13 @@ func IsValidGitHubIdentifier(s string) bool {
}
return true
}

// IsValidGitHubIdentifier checks if a string is a valid GitHub user or organization identifier.
func IsValidGitHubIdentifier(s string) bool {
return isValidGitHubNameWithMaxLength(s, maxGitHubOwnerIdentifierLength)
}
Comment on lines +377 to +380

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in e31d8ee. I corrected the parser API docs in pkg/parser/README.md to split owner/org vs repository validation and added README-backed spec coverage for IsValidGitHubRepositoryName in pkg/parser/spec_test.go.


// IsValidGitHubRepositoryName checks if a string is a valid GitHub repository name.
func IsValidGitHubRepositoryName(s string) bool {
return isValidGitHubNameWithMaxLength(s, maxGitHubRepositoryNameLength)
}
38 changes: 38 additions & 0 deletions pkg/parser/github_urls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,44 @@ func TestIsValidGitHubIdentifier(t *testing.T) {
}
}

func TestIsValidGitHubRepositoryName(t *testing.T) {
tests := []struct {
name string
input string
want bool
}{
{
name: "Valid short name",
input: "repo",
want: true,
},
{
name: "Valid long hyphenated name",
input: "long-repository-name-with-many-hyphens-that-exceeds-thirty-nine-characters",
want: true,
},
{
name: "Invalid too long name",
input: "this-repository-name-is-intentionally-very-long-to-exceed-the-github-repository-name-limit-of-one-hundred-characters-total",
want: false,
},
{
name: "Invalid starts with hyphen",
input: "-repo",
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := IsValidGitHubRepositoryName(tt.input)
if got != tt.want {
t.Errorf("IsValidGitHubRepositoryName() = %v, want %v", got, tt.want)
}
})
}
}

// TestParseGitHubURL_AdditionalEdgeCases tests additional edge cases for comprehensive coverage
func TestParseGitHubURL_AdditionalEdgeCases(t *testing.T) {
tests := []struct {
Expand Down
38 changes: 37 additions & 1 deletion pkg/parser/spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ func TestSpec_PublicAPI_LevenshteinDistance(t *testing.T) {
// TestSpec_PublicAPI_IsValidGitHubIdentifier validates the documented behavior
// of IsValidGitHubIdentifier as described in the package README.md.
//
// Specification: Validates a GitHub username/org/repo name.
// Specification: Validates a GitHub username/org identifier.
func TestSpec_PublicAPI_IsValidGitHubIdentifier(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -423,6 +423,11 @@ func TestSpec_PublicAPI_IsValidGitHubIdentifier(t *testing.T) {
input: "owner/repo",
expected: false,
},
{
name: "owner name over 39 chars is invalid",
input: "this-owner-name-is-longer-than-thirty-nine",
expected: false,
},
}

for _, tt := range tests {
Expand All @@ -434,6 +439,37 @@ func TestSpec_PublicAPI_IsValidGitHubIdentifier(t *testing.T) {
}
}

// TestSpec_PublicAPI_IsValidGitHubRepositoryName validates the documented behavior
// of IsValidGitHubRepositoryName as described in the package README.md.
//
// Specification: Validates a GitHub repository name.
func TestSpec_PublicAPI_IsValidGitHubRepositoryName(t *testing.T) {
tests := []struct {
name string
input string
expected bool
}{
{
name: "repo name over owner length and within 100 chars is valid",
input: "this-repository-name-is-significantly-longer-than-thirty-nine",
expected: true,
},
{
name: "repo name over 100 chars is invalid",
input: "this-repository-name-is-way-too-long-because-it-exceeds-one-hundred-characters-when-you-keep-adding-more",
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := IsValidGitHubRepositoryName(tt.input)
assert.Equal(t, tt.expected, result,
"IsValidGitHubRepositoryName(%q) should match documented behavior", tt.input)
})
}
}

// TestSpec_PublicAPI_IsMCPType validates the documented behavior of IsMCPType
// as described in the package README.md.
//
Expand Down
Loading