diff --git a/mcp_server/pkg/tools/jobs/jobs_test.go b/mcp_server/pkg/tools/jobs/jobs_test.go index 9912d6dd0..343fa82f3 100644 --- a/mcp_server/pkg/tools/jobs/jobs_test.go +++ b/mcp_server/pkg/tools/jobs/jobs_test.go @@ -2,6 +2,7 @@ package jobs import ( "context" + "fmt" "net/http" "strings" "testing" @@ -287,60 +288,149 @@ func TestDescribeJobScopeMismatchMissingProject(t *testing.T) { } } -func TestFetchHostedLogs(t *testing.T) { - jobID := "99999999-aaaa-bbbb-cccc-dddddddddddd" +func TestFetchHostedLogsPagination(t *testing.T) { + const orgID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + const jobID = "99999999-aaaa-bbbb-cccc-dddddddddddd" + jobClient := &jobClientStub{ describeResp: &jobpb.DescribeResponse{ Status: &responsepb.ResponseStatus{Code: responsepb.ResponseStatus_OK}, Job: &jobpb.Job{ Id: jobID, ProjectId: "proj-1", - OrganizationId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", + OrganizationId: orgID, SelfHosted: false, }, }, } - loghubClient := &loghubClientStub{ - resp: &loghubpb.GetLogEventsResponse{ - Status: &responsepb.ResponseStatus{Code: responsepb.ResponseStatus_OK}, - Events: []string{"line1", "line2"}, - Final: false, - }, - } - - provider := &support.MockProvider{ - JobClient: jobClient, - LoghubClient: loghubClient, - RBACClient: newRBACStub("project.view"), - Timeout: time.Second, - } - - handler := logsHandler(provider) - req := mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: map[string]any{ - "organization_id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", - "job_id": jobID, - "cursor": "5", - }}} - header := http.Header{} - header.Set("X-Semaphore-User-ID", "99999999-aaaa-bbbb-cccc-dddddddddddd") - req.Header = header - - res, err := handler(context.Background(), req) - if err != nil { - toFail(t, "handler error: %v", err) - } - result, ok := res.StructuredContent.(logsResult) - if !ok { - toFail(t, "unexpected structured content type: %T", res.StructuredContent) - } - - if result.Source != loghubSource || result.NextCursor != "7" || len(result.Preview) != 2 { - toFail(t, "unexpected log result: %+v", result) + makeEvents := func(start, end int) []string { + events := make([]string, end-start) + for i := start; i < end; i++ { + events[i-start] = fmt.Sprintf("line-%d", i) + } + return events + } + + testCases := []struct { + name string + cursor string + events []string + expectedStart int + expectedLen int + expectedFirst string + expectedLast string + expectedCursor string + truncated bool + }{ + { + name: "initialRequestTruncatesToNewestLines", + cursor: "", + events: makeEvents(0, 500), + expectedStart: 500 - maxLogPreviewLines, + expectedLen: maxLogPreviewLines, + expectedFirst: fmt.Sprintf("line-%d", 500-maxLogPreviewLines), + expectedLast: "line-499", + expectedCursor: fmt.Sprintf("%d", 500-(2*maxLogPreviewLines)), + truncated: true, + }, + { + name: "initialRequestShortLog", + cursor: "", + events: makeEvents(0, 100), + expectedStart: 0, + expectedLen: 100, + expectedFirst: "line-0", + expectedLast: "line-99", + expectedCursor: "", + truncated: false, + }, + { + name: "cursorInMiddleReturnsOlderChunk", + cursor: "150", + events: makeEvents(150, 500), + expectedStart: 150, + expectedLen: maxLogPreviewLines, + expectedFirst: "line-150", + expectedLast: "line-349", + expectedCursor: "0", + truncated: true, + }, + { + name: "cursorAtBeginningHasNoFurtherPages", + cursor: "0", + events: makeEvents(0, 500), + expectedStart: 0, + expectedLen: maxLogPreviewLines, + expectedFirst: "line-0", + expectedLast: "line-199", + expectedCursor: "", + truncated: true, + }, } - if loghubClient.lastRequest == nil || loghubClient.lastRequest.GetStartingLine() != 5 { - toFail(t, "unexpected loghub request: %+v", loghubClient.lastRequest) + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + loghubClient := &loghubClientStub{ + resp: &loghubpb.GetLogEventsResponse{ + Status: &responsepb.ResponseStatus{Code: responsepb.ResponseStatus_OK}, + Events: tc.events, + Final: false, + }, + } + + provider := &support.MockProvider{ + JobClient: jobClient, + LoghubClient: loghubClient, + RBACClient: newRBACStub("project.view"), + Timeout: time.Second, + } + + handler := logsHandler(provider) + args := map[string]any{ + "organization_id": orgID, + "job_id": jobID, + } + if tc.cursor != "" { + args["cursor"] = tc.cursor + } + req := mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: args}} + header := http.Header{} + header.Set("X-Semaphore-User-ID", "99999999-aaaa-bbbb-cccc-dddddddddddd") + req.Header = header + + res, err := handler(context.Background(), req) + if err != nil { + toFail(t, "handler error: %v", err) + } + + result, ok := res.StructuredContent.(logsResult) + if !ok { + toFail(t, "unexpected structured content type: %T", res.StructuredContent) + } + + if result.StartLine != tc.expectedStart { + toFail(t, "expected start line %d, got %d", tc.expectedStart, result.StartLine) + } + if len(result.Preview) != tc.expectedLen { + toFail(t, "expected preview length %d, got %d", tc.expectedLen, len(result.Preview)) + } + if tc.expectedLen > 0 { + if got := result.Preview[0]; got != tc.expectedFirst { + toFail(t, "unexpected first preview line: %s", got) + } + if got := result.Preview[len(result.Preview)-1]; got != tc.expectedLast { + toFail(t, "unexpected last preview line: %s", got) + } + } + if result.NextCursor != tc.expectedCursor { + toFail(t, "expected next cursor %q, got %q", tc.expectedCursor, result.NextCursor) + } + if result.PreviewTruncated != tc.truncated { + toFail(t, "expected truncated=%v, got %v", tc.truncated, result.PreviewTruncated) + } + }) } } diff --git a/mcp_server/pkg/tools/jobs/logs.go b/mcp_server/pkg/tools/jobs/logs.go index 66fad5f90..9e7958930 100644 --- a/mcp_server/pkg/tools/jobs/logs.go +++ b/mcp_server/pkg/tools/jobs/logs.go @@ -61,7 +61,7 @@ type logsResult struct { Source string `json:"source"` Preview []string `json:"preview,omitempty"` NextCursor string `json:"nextCursor,omitempty"` - Final bool `json:"final,omitempty"` + Final bool `json:"Final,omitempty"` StartLine int `json:"startLine,omitempty"` PreviewTruncated bool `json:"previewTruncated,omitempty"` Token string `json:"token,omitempty"` @@ -206,11 +206,11 @@ Troubleshooting: func parseCursor(cursor string) (int, error) { if cursor == "" { - return 0, nil + return -1, nil } value, err := strconv.Atoi(cursor) if err != nil || value < 0 { - return 0, fmt.Errorf("cursor must be a non-negative integer produced by the previous response (got %q)", cursor) + return -1, fmt.Errorf("cursor must be a non-negative integer produced by the previous response (got %q)", cursor) } return value, nil } @@ -222,7 +222,7 @@ func fetchHostedLogs(ctx context.Context, api internalapi.Provider, jobID string } request := &loghubpb.GetLogEventsRequest{JobId: jobID} - if startingLine > 0 { + if startingLine >= 0 { offset, err := utils.IntToInt32(startingLine, "cursor offset") if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -257,10 +257,46 @@ func fetchHostedLogs(ctx context.Context, api internalapi.Provider, jobID string } events := append([]string(nil), resp.GetEvents()...) - displayEvents := events + totalEvents := len(events) + displayStartLine := 0 truncated := false - if len(events) > maxLogPreviewLines { - displayEvents, truncated = shared.TruncateList(events, maxLogPreviewLines) + relativeStart := 0 + relativeEnd := totalEvents + + if startingLine < 0 { + if totalEvents > maxLogPreviewLines { + displayStartLine = totalEvents - maxLogPreviewLines + relativeStart = displayStartLine + truncated = true + } + } else { + displayStartLine = startingLine + truncated = true + relativeEnd = maxLogPreviewLines + if relativeEnd > totalEvents { + relativeEnd = totalEvents + } + } + + if relativeStart < 0 { + relativeStart = 0 + } + if relativeEnd > totalEvents { + relativeEnd = totalEvents + } + if relativeStart > relativeEnd { + relativeStart = relativeEnd + } + + displayEvents := events[relativeStart:relativeEnd] + + var nextCursor string + if displayStartLine > 0 { + prev := displayStartLine - maxLogPreviewLines + if prev < 0 { + prev = 0 + } + nextCursor = strconv.Itoa(prev) } result := logsResult{ @@ -268,13 +304,12 @@ func fetchHostedLogs(ctx context.Context, api internalapi.Provider, jobID string Source: loghubSource, Preview: displayEvents, Final: resp.GetFinal(), - StartLine: startingLine, + StartLine: displayStartLine, PreviewTruncated: truncated, } - if !resp.GetFinal() && len(events) > 0 { - next := startingLine + len(events) - result.NextCursor = strconv.Itoa(next) + if nextCursor != "" { + result.NextCursor = nextCursor } markdown := formatHostedLogsMarkdown(result) @@ -348,22 +383,24 @@ func formatHostedLogsMarkdown(result logsResult) string { start = 0 } - mb.Paragraph(fmt.Sprintf("Showing log lines %d-%d (newest first).", start, end)) + mb.Paragraph(fmt.Sprintf("Showing log lines %d-%d.", start, end)) mb.Raw("```\n") mb.Raw(strings.Join(result.Preview, "\n")) mb.Raw("\n```\n") if result.PreviewTruncated { - mb.Paragraph(fmt.Sprintf("⚠️ Preview truncated to the most recent %d lines. Use pagination to retrieve the full log.", maxLogPreviewLines)) + if result.NextCursor != "" { + mb.Paragraph(fmt.Sprintf("⚠️ Preview is truncated. If you want to see the full log, paginate using `cursor=\"%s\"` to retrieve additional lines.", result.NextCursor)) + } else { + mb.Paragraph("⚠️ Preview is truncated. No further logs are available at this time.") + } } } if result.Final { - mb.Paragraph("✅ This job reported final logs. No additional pages are available.") - } else if result.NextCursor != "" { - mb.Paragraph(fmt.Sprintf("📄 **More available**. Use `cursor=\"%s\"`", result.NextCursor)) + mb.Paragraph("✅ This job is finished and it reported final logs.") } else { - mb.Paragraph("ℹ️ Logs are still streaming. Retry shortly for additional output.") + mb.Paragraph("ℹ️ Job is still running and logs are still streaming. Retry shortly without cursor to fetch most recent output.") } mb.Line()