From 174635e5118d4301d137b5bcedaa8a1d27da26b7 Mon Sep 17 00:00:00 2001 From: CriSTEM Date: Wed, 15 Apr 2026 00:18:17 -0600 Subject: [PATCH 1/2] feat(github): add owner-scoped aggregation with optional token auth --- internal/config/validator.go | 4 + internal/config/validator_test.go | 71 ++++ internal/fetch/github.go | 541 ++++++++++++++++++++++++++---- internal/fetch/github_test.go | 318 +++++++++++++++++- 4 files changed, 862 insertions(+), 72 deletions(-) diff --git a/internal/config/validator.go b/internal/config/validator.go index 11f0f23..e7d743c 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -483,6 +483,10 @@ func (v *packValidator) validateGitHubScope(file, path string, scope JSONMap) { } } + if owner != "" && len(repos) > 0 { + v.addIssue(file, path, "must define either a non-empty repos list or an owner/owner_type pair, not both") + } + if owner == "" && len(repos) == 0 { v.addIssue( file, diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 10c095e..4daae3d 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -209,6 +209,77 @@ func TestValidatePackAggregatesCrossFileIssues(t *testing.T) { } } +func TestValidatePackRejectsMixedGitHubRepoAndOwnerScopes(t *testing.T) { + t.Parallel() + + pack := Pack{ + Entities: EntitiesFile{ + Version: 1, + DatasetID: "demo", + DatasetName: "Demo", + Entities: []Entity{ + { + ID: "org-1", + Slug: "org-1", + Kind: "organization", + Name: "Org 1", + }, + }, + Relationships: []Relationship{}, + }, + Sources: SourcesFile{ + Version: 1, + DatasetID: "demo", + Sources: []Source{ + { + ID: "github", + Kind: "github", + Enabled: true, + }, + }, + Bindings: []Binding{ + { + ID: "github-binding-1", + EntityID: "org-1", + SourceID: "github", + Enabled: true, + Scope: JSONMap{ + "owner": "example", + "owner_type": "organization", + "repos": []string{"example/repo-1"}, + }, + }, + }, + }, + Alerts: AlertsFile{ + Version: 1, + DatasetID: "demo", + Rules: []AlertRule{}, + }, + Schedules: SchedulesFile{ + Version: 1, + DatasetID: "demo", + Jobs: []ScheduleJob{}, + }, + } + + err := ValidatePack(pack) + if err == nil { + t.Fatal("ValidatePack() error = nil, want validation error") + } + + var validationErr *ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("ValidatePack() error = %T, want *ValidationError", err) + } + + got := joinIssues(validationErr.Issues) + want := `sources.json: bindings[0].scope: must define either a non-empty repos list or an owner/owner_type pair, not both` + if !strings.Contains(got, want) { + t.Fatalf("ValidatePack() issues missing %q\nGot:\n%s", want, got) + } +} + func joinIssues(issues []ValidationIssue) string { lines := make([]string, 0, len(issues)) for _, issue := range issues { diff --git a/internal/fetch/github.go b/internal/fetch/github.go index 4be00b9..2b7121a 100644 --- a/internal/fetch/github.go +++ b/internal/fetch/github.go @@ -8,6 +8,8 @@ import ( "io" "net/http" "net/url" + "os" + "sort" "strconv" "strings" "time" @@ -25,15 +27,19 @@ const ( MetricKeyGitHubOpenIssues = "github.open_issues" MetricKeyGitHubCommits30d = "github.commits_30d" MetricKeyGitHubCommits90d = "github.commits_90d" + MetricKeyGitHubPublicRepoCount = "github.public_repo_count" MetricKeyGitHubLatestReleasePublishedAt = "github.latest_release_published_at" defaultGitHubAPIBaseURL = "https://api.github.com/" defaultGitHubUserAgent = "signalscope-fetch/0.3.0" + githubTokenEnvVar = "GITHUB_TOKEN" ) type GitHubFetcherOptions struct { - Client *GitHubClient - Now func() time.Time + Client *GitHubClient + ClientOptions GitHubClientOptions + TokenLookup func() string + Now func() time.Time } type GitHubFetcher struct { @@ -44,7 +50,20 @@ type GitHubFetcher struct { func NewGitHubFetcher(options GitHubFetcherOptions) *GitHubFetcher { client := options.Client if client == nil { - client = NewGitHubClient(GitHubClientOptions{}) + clientOptions := options.ClientOptions + + tokenLookup := options.TokenLookup + if tokenLookup == nil { + tokenLookup = func() string { + return strings.TrimSpace(os.Getenv(githubTokenEnvVar)) + } + } + + if strings.TrimSpace(clientOptions.Token) == "" { + clientOptions.Token = strings.TrimSpace(tokenLookup()) + } + + client = NewGitHubClient(clientOptions) } now := options.Now @@ -58,6 +77,20 @@ func NewGitHubFetcher(options GitHubFetcherOptions) *GitHubFetcher { } } +type githubBindingScopeKind string + +const ( + githubBindingScopeRepos githubBindingScopeKind = "repos" + githubBindingScopeOwner githubBindingScopeKind = "owner" +) + +type githubBindingScope struct { + Kind githubBindingScopeKind + Repositories []githubRepositoryRef + Owner string + OwnerType string +} + type githubRepositoryRef struct { Owner string Name string @@ -67,17 +100,46 @@ func (repository githubRepositoryRef) FullName() string { return repository.Owner + "/" + repository.Name } +type githubRepositoryOwner struct { + Login string `json:"login"` +} + type githubRepository struct { - FullName string `json:"full_name"` - StargazersCount int `json:"stargazers_count"` - ForksCount int `json:"forks_count"` - OpenIssuesCount int `json:"open_issues_count"` + Name string `json:"name"` + FullName string `json:"full_name"` + StargazersCount int `json:"stargazers_count"` + ForksCount int `json:"forks_count"` + OpenIssuesCount int `json:"open_issues_count"` + Private bool `json:"private"` + Fork bool `json:"fork"` + Archived bool `json:"archived"` + Owner githubRepositoryOwner `json:"owner"` +} + +func (repository githubRepository) RepositoryRef() (githubRepositoryRef, error) { + if owner := strings.TrimSpace(repository.Owner.Login); owner != "" && strings.TrimSpace(repository.Name) != "" { + return githubRepositoryRef{ + Owner: owner, + Name: strings.TrimSpace(repository.Name), + }, nil + } + + if strings.TrimSpace(repository.FullName) != "" { + return parseGitHubRepositoryRef(repository.FullName) + } + + return githubRepositoryRef{}, fmt.Errorf("repository reference is missing owner/name fields") } type githubRelease struct { PublishedAt string `json:"published_at"` } +type githubRepositoryFetchOptions struct { + IncludeContributors bool + RequireLatestRelease bool +} + type githubRepositoryMetrics struct { ScopeKey string Stars int @@ -86,6 +148,7 @@ type githubRepositoryMetrics struct { OpenIssues int Commits30d int Commits90d int + PublicRepoCount int LatestReleasePublishedAt string } @@ -118,7 +181,7 @@ func (fetcher *GitHubFetcher) Fetch(ctx context.Context, request Request) (Resul return Result{}, fmt.Errorf("source ID is required") } - repositories, err := parseGitHubRepositoryRefs(request.Binding) + scope, err := parseGitHubBindingScope(request.Binding) if err != nil { return Result{}, err } @@ -128,46 +191,81 @@ func (fetcher *GitHubFetcher) Fetch(ctx context.Context, request Request) (Resul capturedAt = fetcher.now().UTC() } - snapshots := make([]githubSnapshotRecord, 0, len(repositories)*7) + switch scope.Kind { + case githubBindingScopeRepos: + snapshots := make([]githubSnapshotRecord, 0, len(scope.Repositories)*7) + + for _, repositoryRef := range scope.Repositories { + metrics, err := fetcher.fetchRepositoryMetrics( + ctx, + repositoryRef, + capturedAt, + githubRepositoryFetchOptions{ + IncludeContributors: true, + RequireLatestRelease: true, + }, + ) + if err != nil { + return Result{}, fmt.Errorf( + "binding %q repo %q: %w", + request.Binding.ID, + repositoryRef.FullName(), + err, + ) + } - for _, repositoryRef := range repositories { - metrics, err := fetcher.fetchRepositoryMetrics(ctx, repositoryRef, capturedAt) + snapshots = append(snapshots, buildGitHubRepositorySnapshotRecords(metrics)...) + } + + if err := persistGitHubMetricSnapshots(ctx, request.DB, request, capturedAt, snapshots); err != nil { + return Result{}, fmt.Errorf("persist metric snapshots: %w", err) + } + + return Result{ + RecordsWritten: len(scope.Repositories), + MetricsWritten: len(snapshots), + ContentItemsWritten: 0, + }, nil + + case githubBindingScopeOwner: + metrics, err := fetcher.fetchOwnerMetrics(ctx, scope, capturedAt) if err != nil { - return Result{}, fmt.Errorf( - "binding %q repo %q: %w", - request.Binding.ID, - repositoryRef.FullName(), - err, - ) + return Result{}, fmt.Errorf("binding %q owner %q: %w", request.Binding.ID, scope.Owner, err) } - snapshots = append(snapshots, buildGitHubSnapshotRecords(metrics)...) - } + snapshots := buildGitHubOwnerSnapshotRecords(metrics) + if err := persistGitHubMetricSnapshots(ctx, request.DB, request, capturedAt, snapshots); err != nil { + return Result{}, fmt.Errorf("persist metric snapshots: %w", err) + } - if err := persistGitHubMetricSnapshots(ctx, request.DB, request, capturedAt, snapshots); err != nil { - return Result{}, fmt.Errorf("persist metric snapshots: %w", err) - } + return Result{ + RecordsWritten: metrics.PublicRepoCount, + MetricsWritten: len(snapshots), + ContentItemsWritten: 0, + }, nil - return Result{ - RecordsWritten: len(repositories), - MetricsWritten: len(snapshots), - ContentItemsWritten: 0, - }, nil + default: + return Result{}, fmt.Errorf("binding %q has unsupported GitHub scope", request.Binding.ID) + } } func (fetcher *GitHubFetcher) fetchRepositoryMetrics( ctx context.Context, repositoryRef githubRepositoryRef, capturedAt time.Time, + options githubRepositoryFetchOptions, ) (githubRepositoryMetrics, error) { repository, err := fetcher.client.Repository(ctx, repositoryRef) if err != nil { return githubRepositoryMetrics{}, err } - contributors, err := fetcher.client.CountContributors(ctx, repositoryRef) - if err != nil { - return githubRepositoryMetrics{}, err + contributors := 0 + if options.IncludeContributors { + contributors, err = fetcher.client.CountContributors(ctx, repositoryRef) + if err != nil { + return githubRepositoryMetrics{}, err + } } commits30d, err := fetcher.client.CountCommitsSince(ctx, repositoryRef, capturedAt.Add(-30*24*time.Hour)) @@ -184,7 +282,7 @@ func (fetcher *GitHubFetcher) fetchRepositoryMetrics( if err != nil { return githubRepositoryMetrics{}, err } - if strings.TrimSpace(latestReleasePublishedAt) == "" { + if options.RequireLatestRelease && strings.TrimSpace(latestReleasePublishedAt) == "" { return githubRepositoryMetrics{}, fmt.Errorf( "repository %q has no latest release for metric %q", repositoryRef.FullName(), @@ -211,7 +309,58 @@ func (fetcher *GitHubFetcher) fetchRepositoryMetrics( }, nil } -func buildGitHubSnapshotRecords(metrics githubRepositoryMetrics) []githubSnapshotRecord { +func (fetcher *GitHubFetcher) fetchOwnerMetrics( + ctx context.Context, + scope githubBindingScope, + capturedAt time.Time, +) (githubRepositoryMetrics, error) { + repositories, err := fetcher.client.ListOwnerRepositories(ctx, scope.Owner, scope.OwnerType) + if err != nil { + return githubRepositoryMetrics{}, err + } + + aggregated := githubRepositoryMetrics{ + ScopeKey: "owner:" + strings.TrimSpace(scope.Owner), + PublicRepoCount: len(repositories), + } + + for _, repositoryRef := range repositories { + metrics, err := fetcher.fetchRepositoryMetrics( + ctx, + repositoryRef, + capturedAt, + githubRepositoryFetchOptions{ + IncludeContributors: false, + RequireLatestRelease: false, + }, + ) + if err != nil { + return githubRepositoryMetrics{}, fmt.Errorf("repo %q: %w", repositoryRef.FullName(), err) + } + + aggregated.Stars += metrics.Stars + aggregated.Forks += metrics.Forks + aggregated.OpenIssues += metrics.OpenIssues + aggregated.Commits30d += metrics.Commits30d + aggregated.Commits90d += metrics.Commits90d + aggregated.LatestReleasePublishedAt = maxRFC3339Timestamp( + aggregated.LatestReleasePublishedAt, + metrics.LatestReleasePublishedAt, + ) + } + + if strings.TrimSpace(aggregated.LatestReleasePublishedAt) == "" { + return githubRepositoryMetrics{}, fmt.Errorf( + "owner %q has no latest release across public repositories for metric %q", + scope.Owner, + MetricKeyGitHubLatestReleasePublishedAt, + ) + } + + return aggregated, nil +} + +func buildGitHubRepositorySnapshotRecords(metrics githubRepositoryMetrics) []githubSnapshotRecord { return []githubSnapshotRecord{ newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubStars, metrics.Stars), newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubForks, metrics.Forks), @@ -223,6 +372,18 @@ func buildGitHubSnapshotRecords(metrics githubRepositoryMetrics) []githubSnapsho } } +func buildGitHubOwnerSnapshotRecords(metrics githubRepositoryMetrics) []githubSnapshotRecord { + return []githubSnapshotRecord{ + newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubPublicRepoCount, metrics.PublicRepoCount), + newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubStars, metrics.Stars), + newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubForks, metrics.Forks), + newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubOpenIssues, metrics.OpenIssues), + newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubCommits30d, metrics.Commits30d), + newNumericGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubCommits90d, metrics.Commits90d), + newTextGitHubSnapshot(metrics.ScopeKey, MetricKeyGitHubLatestReleasePublishedAt, metrics.LatestReleasePublishedAt), + } +} + func newNumericGitHubSnapshot(scopeKey, metricKey string, value int) githubSnapshotRecord { numericValue := float64(value) @@ -282,29 +443,80 @@ func persistGitHubMetricSnapshots( return nil } -func parseGitHubRepositoryRefs(binding config.Binding) ([]githubRepositoryRef, error) { +func parseGitHubBindingScope(binding config.Binding) (githubBindingScope, error) { + repositories, hasRepos, err := parseOptionalGitHubRepositoryRefs(binding) + if err != nil { + return githubBindingScope{}, err + } + + owner, hasOwner, err := parseOptionalGitHubOwner(binding) + if err != nil { + return githubBindingScope{}, err + } + + ownerType, hasOwnerType, err := parseOptionalGitHubOwnerType(binding) + if err != nil { + return githubBindingScope{}, err + } + + switch { + case hasRepos && (hasOwner || hasOwnerType): + return githubBindingScope{}, fmt.Errorf("binding %q must not combine scope.repos with scope.owner/owner_type", binding.ID) + case hasRepos: + return githubBindingScope{ + Kind: githubBindingScopeRepos, + Repositories: repositories, + }, nil + case hasOwner || hasOwnerType: + if !hasOwner { + return githubBindingScope{}, fmt.Errorf("binding %q scope.owner is required when scope.owner_type is set", binding.ID) + } + if !hasOwnerType { + return githubBindingScope{}, fmt.Errorf("binding %q scope.owner_type is required when scope.owner is set", binding.ID) + } + + return githubBindingScope{ + Kind: githubBindingScopeOwner, + Owner: owner, + OwnerType: ownerType, + }, nil + default: + return githubBindingScope{}, fmt.Errorf("binding %q must define either scope.repos or scope.owner/owner_type", binding.ID) + } +} + +func parseOptionalGitHubRepositoryRefs(binding config.Binding) ([]githubRepositoryRef, bool, error) { rawRepos, ok := binding.Scope["repos"] if !ok { - return nil, fmt.Errorf("binding %q scope.repos is required", binding.ID) + return nil, false, nil + } + + repositories, err := parseGitHubRepositoryRefsValue(binding.ID, rawRepos) + if err != nil { + return nil, false, err } + return repositories, true, nil +} + +func parseGitHubRepositoryRefsValue(bindingID string, rawRepos any) ([]githubRepositoryRef, error) { switch typed := rawRepos.(type) { case []string: - return parseGitHubRepositoryRefStrings(binding.ID, typed) + return parseGitHubRepositoryRefStrings(bindingID, typed) case []any: values := make([]string, 0, len(typed)) for index, rawValue := range typed { value, ok := rawValue.(string) if !ok { - return nil, fmt.Errorf("binding %q scope.repos[%d] must be a string", binding.ID, index) + return nil, fmt.Errorf("binding %q scope.repos[%d] must be a string", bindingID, index) } values = append(values, value) } - return parseGitHubRepositoryRefStrings(binding.ID, values) + return parseGitHubRepositoryRefStrings(bindingID, values) default: - return nil, fmt.Errorf("binding %q scope.repos must be an array of repository strings", binding.ID) + return nil, fmt.Errorf("binding %q scope.repos must be an array of repository strings", bindingID) } } @@ -326,6 +538,47 @@ func parseGitHubRepositoryRefStrings(bindingID string, values []string) ([]githu return repositories, nil } +func parseOptionalGitHubOwner(binding config.Binding) (string, bool, error) { + rawOwner, ok := binding.Scope["owner"] + if !ok { + return "", false, nil + } + + owner, ok := rawOwner.(string) + if !ok { + return "", false, fmt.Errorf("binding %q scope.owner must be a string", binding.ID) + } + + owner = strings.TrimSpace(owner) + if owner == "" { + return "", false, fmt.Errorf("binding %q scope.owner must not be empty", binding.ID) + } + + return owner, true, nil +} + +func parseOptionalGitHubOwnerType(binding config.Binding) (string, bool, error) { + rawOwnerType, ok := binding.Scope["owner_type"] + if !ok { + return "", false, nil + } + + ownerType, ok := rawOwnerType.(string) + if !ok { + return "", false, fmt.Errorf("binding %q scope.owner_type must be a string", binding.ID) + } + + ownerType = strings.TrimSpace(strings.ToLower(ownerType)) + switch ownerType { + case "organization", "user": + return ownerType, true, nil + case "": + return "", false, fmt.Errorf("binding %q scope.owner_type must not be empty", binding.ID) + default: + return "", false, fmt.Errorf("binding %q scope.owner_type %q is unsupported", binding.ID, ownerType) + } +} + func parseGitHubRepositoryRef(value string) (githubRepositoryRef, error) { trimmed := strings.TrimSpace(value) if trimmed == "" { @@ -354,16 +607,19 @@ type GitHubClientOptions struct { BaseURL string HTTPClient *http.Client UserAgent string + Token string } type GitHubClient struct { baseURL *url.URL httpClient *http.Client userAgent string + token string } func NewGitHubClient(options GitHubClientOptions) *GitHubClient { baseURL := normalizeGitHubBaseURL(options.BaseURL) + httpClient := options.HTTPClient if httpClient == nil { httpClient = http.DefaultClient @@ -374,11 +630,77 @@ func NewGitHubClient(options GitHubClientOptions) *GitHubClient { userAgent = defaultGitHubUserAgent } + token := strings.TrimSpace(options.Token) + return &GitHubClient{ baseURL: mustParseGitHubBaseURL(baseURL), httpClient: httpClient, userAgent: userAgent, + token: token, + } +} + +func (client *GitHubClient) ListOwnerRepositories(ctx context.Context, owner, ownerType string) ([]githubRepositoryRef, error) { + requestURL, err := client.ownerRepositoriesURL(owner, ownerType) + if err != nil { + return nil, err + } + + repositories := make([]githubRepositoryRef, 0) + seen := make(map[string]struct{}) + + for { + response, err := client.doRequestURL(ctx, http.MethodGet, requestURL) + if err != nil { + return nil, err + } + + if response.StatusCode != http.StatusOK { + responseErr := client.responseError("list owner repositories", response) + _ = response.Body.Close() + return nil, responseErr + } + + var page []githubRepository + if err := json.NewDecoder(response.Body).Decode(&page); err != nil { + _ = response.Body.Close() + return nil, fmt.Errorf("decode owner repositories response: %w", err) + } + + nextRequestURL, hasNext := parseLinkRelationURL(response.Header.Get("Link"), "next") + _ = response.Body.Close() + + for _, repository := range page { + if repository.Private { + continue + } + + repositoryRef, err := repository.RepositoryRef() + if err != nil { + return nil, fmt.Errorf("parse listed repository: %w", err) + } + + fullName := repositoryRef.FullName() + if _, exists := seen[fullName]; exists { + continue + } + + seen[fullName] = struct{}{} + repositories = append(repositories, repositoryRef) + } + + if !hasNext { + break + } + + requestURL = nextRequestURL } + + sort.Slice(repositories, func(i, j int) bool { + return repositories[i].FullName() < repositories[j].FullName() + }) + + return repositories, nil } func (client *GitHubClient) Repository(ctx context.Context, repositoryRef githubRepositoryRef) (githubRepository, error) { @@ -511,50 +833,57 @@ func countFromSingleItemPage(linkHeader string, itemsOnPage int) (int, error) { return itemsOnPage, nil } -func parseLastPage(linkHeader string) (int, bool, error) { +func parseLinkRelationURL(linkHeader, relation string) (string, bool) { linkHeader = strings.TrimSpace(linkHeader) if linkHeader == "" { - return 0, false, nil + return "", false } - parts := strings.Split(linkHeader, ",") - for _, part := range parts { + for _, part := range strings.Split(linkHeader, ",") { sections := strings.Split(strings.TrimSpace(part), ";") if len(sections) < 2 { continue } - rel := strings.TrimSpace(sections[1]) - if !strings.Contains(rel, `rel="last"`) { - continue - } - rawURL := strings.TrimSpace(sections[0]) rawURL = strings.TrimPrefix(rawURL, "<") rawURL = strings.TrimSuffix(rawURL, ">") - parsed, err := url.Parse(rawURL) - if err != nil { - return 0, false, fmt.Errorf("parse Link header URL %q: %w", rawURL, err) + for _, section := range sections[1:] { + if strings.Contains(strings.TrimSpace(section), fmt.Sprintf(`rel="%s"`, relation)) { + return rawURL, true + } } + } - pageValue := strings.TrimSpace(parsed.Query().Get("page")) - if pageValue == "" { - return 0, false, fmt.Errorf("Link header last page is missing page query parameter") - } + return "", false +} - page, err := strconv.Atoi(pageValue) - if err != nil { - return 0, false, fmt.Errorf("parse Link header page value %q: %w", pageValue, err) - } - if page < 1 { - return 0, false, fmt.Errorf("Link header last page must be greater than zero") - } +func parseLastPage(linkHeader string) (int, bool, error) { + rawURL, ok := parseLinkRelationURL(linkHeader, "last") + if !ok { + return 0, false, nil + } + + parsed, err := url.Parse(rawURL) + if err != nil { + return 0, false, fmt.Errorf("parse Link header URL %q: %w", rawURL, err) + } + + pageValue := strings.TrimSpace(parsed.Query().Get("page")) + if pageValue == "" { + return 0, false, fmt.Errorf("Link header last page is missing page query parameter") + } - return page, true, nil + page, err := strconv.Atoi(pageValue) + if err != nil { + return 0, false, fmt.Errorf("parse Link header page value %q: %w", pageValue, err) + } + if page < 1 { + return 0, false, fmt.Errorf("Link header last page must be greater than zero") } - return 0, false, nil + return page, true, nil } func (client *GitHubClient) doRequest( @@ -568,18 +897,30 @@ func (client *GitHubClient) doRequest( RawQuery: query.Encode(), }) - request, err := http.NewRequestWithContext(ctx, method, requestURL.String(), nil) + return client.doRequestURL(ctx, method, requestURL.String()) +} + +func (client *GitHubClient) doRequestURL(ctx context.Context, method, rawURL string) (*http.Response, error) { + requestURL, err := client.resolveURL(rawURL) if err != nil { - return nil, fmt.Errorf("build GitHub request %s %s: %w", method, requestURL.String(), err) + return nil, fmt.Errorf("resolve GitHub request URL %q: %w", rawURL, err) + } + + request, err := http.NewRequestWithContext(ctx, method, requestURL, nil) + if err != nil { + return nil, fmt.Errorf("build GitHub request %s %s: %w", method, requestURL, err) } request.Header.Set("Accept", "application/vnd.github+json") request.Header.Set("User-Agent", client.userAgent) request.Header.Set("X-GitHub-Api-Version", "2022-11-28") + if client.token != "" { + request.Header.Set("Authorization", "Bearer "+client.token) + } response, err := client.httpClient.Do(request) if err != nil { - return nil, fmt.Errorf("perform GitHub request %s %s: %w", method, requestURL.String(), err) + return nil, fmt.Errorf("perform GitHub request %s %s: %w", method, requestURL, err) } return response, nil @@ -604,15 +945,19 @@ func (client *GitHubClient) responseError(action string, response *http.Response message = "unknown GitHub API error" } + requestMethod := "" requestPath := "" - if response.Request != nil && response.Request.URL != nil { - requestPath = response.Request.URL.Path + if response.Request != nil { + requestMethod = response.Request.Method + if response.Request.URL != nil { + requestPath = response.Request.URL.Path + } } return fmt.Errorf( "%s: GitHub API %s %s returned %d: %s", action, - response.Request.Method, + requestMethod, requestPath, response.StatusCode, message, @@ -627,6 +972,53 @@ func repositoryPath(repositoryRef githubRepositoryRef) string { ) } +func (client *GitHubClient) ownerRepositoriesURL(owner, ownerType string) (string, error) { + path, query, err := ownerRepositoriesPath(owner, ownerType) + if err != nil { + return "", err + } + + query.Set("per_page", "100") + + return client.baseURL.ResolveReference(&url.URL{ + Path: strings.TrimPrefix(path, "/"), + RawQuery: query.Encode(), + }).String(), nil +} + +func ownerRepositoriesPath(owner, ownerType string) (string, url.Values, error) { + owner = strings.TrimSpace(owner) + if owner == "" { + return "", nil, fmt.Errorf("owner is required") + } + + ownerType = strings.TrimSpace(strings.ToLower(ownerType)) + query := url.Values{} + + switch ownerType { + case "organization": + query.Set("type", "public") + return fmt.Sprintf("orgs/%s/repos", url.PathEscape(owner)), query, nil + case "user": + query.Set("type", "owner") + return fmt.Sprintf("users/%s/repos", url.PathEscape(owner)), query, nil + default: + return "", nil, fmt.Errorf("unsupported owner_type %q", ownerType) + } +} + +func (client *GitHubClient) resolveURL(rawURL string) (string, error) { + parsed, err := url.Parse(strings.TrimSpace(rawURL)) + if err != nil { + return "", err + } + if parsed.IsAbs() { + return parsed.String(), nil + } + + return client.baseURL.ResolveReference(parsed).String(), nil +} + func normalizeGitHubBaseURL(value string) string { trimmed := strings.TrimSpace(value) if trimmed == "" { @@ -647,3 +1039,14 @@ func mustParseGitHubBaseURL(value string) *url.URL { return parsed } + +func maxRFC3339Timestamp(current, candidate string) string { + current = strings.TrimSpace(current) + candidate = strings.TrimSpace(candidate) + + if candidate > current { + return candidate + } + + return current +} diff --git a/internal/fetch/github_test.go b/internal/fetch/github_test.go index 898d4a4..65ef2eb 100644 --- a/internal/fetch/github_test.go +++ b/internal/fetch/github_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/CrisSTEM/signalscope/internal/config" "github.com/CrisSTEM/signalscope/internal/storage" ) @@ -22,8 +23,10 @@ type metricSnapshotRow struct { } type githubTestServerOptions struct { - Repo2Status int - Repo1ReleaseStatus int + Repo2Status int + Repo1ReleaseStatus int + Repo2ReleaseStatus int + RequireAuthorization string } func TestGitHubFetcherPersistsRepositorySnapshots(t *testing.T) { @@ -314,6 +317,202 @@ func TestGitHubFetcherFailsWhenRepositoryHasNoLatestRelease(t *testing.T) { } } +func TestGitHubFetcherPersistsOwnerScopedSnapshots(t *testing.T) { + t.Parallel() + + ctx := context.Background() + capturedAt := time.Date(2026, 4, 20, 15, 0, 0, 0, time.UTC) + + server := newGitHubTestServer(t, capturedAt, githubTestServerOptions{ + Repo2ReleaseStatus: http.StatusNotFound, + }) + defer server.Close() + + pack := ownerRuntimeFixturePack("github-owner-fetch-success") + db := openRuntimeTestDB(t, pack) + defer db.Close() + + registry := NewRegistry() + registry.MustRegister(SourceKindGitHub, newGitHubTestFetcher(server)) + + runtime := Runtime{ + DB: db, + Registry: registry, + Now: sequenceClock( + capturedAt, + capturedAt.Add(1*time.Second), + ), + } + + summary, err := runtime.Execute(ctx, pack, Options{ + SourceKind: SourceKindGitHub, + BindingID: "github-owner-binding", + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + + if summary.Attempted != 1 { + t.Fatalf("summary.Attempted = %d, want %d", summary.Attempted, 1) + } + if summary.Succeeded != 1 { + t.Fatalf("summary.Succeeded = %d, want %d", summary.Succeeded, 1) + } + if summary.Failed != 0 { + t.Fatalf("summary.Failed = %d, want %d", summary.Failed, 0) + } + if summary.RecordsWritten != 3 { + t.Fatalf("summary.RecordsWritten = %d, want %d", summary.RecordsWritten, 3) + } + if summary.MetricsWritten != 7 { + t.Fatalf("summary.MetricsWritten = %d, want %d", summary.MetricsWritten, 7) + } + + rows, err := readMetricSnapshotsForBinding(ctx, db, "github-owner-binding") + if err != nil { + t.Fatalf("readMetricSnapshotsForBinding() error = %v", err) + } + if len(rows) != 7 { + t.Fatalf("len(rows) = %d, want %d", len(rows), 7) + } + + expectedNumeric := map[string]int{ + MetricKeyGitHubPublicRepoCount: 3, + MetricKeyGitHubStars: 18, + MetricKeyGitHubForks: 7, + MetricKeyGitHubOpenIssues: 10, + MetricKeyGitHubCommits30d: 10, + MetricKeyGitHubCommits90d: 23, + } + expectedText := map[string]string{ + MetricKeyGitHubLatestReleasePublishedAt: "2026-04-10T00:00:00Z", + } + + for _, row := range rows { + if row.ScopeKey != "owner:example" { + t.Fatalf("row.ScopeKey = %q, want %q", row.ScopeKey, "owner:example") + } + if row.MetricKey == MetricKeyGitHubContributors { + t.Fatalf("unexpected owner-scoped metric key %q", row.MetricKey) + } + + if expectedValue, ok := expectedNumeric[row.MetricKey]; ok { + if !row.MetricValueNum.Valid { + t.Fatalf("row %q numeric value is invalid, want %d", row.MetricKey, expectedValue) + } + if int(row.MetricValueNum.Float64) != expectedValue { + t.Fatalf("row %q numeric value = %v, want %d", row.MetricKey, row.MetricValueNum.Float64, expectedValue) + } + continue + } + + if expectedValue, ok := expectedText[row.MetricKey]; ok { + if !row.MetricValueText.Valid { + t.Fatalf("row %q text value is invalid, want %q", row.MetricKey, expectedValue) + } + if row.MetricValueText.String != expectedValue { + t.Fatalf("row %q text value = %q, want %q", row.MetricKey, row.MetricValueText.String, expectedValue) + } + continue + } + + t.Fatalf("unexpected metric key %q", row.MetricKey) + } +} + +func TestGitHubFetcherUsesOptionalTokenAuthWhenConfigured(t *testing.T) { + t.Parallel() + + ctx := context.Background() + capturedAt := time.Date(2026, 4, 20, 16, 0, 0, 0, time.UTC) + + server := newGitHubTestServer(t, capturedAt, githubTestServerOptions{ + RequireAuthorization: "Bearer owner-token", + }) + defer server.Close() + + pack := ownerRuntimeFixturePack("github-owner-fetch-token") + db := openRuntimeTestDB(t, pack) + defer db.Close() + + registry := NewRegistry() + registry.MustRegister(SourceKindGitHub, NewGitHubFetcher(GitHubFetcherOptions{ + ClientOptions: GitHubClientOptions{ + BaseURL: server.URL + "/", + HTTPClient: server.Client(), + UserAgent: "signalscope-test", + }, + TokenLookup: func() string { + // This simulates the default GITHUB_TOKEN lookup path deterministically. + return "owner-token" + }, + })) + + runtime := Runtime{ + DB: db, + Registry: registry, + Now: sequenceClock( + capturedAt, + capturedAt.Add(1*time.Second), + ), + } + + summary, err := runtime.Execute(ctx, pack, Options{ + SourceKind: SourceKindGitHub, + BindingID: "github-owner-binding", + }) + if err != nil { + t.Fatalf("Execute() error = %v", err) + } + if summary.Failed != 0 { + t.Fatalf("summary.Failed = %d, want %d", summary.Failed, 0) + } + if summary.Succeeded != 1 { + t.Fatalf("summary.Succeeded = %d, want %d", summary.Succeeded, 1) + } +} + +func TestGitHubClientListOwnerRepositoriesSupportsUserOwners(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + w.Header().Set("Content-Type", "application/json") + + if request.URL.Path != "/users/example-user/repos" { + http.Error(w, `{"message":"unexpected path"}`, http.StatusNotFound) + return + } + if request.URL.Query().Get("type") != "owner" { + http.Error(w, `{"message":"expected type=owner"}`, http.StatusBadRequest) + return + } + if request.URL.Query().Get("per_page") != "100" { + http.Error(w, `{"message":"expected per_page=100"}`, http.StatusBadRequest) + return + } + + fmt.Fprint(w, `[{"name":"repo-1","full_name":"example-user/repo-1","owner":{"login":"example-user"},"private":false}]`) + })) + defer server.Close() + + client := NewGitHubClient(GitHubClientOptions{ + BaseURL: server.URL + "/", + HTTPClient: server.Client(), + UserAgent: "signalscope-test", + }) + + repositories, err := client.ListOwnerRepositories(context.Background(), "example-user", "user") + if err != nil { + t.Fatalf("ListOwnerRepositories() error = %v", err) + } + if len(repositories) != 1 { + t.Fatalf("len(repositories) = %d, want %d", len(repositories), 1) + } + if repositories[0].FullName() != "example-user/repo-1" { + t.Fatalf("repositories[0].FullName() = %q, want %q", repositories[0].FullName(), "example-user/repo-1") + } +} + func newGitHubTestFetcher(server *httptest.Server) *GitHubFetcher { return NewGitHubFetcher(GitHubFetcherOptions{ Client: NewGitHubClient(GitHubClientOptions{ @@ -340,10 +539,42 @@ func newGitHubTestServer(t *testing.T, capturedAt time.Time, options githubTestS repo1ReleaseStatus = http.StatusOK } + repo2ReleaseStatus := options.Repo2ReleaseStatus + if repo2ReleaseStatus == 0 { + repo2ReleaseStatus = http.StatusOK + } + + requiredAuthorization := strings.TrimSpace(options.RequireAuthorization) + handler := http.HandlerFunc(func(w http.ResponseWriter, request *http.Request) { + if requiredAuthorization != "" && request.Header.Get("Authorization") != requiredAuthorization { + http.Error(w, `{"message":"missing or invalid authorization"}`, http.StatusUnauthorized) + return + } + w.Header().Set("Content-Type", "application/json") switch request.URL.Path { + case "/orgs/example/repos": + if request.URL.Query().Get("per_page") != "100" { + http.Error(w, `{"message":"expected per_page=100"}`, http.StatusBadRequest) + return + } + if request.URL.Query().Get("type") != "public" { + http.Error(w, `{"message":"expected type=public"}`, http.StatusBadRequest) + return + } + + switch request.URL.Query().Get("page") { + case "", "1": + w.Header().Set("Link", `; rel="next", ; rel="last"`) + fmt.Fprint(w, `[{"name":"repo-1","full_name":"example/repo-1","owner":{"login":"example"},"archived":false,"fork":false,"private":false},{"name":"repo-2","full_name":"example/repo-2","owner":{"login":"example"},"archived":true,"fork":false,"private":false}]`) + case "2": + fmt.Fprint(w, `[{"name":"repo-3","full_name":"example/repo-3","owner":{"login":"example"},"archived":false,"fork":true,"private":false}]`) + default: + http.Error(w, `{"message":"unexpected page"}`, http.StatusBadRequest) + } + case "/repos/example/repo-1": fmt.Fprint(w, `{"full_name":"example/repo-1","stargazers_count":10,"forks_count":4,"open_issues_count":7}`) @@ -412,8 +643,36 @@ func newGitHubTestServer(t *testing.T, capturedAt time.Time, options githubTestS } case "/repos/example/repo-2/releases/latest": + if repo2ReleaseStatus == http.StatusNotFound { + w.WriteHeader(http.StatusNotFound) + fmt.Fprint(w, `{"message":"Not Found"}`) + return + } + fmt.Fprint(w, `{"published_at":"2026-03-15T00:00:00Z"}`) + case "/repos/example/repo-3": + fmt.Fprint(w, `{"full_name":"example/repo-3","stargazers_count":2,"forks_count":1,"open_issues_count":0}`) + + case "/repos/example/repo-3/commits": + if request.URL.Query().Get("per_page") != "1" { + http.Error(w, `{"message":"expected per_page=1"}`, http.StatusBadRequest) + return + } + + switch request.URL.Query().Get("since") { + case capturedAt.Add(-30 * 24 * time.Hour).Format(time.RFC3339): + fmt.Fprint(w, `[{"sha":"repo-3-30d"}]`) + case capturedAt.Add(-90 * 24 * time.Hour).Format(time.RFC3339): + w.Header().Set("Link", `; rel="last"`) + fmt.Fprint(w, `[{"sha":"repo-3-90d"}]`) + default: + http.Error(w, `{"message":"unexpected since query"}`, http.StatusBadRequest) + } + + case "/repos/example/repo-3/releases/latest": + fmt.Fprint(w, `{"published_at":"2026-04-10T00:00:00Z"}`) + default: http.Error(w, `{"message":"unexpected path"}`, http.StatusNotFound) } @@ -422,6 +681,60 @@ func newGitHubTestServer(t *testing.T, capturedAt time.Time, options githubTestS return httptest.NewServer(handler) } +func ownerRuntimeFixturePack(datasetID string) config.Pack { + return config.Pack{ + Entities: config.EntitiesFile{ + Version: 1, + DatasetID: datasetID, + DatasetName: "GitHub Owner Fixture Dataset", + Entities: []config.Entity{ + { + ID: "org-1", + Slug: "org-1", + Kind: "organization", + Name: "Org 1", + }, + }, + Relationships: []config.Relationship{}, + }, + Sources: config.SourcesFile{ + Version: 1, + DatasetID: datasetID, + Sources: []config.Source{ + { + ID: "github", + Kind: "github", + Enabled: true, + Defaults: config.JSONMap{}, + }, + }, + Bindings: []config.Binding{ + { + ID: "github-owner-binding", + EntityID: "org-1", + SourceID: "github", + Enabled: true, + Scope: config.JSONMap{ + "owner": "example", + "owner_type": "organization", + }, + Notes: "owner-scoped github binding", + }, + }, + }, + Alerts: config.AlertsFile{ + Version: 1, + DatasetID: datasetID, + Rules: []config.AlertRule{}, + }, + Schedules: config.SchedulesFile{ + Version: 1, + DatasetID: datasetID, + Jobs: []config.ScheduleJob{}, + }, + } +} + func readMetricSnapshotsForBinding(ctx context.Context, db *sql.DB, bindingID string) ([]metricSnapshotRow, error) { rows, err := db.QueryContext( ctx, @@ -437,7 +750,6 @@ func readMetricSnapshotsForBinding(ctx context.Context, db *sql.DB, bindingID st defer rows.Close() var result []metricSnapshotRow - for rows.Next() { var row metricSnapshotRow if err := rows.Scan( From 0e7af1f9f1f03eaa2ed2272092b66818a83cecc6 Mon Sep 17 00:00:00 2001 From: CriSTEM Date: Wed, 15 Apr 2026 00:18:17 -0600 Subject: [PATCH 2/2] docs(github): clarify owner-scope archive and fork behavior --- docs/configuration.md | 11 +++++++---- docs/reference-dataset.md | 6 ++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index f85933e..de839da 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -264,16 +264,19 @@ Owner-scoped GitHub ingestion may emit: - `github.public_repo_count` Notes: -- owner-scoped rows should use `scope_key = owner:` -- aggregate metrics are computed across the repositories included by that owner-scoped binding +- owner-scoped rows should use `scope_key = "owner:"` +- aggregate metrics are computed across the public repositories included by that owner-scoped binding +- the runtime resolves owner scope through the GitHub owner listing endpoint selected by `owner_type` +- when `GITHUB_TOKEN` is present in the runtime environment, GitHub requests use it to improve rate-limit headroom; unauthenticated mode remains supported but is more exposed to lower anonymous rate limits +- private repositories remain out of scope for owner aggregation even when authenticated +- public archived repositories and public forks are currently included in the owner aggregate because the binding contract does not yet define archive/fork filters - owner-scoped `github.contributors` is intentionally deferred for `v0.3.0` and must not be implied by the presence of `scope.owner` Examples: ```json { "owner": "Blockstream", - "owner_type": "organization", - "include_owner_repos": true + "owner_type": "organization" } ``` diff --git a/docs/reference-dataset.md b/docs/reference-dataset.md index 776c853..9f0a057 100644 --- a/docs/reference-dataset.md +++ b/docs/reference-dataset.md @@ -92,6 +92,12 @@ Important `v0.3.0` limitation: - owner-scoped `github.contributors` is intentionally deferred - contributor counts are only expected from explicit repo-scoped GitHub bindings +Operational note for owner-scoped bindings in `v0.3.0`: +- the runtime aggregates across the public repositories returned by the configured owner endpoint +- `GITHUB_TOKEN` is optional and only improves rate-limit headroom; unauthenticated mode remains supported +- public archived repositories and public forks are currently included in that aggregate +- private repositories are excluded from the owner aggregate + ### 2. News RSS Every tracked entity in the demo pack is assumed to have a keyword-based news binding.