fix(mcp_server): Fix job_logs tool not working for large logs

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)
+			}
+		})
 	}
 }
 

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,24 +257,59 @@ 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{
 		JobID:            jobID,
 		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()