diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 7fc62b1ae..89cc37c22 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -64,7 +64,7 @@ func generateReadmeDocs(readmePath string) error { t, _ := translations.TranslationHelper() // Create toolset group with mock clients - tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t) + tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000) // Generate toolsets documentation toolsetsDoc := generateToolsetsDoc(tsg) @@ -302,7 +302,7 @@ func generateRemoteToolsetsDoc() string { t, _ := translations.TranslationHelper() // Create toolset group with mock clients - tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t) + tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000) // Generate table header buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n") diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index cad002666..0a4545835 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -55,6 +55,7 @@ var ( ExportTranslations: viper.GetBool("export-translations"), EnableCommandLogging: viper.GetBool("enable-command-logging"), LogFilePath: viper.GetString("log-file"), + ContentWindowSize: viper.GetInt("content-window-size"), } return ghmcp.RunStdioServer(stdioServerConfig) }, @@ -75,6 +76,7 @@ func init() { rootCmd.PersistentFlags().Bool("enable-command-logging", false, "When enabled, the server will log all command requests and responses to the log file") rootCmd.PersistentFlags().Bool("export-translations", false, "Save translations to a JSON file") rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") + rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size") // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) @@ -84,6 +86,7 @@ func init() { _ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging")) _ = viper.BindPFlag("export-translations", rootCmd.PersistentFlags().Lookup("export-translations")) _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) + _ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size")) // Add subcommands rootCmd.AddCommand(stdioCmd) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 5fb9582b9..7ad71532f 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -47,6 +47,9 @@ type MCPServerConfig struct { // Translator provides translated text for the server tooling Translator translations.TranslationHelperFunc + + // Content window size + ContentWindowSize int } const stdioServerLogPrefix = "stdioserver" @@ -132,7 +135,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { } // Create default toolsets - tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, getRawClient, cfg.Translator) + tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, getRawClient, cfg.Translator, cfg.ContentWindowSize) err = tsg.EnableToolsets(enabledToolsets) if err != nil { @@ -180,6 +183,9 @@ type StdioServerConfig struct { // Path to the log file if not stderr LogFilePath string + + // Content window size + ContentWindowSize int } // RunStdioServer is not concurrent safe. @@ -191,13 +197,14 @@ func RunStdioServer(cfg StdioServerConfig) error { t, dumpTranslations := translations.TranslationHelper() ghServer, err := NewMCPServer(MCPServerConfig{ - Version: cfg.Version, - Host: cfg.Host, - Token: cfg.Token, - EnabledToolsets: cfg.EnabledToolsets, - DynamicToolsets: cfg.DynamicToolsets, - ReadOnly: cfg.ReadOnly, - Translator: t, + Version: cfg.Version, + Host: cfg.Host, + Token: cfg.Token, + EnabledToolsets: cfg.EnabledToolsets, + DynamicToolsets: cfg.DynamicToolsets, + ReadOnly: cfg.ReadOnly, + Translator: t, + ContentWindowSize: cfg.ContentWindowSize, }) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) diff --git a/internal/profiler/profiler.go b/internal/profiler/profiler.go new file mode 100644 index 000000000..1cfb7ffae --- /dev/null +++ b/internal/profiler/profiler.go @@ -0,0 +1,215 @@ +package profiler + +import ( + "context" + "fmt" + "os" + "runtime" + "strconv" + "time" + + "log/slog" + "math" +) + +// Profile represents performance metrics for an operation +type Profile struct { + Operation string `json:"operation"` + Duration time.Duration `json:"duration_ns"` + MemoryBefore uint64 `json:"memory_before_bytes"` + MemoryAfter uint64 `json:"memory_after_bytes"` + MemoryDelta int64 `json:"memory_delta_bytes"` + LinesCount int `json:"lines_count,omitempty"` + BytesCount int64 `json:"bytes_count,omitempty"` + Timestamp time.Time `json:"timestamp"` +} + +// String returns a human-readable representation of the profile +func (p *Profile) String() string { + return fmt.Sprintf("[%s] %s: duration=%v, memory_delta=%+dB, lines=%d, bytes=%d", + p.Timestamp.Format("15:04:05.000"), + p.Operation, + p.Duration, + p.MemoryDelta, + p.LinesCount, + p.BytesCount, + ) +} + +func safeMemoryDelta(after, before uint64) int64 { + if after > math.MaxInt64 || before > math.MaxInt64 { + if after >= before { + diff := after - before + if diff > math.MaxInt64 { + return math.MaxInt64 + } + return int64(diff) + } + diff := before - after + if diff > math.MaxInt64 { + return -math.MaxInt64 + } + return -int64(diff) + } + + return int64(after) - int64(before) +} + +// Profiler provides minimal performance profiling capabilities +type Profiler struct { + logger *slog.Logger + enabled bool +} + +// New creates a new Profiler instance +func New(logger *slog.Logger, enabled bool) *Profiler { + return &Profiler{ + logger: logger, + enabled: enabled, + } +} + +// ProfileFunc profiles a function execution +func (p *Profiler) ProfileFunc(ctx context.Context, operation string, fn func() error) (*Profile, error) { + if !p.enabled { + return nil, fn() + } + + profile := &Profile{ + Operation: operation, + Timestamp: time.Now(), + } + + var memBefore runtime.MemStats + runtime.ReadMemStats(&memBefore) + profile.MemoryBefore = memBefore.Alloc + + start := time.Now() + err := fn() + profile.Duration = time.Since(start) + + var memAfter runtime.MemStats + runtime.ReadMemStats(&memAfter) + profile.MemoryAfter = memAfter.Alloc + profile.MemoryDelta = safeMemoryDelta(memAfter.Alloc, memBefore.Alloc) + + if p.logger != nil { + p.logger.InfoContext(ctx, "Performance profile", "profile", profile.String()) + } + + return profile, err +} + +// ProfileFuncWithMetrics profiles a function execution and captures additional metrics +func (p *Profiler) ProfileFuncWithMetrics(ctx context.Context, operation string, fn func() (int, int64, error)) (*Profile, error) { + if !p.enabled { + _, _, err := fn() + return nil, err + } + + profile := &Profile{ + Operation: operation, + Timestamp: time.Now(), + } + + var memBefore runtime.MemStats + runtime.ReadMemStats(&memBefore) + profile.MemoryBefore = memBefore.Alloc + + start := time.Now() + lines, bytes, err := fn() + profile.Duration = time.Since(start) + profile.LinesCount = lines + profile.BytesCount = bytes + + var memAfter runtime.MemStats + runtime.ReadMemStats(&memAfter) + profile.MemoryAfter = memAfter.Alloc + profile.MemoryDelta = safeMemoryDelta(memAfter.Alloc, memBefore.Alloc) + + if p.logger != nil { + p.logger.InfoContext(ctx, "Performance profile", "profile", profile.String()) + } + + return profile, err +} + +// Start begins timing an operation and returns a function to complete the profiling +func (p *Profiler) Start(ctx context.Context, operation string) func(lines int, bytes int64) *Profile { + if !p.enabled { + return func(int, int64) *Profile { return nil } + } + + profile := &Profile{ + Operation: operation, + Timestamp: time.Now(), + } + + var memBefore runtime.MemStats + runtime.ReadMemStats(&memBefore) + profile.MemoryBefore = memBefore.Alloc + + start := time.Now() + + return func(lines int, bytes int64) *Profile { + profile.Duration = time.Since(start) + profile.LinesCount = lines + profile.BytesCount = bytes + + var memAfter runtime.MemStats + runtime.ReadMemStats(&memAfter) + profile.MemoryAfter = memAfter.Alloc + profile.MemoryDelta = safeMemoryDelta(memAfter.Alloc, memBefore.Alloc) + + if p.logger != nil { + p.logger.InfoContext(ctx, "Performance profile", "profile", profile.String()) + } + + return profile + } +} + +var globalProfiler *Profiler + +// IsProfilingEnabled checks if profiling is enabled via environment variables +func IsProfilingEnabled() bool { + if enabled, err := strconv.ParseBool(os.Getenv("GITHUB_MCP_PROFILING_ENABLED")); err == nil { + return enabled + } + return false +} + +// Init initializes the global profiler +func Init(logger *slog.Logger, enabled bool) { + globalProfiler = New(logger, enabled) +} + +// InitFromEnv initializes the global profiler using environment variables +func InitFromEnv(logger *slog.Logger) { + globalProfiler = New(logger, IsProfilingEnabled()) +} + +// ProfileFunc profiles a function using the global profiler +func ProfileFunc(ctx context.Context, operation string, fn func() error) (*Profile, error) { + if globalProfiler == nil { + return nil, fn() + } + return globalProfiler.ProfileFunc(ctx, operation, fn) +} + +// ProfileFuncWithMetrics profiles a function with metrics using the global profiler +func ProfileFuncWithMetrics(ctx context.Context, operation string, fn func() (int, int64, error)) (*Profile, error) { + if globalProfiler == nil { + _, _, err := fn() + return nil, err + } + return globalProfiler.ProfileFuncWithMetrics(ctx, operation, fn) +} + +// Start begins timing using the global profiler +func Start(ctx context.Context, operation string) func(int, int64) *Profile { + if globalProfiler == nil { + return func(int, int64) *Profile { return nil } + } + return globalProfiler.Start(ctx, operation) +} diff --git a/pkg/buffer/buffer.go b/pkg/buffer/buffer.go new file mode 100644 index 000000000..546b5324c --- /dev/null +++ b/pkg/buffer/buffer.go @@ -0,0 +1,69 @@ +package buffer + +import ( + "bufio" + "fmt" + "net/http" + "strings" +) + +// ProcessResponseAsRingBufferToEnd reads the body of an HTTP response line by line, +// storing only the last maxJobLogLines lines using a ring buffer (sliding window). +// This efficiently retains the most recent lines, overwriting older ones as needed. +// +// Parameters: +// +// httpResp: The HTTP response whose body will be read. +// maxJobLogLines: The maximum number of log lines to retain. +// +// Returns: +// +// string: The concatenated log lines (up to maxJobLogLines), separated by newlines. +// int: The total number of lines read from the response. +// *http.Response: The original HTTP response. +// error: Any error encountered during reading. +// +// The function uses a ring buffer to efficiently store only the last maxJobLogLines lines. +// If the response contains more lines than maxJobLogLines, only the most recent lines are kept. +func ProcessResponseAsRingBufferToEnd(httpResp *http.Response, maxJobLogLines int) (string, int, *http.Response, error) { + lines := make([]string, maxJobLogLines) + validLines := make([]bool, maxJobLogLines) + totalLines := 0 + writeIndex := 0 + + scanner := bufio.NewScanner(httpResp.Body) + scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024) + + for scanner.Scan() { + line := scanner.Text() + totalLines++ + + lines[writeIndex] = line + validLines[writeIndex] = true + writeIndex = (writeIndex + 1) % maxJobLogLines + } + + if err := scanner.Err(); err != nil { + return "", 0, httpResp, fmt.Errorf("failed to read log content: %w", err) + } + + var result []string + linesInBuffer := totalLines + if linesInBuffer > maxJobLogLines { + linesInBuffer = maxJobLogLines + } + + startIndex := 0 + if totalLines > maxJobLogLines { + startIndex = writeIndex + } + + for i := 0; i < linesInBuffer; i++ { + idx := (startIndex + i) % maxJobLogLines + if validLines[idx] { + result = append(result, lines[idx]) + } + } + + return strings.Join(result, "\n"), totalLines, httpResp, nil +} diff --git a/pkg/github/actions.go b/pkg/github/actions.go index 38719f155..ace9d7288 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -4,11 +4,12 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "strconv" "strings" + "github.com/github/github-mcp-server/internal/profiler" + buffer "github.com/github/github-mcp-server/pkg/buffer" ghErrors "github.com/github/github-mcp-server/pkg/errors" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v74/github" @@ -530,7 +531,7 @@ func ListWorkflowJobs(getClient GetClientFn, t translations.TranslationHelperFun } // GetJobLogs creates a tool to download logs for a specific workflow job or efficiently get all failed job logs for a workflow run -func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc, contentWindowSize int) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("get_job_logs", mcp.WithDescription(t("TOOL_GET_JOB_LOGS_DESCRIPTION", "Download logs for a specific workflow job or efficiently get all failed job logs for a workflow run")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ @@ -613,10 +614,10 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc) (to if failedOnly && runID > 0 { // Handle failed-only mode: get logs for all failed jobs in the workflow run - return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines) + return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, contentWindowSize) } else if jobID > 0 { // Handle single job mode - return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines) + return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, contentWindowSize) } return mcp.NewToolResultError("Either job_id must be provided for single job logs, or run_id with failed_only=true for failed job logs"), nil @@ -624,7 +625,7 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc) (to } // handleFailedJobLogs gets logs for all failed jobs in a workflow run -func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int) (*mcp.CallToolResult, error) { +func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { // First, get all jobs for the workflow run jobs, resp, err := client.Actions.ListWorkflowJobs(ctx, owner, repo, runID, &github.ListWorkflowJobsOptions{ Filter: "latest", @@ -656,7 +657,7 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo // Collect logs for all failed jobs var logResults []map[string]any for _, job := range failedJobs { - jobResult, resp, err := getJobLogData(ctx, client, owner, repo, job.GetID(), job.GetName(), returnContent, tailLines) + jobResult, resp, err := getJobLogData(ctx, client, owner, repo, job.GetID(), job.GetName(), returnContent, tailLines, contentWindowSize) if err != nil { // Continue with other jobs even if one fails jobResult = map[string]any{ @@ -689,8 +690,8 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo } // handleSingleJobLogs gets logs for a single job -func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, tailLines int) (*mcp.CallToolResult, error) { - jobResult, resp, err := getJobLogData(ctx, client, owner, repo, jobID, "", returnContent, tailLines) +func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { + jobResult, resp, err := getJobLogData(ctx, client, owner, repo, jobID, "", returnContent, tailLines, contentWindowSize) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get job logs", resp, err), nil } @@ -704,7 +705,7 @@ func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo } // getJobLogData retrieves log data for a single job, either as URL or content -func getJobLogData(ctx context.Context, client *github.Client, owner, repo string, jobID int64, jobName string, returnContent bool, tailLines int) (map[string]any, *github.Response, error) { +func getJobLogData(ctx context.Context, client *github.Client, owner, repo string, jobID int64, jobName string, returnContent bool, tailLines int, contentWindowSize int) (map[string]any, *github.Response, error) { // Get the download URL for the job logs url, resp, err := client.Actions.GetWorkflowJobLogs(ctx, owner, repo, jobID, 1) if err != nil { @@ -721,7 +722,7 @@ func getJobLogData(ctx context.Context, client *github.Client, owner, repo strin if returnContent { // Download and return the actual log content - content, originalLength, httpResp, err := downloadLogContent(url.String(), tailLines) //nolint:bodyclose // Response body is closed in downloadLogContent, but we need to return httpResp + content, originalLength, httpResp, err := downloadLogContent(ctx, url.String(), tailLines, contentWindowSize) //nolint:bodyclose // Response body is closed in downloadLogContent, but we need to return httpResp if err != nil { // To keep the return value consistent wrap the response as a GitHub Response ghRes := &github.Response{ @@ -742,9 +743,11 @@ func getJobLogData(ctx context.Context, client *github.Client, owner, repo strin return result, resp, nil } -// downloadLogContent downloads the actual log content from a GitHub logs URL -func downloadLogContent(logURL string, tailLines int) (string, int, *http.Response, error) { - httpResp, err := http.Get(logURL) //nolint:gosec // URLs are provided by GitHub API and are safe +func downloadLogContent(ctx context.Context, logURL string, tailLines int, maxLines int) (string, int, *http.Response, error) { + prof := profiler.New(nil, profiler.IsProfilingEnabled()) + finish := prof.Start(ctx, "log_buffer_processing") + + httpResp, err := http.Get(logURL) //nolint:gosec if err != nil { return "", 0, httpResp, fmt.Errorf("failed to download logs: %w", err) } @@ -754,36 +757,25 @@ func downloadLogContent(logURL string, tailLines int) (string, int, *http.Respon return "", 0, httpResp, fmt.Errorf("failed to download logs: HTTP %d", httpResp.StatusCode) } - content, err := io.ReadAll(httpResp.Body) + bufferSize := tailLines + if bufferSize > maxLines { + bufferSize = maxLines + } + + processedInput, totalLines, httpResp, err := buffer.ProcessResponseAsRingBufferToEnd(httpResp, bufferSize) if err != nil { - return "", 0, httpResp, fmt.Errorf("failed to read log content: %w", err) + return "", 0, httpResp, fmt.Errorf("failed to process log content: %w", err) } - // Clean up and format the log content for better readability - logContent := strings.TrimSpace(string(content)) + lines := strings.Split(processedInput, "\n") + if len(lines) > tailLines { + lines = lines[len(lines)-tailLines:] + } + finalResult := strings.Join(lines, "\n") - trimmedContent, lineCount := trimContent(logContent, tailLines) - return trimmedContent, lineCount, httpResp, nil -} + _ = finish(len(lines), int64(len(finalResult))) -// trimContent trims the content to a maximum length and returns the trimmed content and an original length -func trimContent(content string, tailLines int) (string, int) { - // Truncate to tail_lines if specified - lineCount := 0 - if tailLines > 0 { - - // Count backwards to find the nth newline from the end and a total number of lines - for i := len(content) - 1; i >= 0 && lineCount < tailLines; i-- { - if content[i] == '\n' { - lineCount++ - // If we have reached the tailLines, trim the content - if lineCount == tailLines { - content = content[i+1:] - } - } - } - } - return content, lineCount + return finalResult, totalLines, httpResp, nil } // RerunWorkflowRun creates a tool to re-run an entire workflow run diff --git a/pkg/github/actions_test.go b/pkg/github/actions_test.go index 3d7521125..555ec04cb 100644 --- a/pkg/github/actions_test.go +++ b/pkg/github/actions_test.go @@ -3,10 +3,17 @@ package github import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" + "os" + "runtime" + "runtime/debug" + "strings" "testing" + "github.com/github/github-mcp-server/internal/profiler" + buffer "github.com/github/github-mcp-server/pkg/buffer" "github.com/github/github-mcp-server/pkg/translations" "github.com/google/go-github/v74/github" "github.com/migueleliasweb/go-github-mock/src/mock" @@ -807,7 +814,7 @@ func Test_GetWorkflowRunUsage(t *testing.T) { func Test_GetJobLogs(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := GetJobLogs(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := GetJobLogs(stubGetClientFn(mockClient), translations.NullTranslationHelper, 5000) assert.Equal(t, "get_job_logs", tool.Name) assert.NotEmpty(t, tool.Description) @@ -1036,7 +1043,7 @@ func Test_GetJobLogs(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper, 5000) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1095,7 +1102,7 @@ func Test_GetJobLogs_WithContentReturn(t *testing.T) { ) client := github.NewClient(mockedClient) - _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper, 5000) request := createMCPRequest(map[string]any{ "owner": "owner", @@ -1142,7 +1149,7 @@ func Test_GetJobLogs_WithContentReturnAndTailLines(t *testing.T) { ) client := github.NewClient(mockedClient) - _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper, 5000) request := createMCPRequest(map[string]any{ "owner": "owner", @@ -1162,8 +1169,153 @@ func Test_GetJobLogs_WithContentReturnAndTailLines(t *testing.T) { require.NoError(t, err) assert.Equal(t, float64(123), response["job_id"]) - assert.Equal(t, float64(1), response["original_length"]) + assert.Equal(t, float64(3), response["original_length"]) assert.Equal(t, expectedLogContent, response["logs_content"]) assert.Equal(t, "Job logs content retrieved successfully", response["message"]) assert.NotContains(t, response, "logs_url") // Should not have URL when returning content } + +func Test_GetJobLogs_WithContentReturnAndLargeTailLines(t *testing.T) { + logContent := "Line 1\nLine 2\nLine 3" + expectedLogContent := "Line 1\nLine 2\nLine 3" + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(logContent)) + })) + defer testServer.Close() + + mockedClient := mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.GetReposActionsJobsLogsByOwnerByRepoByJobId, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Location", testServer.URL) + w.WriteHeader(http.StatusFound) + }), + ), + ) + + client := github.NewClient(mockedClient) + _, handler := GetJobLogs(stubGetClientFn(client), translations.NullTranslationHelper, 5000) + + request := createMCPRequest(map[string]any{ + "owner": "owner", + "repo": "repo", + "job_id": float64(123), + "return_content": true, + "tail_lines": float64(100), + }) + + result, err := handler(context.Background(), request) + require.NoError(t, err) + require.False(t, result.IsError) + + textContent := getTextResult(t, result) + var response map[string]any + err = json.Unmarshal([]byte(textContent.Text), &response) + require.NoError(t, err) + + assert.Equal(t, float64(123), response["job_id"]) + assert.Equal(t, float64(3), response["original_length"]) + assert.Equal(t, expectedLogContent, response["logs_content"]) + assert.Equal(t, "Job logs content retrieved successfully", response["message"]) + assert.NotContains(t, response, "logs_url") +} + +func Test_MemoryUsage_SlidingWindow_vs_NoWindow(t *testing.T) { + if testing.Short() { + t.Skip("Skipping memory profiling test in short mode") + } + + const logLines = 100000 + const bufferSize = 5000 + largeLogContent := strings.Repeat("log line with some content\n", logLines-1) + "final log line" + + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(largeLogContent)) + })) + defer testServer.Close() + + os.Setenv("GITHUB_MCP_PROFILING_ENABLED", "true") + defer os.Unsetenv("GITHUB_MCP_PROFILING_ENABLED") + + profiler.InitFromEnv(nil) + ctx := context.Background() + + debug.SetGCPercent(-1) + defer debug.SetGCPercent(100) + + for i := 0; i < 3; i++ { + runtime.GC() + } + + var baselineStats runtime.MemStats + runtime.ReadMemStats(&baselineStats) + + profile1, err1 := profiler.ProfileFuncWithMetrics(ctx, "sliding_window", func() (int, int64, error) { + resp1, err := http.Get(testServer.URL) + if err != nil { + return 0, 0, err + } + defer resp1.Body.Close() //nolint:bodyclose + content, totalLines, _, err := buffer.ProcessResponseAsRingBufferToEnd(resp1, bufferSize) //nolint:bodyclose + return totalLines, int64(len(content)), err + }) + require.NoError(t, err1) + + for i := 0; i < 3; i++ { + runtime.GC() + } + + profile2, err2 := profiler.ProfileFuncWithMetrics(ctx, "no_window", func() (int, int64, error) { + resp2, err := http.Get(testServer.URL) + if err != nil { + return 0, 0, err + } + defer resp2.Body.Close() //nolint:bodyclose + + allContent, err := io.ReadAll(resp2.Body) + if err != nil { + return 0, 0, err + } + + allLines := strings.Split(string(allContent), "\n") + var nonEmptyLines []string + for _, line := range allLines { + if line != "" { + nonEmptyLines = append(nonEmptyLines, line) + } + } + totalLines := len(nonEmptyLines) + + var resultLines []string + if totalLines > bufferSize { + resultLines = nonEmptyLines[totalLines-bufferSize:] + } else { + resultLines = nonEmptyLines + } + + result := strings.Join(resultLines, "\n") + return totalLines, int64(len(result)), nil + }) + require.NoError(t, err2) + + assert.Greater(t, profile2.MemoryDelta, profile1.MemoryDelta, + "Sliding window should use less memory than reading all into memory") + + assert.Equal(t, profile1.LinesCount, profile2.LinesCount, + "Both approaches should count the same number of input lines") + assert.InDelta(t, profile1.BytesCount, profile2.BytesCount, 100, + "Both approaches should produce similar output sizes (within 100 bytes)") + + memoryReduction := float64(profile2.MemoryDelta-profile1.MemoryDelta) / float64(profile2.MemoryDelta) * 100 + t.Logf("Memory reduction: %.1f%% (%.2f MB vs %.2f MB)", + memoryReduction, + float64(profile2.MemoryDelta)/1024/1024, + float64(profile1.MemoryDelta)/1024/1024) + + t.Logf("Baseline: %d bytes", baselineStats.Alloc) + t.Logf("Sliding window: %s", profile1.String()) + t.Logf("No window: %s", profile2.String()) +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 3fb39ada7..b50499650 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -16,7 +16,7 @@ type GetGQLClientFn func(context.Context) (*githubv4.Client, error) var DefaultTools = []string{"all"} -func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc) *toolsets.ToolsetGroup { +func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc, contentWindowSize int) *toolsets.ToolsetGroup { tsg := toolsets.NewToolsetGroup(readOnly) // Define all available features with their default state (disabled) @@ -146,7 +146,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(GetWorkflowRun(getClient, t)), toolsets.NewServerTool(GetWorkflowRunLogs(getClient, t)), toolsets.NewServerTool(ListWorkflowJobs(getClient, t)), - toolsets.NewServerTool(GetJobLogs(getClient, t)), + toolsets.NewServerTool(GetJobLogs(getClient, t, contentWindowSize)), toolsets.NewServerTool(ListWorkflowRunArtifacts(getClient, t)), toolsets.NewServerTool(DownloadWorkflowRunArtifact(getClient, t)), toolsets.NewServerTool(GetWorkflowRunUsage(getClient, t)),