Skip to content

Commit d4fece0

Browse files
musch711Isaac
musch711
and
Isaac
committed
Migrate repository traffic tools to typed registration
Register get_traffic_views, get_traffic_clones, get_traffic_referrers and get_traffic_paths with concrete input/output types (NewTool[In, Out]) to match the typed tool registration introduced in #3371/#3377, so they pass TestToolDefinitionsUseConcreteOutputTypes. The text result shape is unchanged. Also switch test fixtures from github.Ptr to new(...) per lint. Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 551345a commit d4fece0

3 files changed

Lines changed: 132 additions & 70 deletions

File tree

‎pkg/github/repositories.go‎

Lines changed: 65 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2372,7 +2372,7 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool
23722372
// GetTrafficViews creates a tool to get the page views and unique visitors for
23732373
// a repository over the last 14 days.
23742374
func GetTrafficViews(t translations.TranslationHelperFunc) inventory.ServerTool {
2375-
return NewTool(
2375+
return NewTool[RepositoryTrafficBreakdownInput, *TrafficViewsOutput](
23762376
ToolsetMetadataRepos,
23772377
mcp.Tool{
23782378
Name: "get_traffic_views",
@@ -2402,19 +2402,16 @@ func GetTrafficViews(t translations.TranslationHelperFunc) inventory.ServerTool
24022402
},
24032403
},
24042404
scopes.RequireAll(scopes.Repo),
2405-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
2406-
owner, err := RequiredParam[string](args, "owner")
2407-
if err != nil {
2408-
return utils.NewToolResultError(err.Error()), nil, nil
2409-
}
2410-
repo, err := RequiredParam[string](args, "repo")
2411-
if err != nil {
2412-
return utils.NewToolResultError(err.Error()), nil, nil
2405+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input RepositoryTrafficBreakdownInput) (*mcp.CallToolResult, *TrafficViewsOutput, error) {
2406+
owner := input.Owner
2407+
if owner == "" {
2408+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
24132409
}
2414-
per, err := OptionalParam[string](args, "per")
2415-
if err != nil {
2416-
return utils.NewToolResultError(err.Error()), nil, nil
2410+
repo := input.Repo
2411+
if repo == "" {
2412+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
24172413
}
2414+
per := input.Per
24182415

24192416
var opts *github.TrafficBreakdownOptions
24202417
if per != "" {
@@ -2440,25 +2437,26 @@ func GetTrafficViews(t translations.TranslationHelperFunc) inventory.ServerTool
24402437
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get traffic views", resp, body), nil, nil
24412438
}
24422439

2443-
r, err := json.Marshal(views)
2444-
if err != nil {
2445-
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
2440+
output := &TrafficViewsOutput{
2441+
Count: views.GetCount(),
2442+
Uniques: views.GetUniques(),
2443+
Views: convertToTrafficDataOutput(views.Views),
24462444
}
24472445

2448-
result := utils.NewToolResultText(string(r))
2446+
result := MarshalledTextResult(output)
24492447
// Traffic analytics require push access to read (GitHub gates the
24502448
// endpoint behind Administration: Read), so the data is never
24512449
// world-readable, even on a public repository. Always private.
24522450
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoTraffic())
2453-
return result, nil, nil
2451+
return result, output, nil
24542452
},
24552453
)
24562454
}
24572455

24582456
// GetTrafficClones creates a tool to get the git clones and unique cloners for
24592457
// a repository over the last 14 days.
24602458
func GetTrafficClones(t translations.TranslationHelperFunc) inventory.ServerTool {
2461-
return NewTool(
2459+
return NewTool[RepositoryTrafficBreakdownInput, *TrafficClonesOutput](
24622460
ToolsetMetadataRepos,
24632461
mcp.Tool{
24642462
Name: "get_traffic_clones",
@@ -2488,19 +2486,16 @@ func GetTrafficClones(t translations.TranslationHelperFunc) inventory.ServerTool
24882486
},
24892487
},
24902488
scopes.RequireAll(scopes.Repo),
2491-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
2492-
owner, err := RequiredParam[string](args, "owner")
2493-
if err != nil {
2494-
return utils.NewToolResultError(err.Error()), nil, nil
2495-
}
2496-
repo, err := RequiredParam[string](args, "repo")
2497-
if err != nil {
2498-
return utils.NewToolResultError(err.Error()), nil, nil
2489+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input RepositoryTrafficBreakdownInput) (*mcp.CallToolResult, *TrafficClonesOutput, error) {
2490+
owner := input.Owner
2491+
if owner == "" {
2492+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
24992493
}
2500-
per, err := OptionalParam[string](args, "per")
2501-
if err != nil {
2502-
return utils.NewToolResultError(err.Error()), nil, nil
2494+
repo := input.Repo
2495+
if repo == "" {
2496+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
25032497
}
2498+
per := input.Per
25042499

25052500
var opts *github.TrafficBreakdownOptions
25062501
if per != "" {
@@ -2526,23 +2521,24 @@ func GetTrafficClones(t translations.TranslationHelperFunc) inventory.ServerTool
25262521
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get traffic clones", resp, body), nil, nil
25272522
}
25282523

2529-
r, err := json.Marshal(clones)
2530-
if err != nil {
2531-
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
2524+
output := &TrafficClonesOutput{
2525+
Count: clones.GetCount(),
2526+
Uniques: clones.GetUniques(),
2527+
Clones: convertToTrafficDataOutput(clones.Clones),
25322528
}
25332529

2534-
result := utils.NewToolResultText(string(r))
2530+
result := MarshalledTextResult(output)
25352531
// Traffic analytics require push access to read; never world-readable.
25362532
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoTraffic())
2537-
return result, nil, nil
2533+
return result, output, nil
25382534
},
25392535
)
25402536
}
25412537

25422538
// GetTrafficReferrers creates a tool to get the top 10 referring sites for a
25432539
// repository over the last 14 days.
25442540
func GetTrafficReferrers(t translations.TranslationHelperFunc) inventory.ServerTool {
2545-
return NewTool(
2541+
return NewTool[RepositoryInput, []TrafficReferrerOutput](
25462542
ToolsetMetadataRepos,
25472543
mcp.Tool{
25482544
Name: "get_traffic_referrers",
@@ -2567,14 +2563,14 @@ func GetTrafficReferrers(t translations.TranslationHelperFunc) inventory.ServerT
25672563
},
25682564
},
25692565
scopes.RequireAll(scopes.Repo),
2570-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
2571-
owner, err := RequiredParam[string](args, "owner")
2572-
if err != nil {
2573-
return utils.NewToolResultError(err.Error()), nil, nil
2566+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input RepositoryInput) (*mcp.CallToolResult, []TrafficReferrerOutput, error) {
2567+
owner := input.Owner
2568+
if owner == "" {
2569+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
25742570
}
2575-
repo, err := RequiredParam[string](args, "repo")
2576-
if err != nil {
2577-
return utils.NewToolResultError(err.Error()), nil, nil
2571+
repo := input.Repo
2572+
if repo == "" {
2573+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
25782574
}
25792575

25802576
client, err := deps.GetClient(ctx)
@@ -2596,23 +2592,27 @@ func GetTrafficReferrers(t translations.TranslationHelperFunc) inventory.ServerT
25962592
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get traffic referrers", resp, body), nil, nil
25972593
}
25982594

2599-
r, err := json.Marshal(referrers)
2600-
if err != nil {
2601-
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
2595+
output := make([]TrafficReferrerOutput, 0, len(referrers))
2596+
for _, r := range referrers {
2597+
output = append(output, TrafficReferrerOutput{
2598+
Referrer: r.GetReferrer(),
2599+
Count: r.GetCount(),
2600+
Uniques: r.GetUniques(),
2601+
})
26022602
}
26032603

2604-
result := utils.NewToolResultText(string(r))
2604+
result := MarshalledTextResult(output)
26052605
// Traffic analytics require push access to read; never world-readable.
26062606
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoTraffic())
2607-
return result, nil, nil
2607+
return result, output, nil
26082608
},
26092609
)
26102610
}
26112611

26122612
// GetTrafficPaths creates a tool to get the top 10 most-visited paths for a
26132613
// repository over the last 14 days.
26142614
func GetTrafficPaths(t translations.TranslationHelperFunc) inventory.ServerTool {
2615-
return NewTool(
2615+
return NewTool[RepositoryInput, []TrafficPathOutput](
26162616
ToolsetMetadataRepos,
26172617
mcp.Tool{
26182618
Name: "get_traffic_paths",
@@ -2637,14 +2637,14 @@ func GetTrafficPaths(t translations.TranslationHelperFunc) inventory.ServerTool
26372637
},
26382638
},
26392639
scopes.RequireAll(scopes.Repo),
2640-
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) {
2641-
owner, err := RequiredParam[string](args, "owner")
2642-
if err != nil {
2643-
return utils.NewToolResultError(err.Error()), nil, nil
2640+
func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input RepositoryInput) (*mcp.CallToolResult, []TrafficPathOutput, error) {
2641+
owner := input.Owner
2642+
if owner == "" {
2643+
return utils.NewToolResultError("missing required parameter: owner"), nil, nil
26442644
}
2645-
repo, err := RequiredParam[string](args, "repo")
2646-
if err != nil {
2647-
return utils.NewToolResultError(err.Error()), nil, nil
2645+
repo := input.Repo
2646+
if repo == "" {
2647+
return utils.NewToolResultError("missing required parameter: repo"), nil, nil
26482648
}
26492649

26502650
client, err := deps.GetClient(ctx)
@@ -2666,15 +2666,20 @@ func GetTrafficPaths(t translations.TranslationHelperFunc) inventory.ServerTool
26662666
return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get traffic paths", resp, body), nil, nil
26672667
}
26682668

2669-
r, err := json.Marshal(paths)
2670-
if err != nil {
2671-
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
2669+
output := make([]TrafficPathOutput, 0, len(paths))
2670+
for _, p := range paths {
2671+
output = append(output, TrafficPathOutput{
2672+
Path: p.GetPath(),
2673+
Title: p.GetTitle(),
2674+
Count: p.GetCount(),
2675+
Uniques: p.GetUniques(),
2676+
})
26722677
}
26732678

2674-
result := utils.NewToolResultText(string(r))
2679+
result := MarshalledTextResult(output)
26752680
// Traffic analytics require push access to read; never world-readable.
26762681
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoTraffic())
2677-
return result, nil, nil
2682+
return result, output, nil
26782683
},
26792684
)
26802685
}

‎pkg/github/repositories_test.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5280,10 +5280,14 @@ func Test_GetTrafficViews(t *testing.T) {
52805280
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"})
52815281

52825282
mockViews := &github.TrafficViews{
5283-
Count: github.Ptr(1234),
5284-
Uniques: github.Ptr(567),
5283+
Count: new(1234),
5284+
Uniques: new(567),
52855285
Views: []*github.TrafficData{
5286-
{Count: github.Ptr(100), Uniques: github.Ptr(40)},
5286+
{
5287+
Timestamp: &github.Timestamp{Time: time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC)},
5288+
Count: new(100),
5289+
Uniques: new(40),
5290+
},
52875291
},
52885292
}
52895293

@@ -5355,6 +5359,9 @@ func Test_GetTrafficViews(t *testing.T) {
53555359
err = json.Unmarshal([]byte(textContent.Text), &returned)
53565360
require.NoError(t, err)
53575361
assert.Equal(t, tc.expectedCount, returned.GetCount())
5362+
require.Len(t, returned.Views, 1)
5363+
assert.Equal(t, mockViews.Views[0].GetTimestamp().Time, returned.Views[0].GetTimestamp().Time)
5364+
assert.Equal(t, 100, returned.Views[0].GetCount())
53585365
})
53595366
}
53605367
}
@@ -5375,10 +5382,10 @@ func Test_GetTrafficClones(t *testing.T) {
53755382
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"})
53765383

53775384
mockClones := &github.TrafficClones{
5378-
Count: github.Ptr(88),
5379-
Uniques: github.Ptr(42),
5385+
Count: new(88),
5386+
Uniques: new(42),
53805387
Clones: []*github.TrafficData{
5381-
{Count: github.Ptr(10), Uniques: github.Ptr(6)},
5388+
{Count: new(10), Uniques: new(6)},
53825389
},
53835390
}
53845391

@@ -5455,8 +5462,8 @@ func Test_GetTrafficReferrers(t *testing.T) {
54555462
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"})
54565463

54575464
mockReferrers := []*github.TrafficReferrer{
5458-
{Referrer: github.Ptr("google.com"), Count: github.Ptr(120), Uniques: github.Ptr(50)},
5459-
{Referrer: github.Ptr("github.com"), Count: github.Ptr(80), Uniques: github.Ptr(30)},
5465+
{Referrer: new("google.com"), Count: new(120), Uniques: new(50)},
5466+
{Referrer: new("github.com"), Count: new(80), Uniques: new(30)},
54605467
}
54615468

54625469
tests := []struct {
@@ -5533,8 +5540,8 @@ func Test_GetTrafficPaths(t *testing.T) {
55335540
assert.ElementsMatch(t, schema.Required, []string{"owner", "repo"})
55345541

55355542
mockPaths := []*github.TrafficPath{
5536-
{Path: github.Ptr("/owner/repo"), Title: github.Ptr("owner/repo"), Count: github.Ptr(200), Uniques: github.Ptr(90)},
5537-
{Path: github.Ptr("/owner/repo/issues"), Title: github.Ptr("Issues"), Count: github.Ptr(50), Uniques: github.Ptr(20)},
5543+
{Path: new("/owner/repo"), Title: new("owner/repo"), Count: new(200), Uniques: new(90)},
5544+
{Path: new("/owner/repo/issues"), Title: new("Issues"), Count: new(50), Uniques: new(20)},
55385545
}
55395546

55405547
tests := []struct {

‎pkg/github/repository_tool_types.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,3 +540,53 @@ func repositoryContentsOutputSchema() *jsonschema.Schema {
540540
directory.Type, directory.Types = "array", nil
541541
return repositoryUnionSchema(&jsonschema.Schema{Type: "null"}, directory, content)
542542
}
543+
544+
type RepositoryTrafficBreakdownInput struct {
545+
Owner string `json:"owner"`
546+
Repo string `json:"repo"`
547+
Per string `json:"per,omitempty"`
548+
}
549+
550+
// TrafficDataOutput is a single day or week bucket of repository traffic.
551+
type TrafficDataOutput struct {
552+
Timestamp string `json:"timestamp,omitempty"`
553+
Count int `json:"count"`
554+
Uniques int `json:"uniques"`
555+
}
556+
557+
type TrafficViewsOutput struct {
558+
Count int `json:"count"`
559+
Uniques int `json:"uniques"`
560+
Views []TrafficDataOutput `json:"views"`
561+
}
562+
563+
type TrafficClonesOutput struct {
564+
Count int `json:"count"`
565+
Uniques int `json:"uniques"`
566+
Clones []TrafficDataOutput `json:"clones"`
567+
}
568+
569+
type TrafficReferrerOutput struct {
570+
Referrer string `json:"referrer"`
571+
Count int `json:"count"`
572+
Uniques int `json:"uniques"`
573+
}
574+
575+
type TrafficPathOutput struct {
576+
Path string `json:"path"`
577+
Title string `json:"title"`
578+
Count int `json:"count"`
579+
Uniques int `json:"uniques"`
580+
}
581+
582+
func convertToTrafficDataOutput(data []*github.TrafficData) []TrafficDataOutput {
583+
result := make([]TrafficDataOutput, 0, len(data))
584+
for _, d := range data {
585+
result = append(result, TrafficDataOutput{
586+
Timestamp: formatMinimalTimestamp(d.Timestamp),
587+
Count: d.GetCount(),
588+
Uniques: d.GetUniques(),
589+
})
590+
}
591+
return result
592+
}

0 commit comments

Comments
 (0)