Skip to content

Commit 3d4fa49

Browse files
fix(mcp_server): Fix job_logs tool not working for large logs (#724)
## 📝 Description - `jobs_logs` now always feeds the cursor (including `0`) to Loghub, normalizes the returned slice, and consistently shows either the newest `maxLogPreviewLines` or the requested chunk; `StartLine`, `PreviewTruncated`, and `nextCursor` match the user-visible contract. - Added a table-driven regression test exercising initial truncation, short logs, and cursor-driven pagination, ensuring cursors walk backward and previews contain the expected lines. ## ✅ Checklist - [x] I have tested this change - [x] This change requires a documentation update --------- Co-authored-by: Amir Hasanbasic <43892661+hamir-suspect@users.noreply.github.com>
1 parent 6ad0696 commit 3d4fa49

File tree

2 files changed

+185
-58
lines changed

2 files changed

+185
-58
lines changed

mcp_server/pkg/tools/jobs/jobs_test.go

Lines changed: 131 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package jobs
22

33
import (
44
"context"
5+
"fmt"
56
"net/http"
67
"strings"
78
"testing"
@@ -287,60 +288,149 @@ func TestDescribeJobScopeMismatchMissingProject(t *testing.T) {
287288
}
288289
}
289290

290-
func TestFetchHostedLogs(t *testing.T) {
291-
jobID := "99999999-aaaa-bbbb-cccc-dddddddddddd"
291+
func TestFetchHostedLogsPagination(t *testing.T) {
292+
const orgID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
293+
const jobID = "99999999-aaaa-bbbb-cccc-dddddddddddd"
294+
292295
jobClient := &jobClientStub{
293296
describeResp: &jobpb.DescribeResponse{
294297
Status: &responsepb.ResponseStatus{Code: responsepb.ResponseStatus_OK},
295298
Job: &jobpb.Job{
296299
Id: jobID,
297300
ProjectId: "proj-1",
298-
OrganizationId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
301+
OrganizationId: orgID,
299302
SelfHosted: false,
300303
},
301304
},
302305
}
303-
loghubClient := &loghubClientStub{
304-
resp: &loghubpb.GetLogEventsResponse{
305-
Status: &responsepb.ResponseStatus{Code: responsepb.ResponseStatus_OK},
306-
Events: []string{"line1", "line2"},
307-
Final: false,
308-
},
309-
}
310-
311-
provider := &support.MockProvider{
312-
JobClient: jobClient,
313-
LoghubClient: loghubClient,
314-
RBACClient: newRBACStub("project.view"),
315-
Timeout: time.Second,
316-
}
317-
318-
handler := logsHandler(provider)
319-
req := mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: map[string]any{
320-
"organization_id": "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
321-
"job_id": jobID,
322-
"cursor": "5",
323-
}}}
324-
header := http.Header{}
325-
header.Set("X-Semaphore-User-ID", "99999999-aaaa-bbbb-cccc-dddddddddddd")
326-
req.Header = header
327-
328-
res, err := handler(context.Background(), req)
329-
if err != nil {
330-
toFail(t, "handler error: %v", err)
331-
}
332306

333-
result, ok := res.StructuredContent.(logsResult)
334-
if !ok {
335-
toFail(t, "unexpected structured content type: %T", res.StructuredContent)
336-
}
337-
338-
if result.Source != loghubSource || result.NextCursor != "7" || len(result.Preview) != 2 {
339-
toFail(t, "unexpected log result: %+v", result)
307+
makeEvents := func(start, end int) []string {
308+
events := make([]string, end-start)
309+
for i := start; i < end; i++ {
310+
events[i-start] = fmt.Sprintf("line-%d", i)
311+
}
312+
return events
313+
}
314+
315+
testCases := []struct {
316+
name string
317+
cursor string
318+
events []string
319+
expectedStart int
320+
expectedLen int
321+
expectedFirst string
322+
expectedLast string
323+
expectedCursor string
324+
truncated bool
325+
}{
326+
{
327+
name: "initialRequestTruncatesToNewestLines",
328+
cursor: "",
329+
events: makeEvents(0, 500),
330+
expectedStart: 500 - maxLogPreviewLines,
331+
expectedLen: maxLogPreviewLines,
332+
expectedFirst: fmt.Sprintf("line-%d", 500-maxLogPreviewLines),
333+
expectedLast: "line-499",
334+
expectedCursor: fmt.Sprintf("%d", 500-(2*maxLogPreviewLines)),
335+
truncated: true,
336+
},
337+
{
338+
name: "initialRequestShortLog",
339+
cursor: "",
340+
events: makeEvents(0, 100),
341+
expectedStart: 0,
342+
expectedLen: 100,
343+
expectedFirst: "line-0",
344+
expectedLast: "line-99",
345+
expectedCursor: "",
346+
truncated: false,
347+
},
348+
{
349+
name: "cursorInMiddleReturnsOlderChunk",
350+
cursor: "150",
351+
events: makeEvents(150, 500),
352+
expectedStart: 150,
353+
expectedLen: maxLogPreviewLines,
354+
expectedFirst: "line-150",
355+
expectedLast: "line-349",
356+
expectedCursor: "0",
357+
truncated: true,
358+
},
359+
{
360+
name: "cursorAtBeginningHasNoFurtherPages",
361+
cursor: "0",
362+
events: makeEvents(0, 500),
363+
expectedStart: 0,
364+
expectedLen: maxLogPreviewLines,
365+
expectedFirst: "line-0",
366+
expectedLast: "line-199",
367+
expectedCursor: "",
368+
truncated: true,
369+
},
340370
}
341371

342-
if loghubClient.lastRequest == nil || loghubClient.lastRequest.GetStartingLine() != 5 {
343-
toFail(t, "unexpected loghub request: %+v", loghubClient.lastRequest)
372+
for _, tc := range testCases {
373+
tc := tc
374+
t.Run(tc.name, func(t *testing.T) {
375+
loghubClient := &loghubClientStub{
376+
resp: &loghubpb.GetLogEventsResponse{
377+
Status: &responsepb.ResponseStatus{Code: responsepb.ResponseStatus_OK},
378+
Events: tc.events,
379+
Final: false,
380+
},
381+
}
382+
383+
provider := &support.MockProvider{
384+
JobClient: jobClient,
385+
LoghubClient: loghubClient,
386+
RBACClient: newRBACStub("project.view"),
387+
Timeout: time.Second,
388+
}
389+
390+
handler := logsHandler(provider)
391+
args := map[string]any{
392+
"organization_id": orgID,
393+
"job_id": jobID,
394+
}
395+
if tc.cursor != "" {
396+
args["cursor"] = tc.cursor
397+
}
398+
req := mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: args}}
399+
header := http.Header{}
400+
header.Set("X-Semaphore-User-ID", "99999999-aaaa-bbbb-cccc-dddddddddddd")
401+
req.Header = header
402+
403+
res, err := handler(context.Background(), req)
404+
if err != nil {
405+
toFail(t, "handler error: %v", err)
406+
}
407+
408+
result, ok := res.StructuredContent.(logsResult)
409+
if !ok {
410+
toFail(t, "unexpected structured content type: %T", res.StructuredContent)
411+
}
412+
413+
if result.StartLine != tc.expectedStart {
414+
toFail(t, "expected start line %d, got %d", tc.expectedStart, result.StartLine)
415+
}
416+
if len(result.Preview) != tc.expectedLen {
417+
toFail(t, "expected preview length %d, got %d", tc.expectedLen, len(result.Preview))
418+
}
419+
if tc.expectedLen > 0 {
420+
if got := result.Preview[0]; got != tc.expectedFirst {
421+
toFail(t, "unexpected first preview line: %s", got)
422+
}
423+
if got := result.Preview[len(result.Preview)-1]; got != tc.expectedLast {
424+
toFail(t, "unexpected last preview line: %s", got)
425+
}
426+
}
427+
if result.NextCursor != tc.expectedCursor {
428+
toFail(t, "expected next cursor %q, got %q", tc.expectedCursor, result.NextCursor)
429+
}
430+
if result.PreviewTruncated != tc.truncated {
431+
toFail(t, "expected truncated=%v, got %v", tc.truncated, result.PreviewTruncated)
432+
}
433+
})
344434
}
345435
}
346436

mcp_server/pkg/tools/jobs/logs.go

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ type logsResult struct {
6161
Source string `json:"source"`
6262
Preview []string `json:"preview,omitempty"`
6363
NextCursor string `json:"nextCursor,omitempty"`
64-
Final bool `json:"final,omitempty"`
64+
Final bool `json:"Final,omitempty"`
6565
StartLine int `json:"startLine,omitempty"`
6666
PreviewTruncated bool `json:"previewTruncated,omitempty"`
6767
Token string `json:"token,omitempty"`
@@ -206,11 +206,11 @@ Troubleshooting:
206206

207207
func parseCursor(cursor string) (int, error) {
208208
if cursor == "" {
209-
return 0, nil
209+
return -1, nil
210210
}
211211
value, err := strconv.Atoi(cursor)
212212
if err != nil || value < 0 {
213-
return 0, fmt.Errorf("cursor must be a non-negative integer produced by the previous response (got %q)", cursor)
213+
return -1, fmt.Errorf("cursor must be a non-negative integer produced by the previous response (got %q)", cursor)
214214
}
215215
return value, nil
216216
}
@@ -222,7 +222,7 @@ func fetchHostedLogs(ctx context.Context, api internalapi.Provider, jobID string
222222
}
223223

224224
request := &loghubpb.GetLogEventsRequest{JobId: jobID}
225-
if startingLine > 0 {
225+
if startingLine >= 0 {
226226
offset, err := utils.IntToInt32(startingLine, "cursor offset")
227227
if err != nil {
228228
return mcp.NewToolResultError(err.Error()), nil
@@ -257,24 +257,59 @@ func fetchHostedLogs(ctx context.Context, api internalapi.Provider, jobID string
257257
}
258258

259259
events := append([]string(nil), resp.GetEvents()...)
260-
displayEvents := events
260+
totalEvents := len(events)
261+
displayStartLine := 0
261262
truncated := false
262-
if len(events) > maxLogPreviewLines {
263-
displayEvents, truncated = shared.TruncateList(events, maxLogPreviewLines)
263+
relativeStart := 0
264+
relativeEnd := totalEvents
265+
266+
if startingLine < 0 {
267+
if totalEvents > maxLogPreviewLines {
268+
displayStartLine = totalEvents - maxLogPreviewLines
269+
relativeStart = displayStartLine
270+
truncated = true
271+
}
272+
} else {
273+
displayStartLine = startingLine
274+
truncated = true
275+
relativeEnd = maxLogPreviewLines
276+
if relativeEnd > totalEvents {
277+
relativeEnd = totalEvents
278+
}
279+
}
280+
281+
if relativeStart < 0 {
282+
relativeStart = 0
283+
}
284+
if relativeEnd > totalEvents {
285+
relativeEnd = totalEvents
286+
}
287+
if relativeStart > relativeEnd {
288+
relativeStart = relativeEnd
289+
}
290+
291+
displayEvents := events[relativeStart:relativeEnd]
292+
293+
var nextCursor string
294+
if displayStartLine > 0 {
295+
prev := displayStartLine - maxLogPreviewLines
296+
if prev < 0 {
297+
prev = 0
298+
}
299+
nextCursor = strconv.Itoa(prev)
264300
}
265301

266302
result := logsResult{
267303
JobID: jobID,
268304
Source: loghubSource,
269305
Preview: displayEvents,
270306
Final: resp.GetFinal(),
271-
StartLine: startingLine,
307+
StartLine: displayStartLine,
272308
PreviewTruncated: truncated,
273309
}
274310

275-
if !resp.GetFinal() && len(events) > 0 {
276-
next := startingLine + len(events)
277-
result.NextCursor = strconv.Itoa(next)
311+
if nextCursor != "" {
312+
result.NextCursor = nextCursor
278313
}
279314

280315
markdown := formatHostedLogsMarkdown(result)
@@ -348,22 +383,24 @@ func formatHostedLogsMarkdown(result logsResult) string {
348383
start = 0
349384
}
350385

351-
mb.Paragraph(fmt.Sprintf("Showing log lines %d-%d (newest first).", start, end))
386+
mb.Paragraph(fmt.Sprintf("Showing log lines %d-%d.", start, end))
352387
mb.Raw("```\n")
353388
mb.Raw(strings.Join(result.Preview, "\n"))
354389
mb.Raw("\n```\n")
355390

356391
if result.PreviewTruncated {
357-
mb.Paragraph(fmt.Sprintf("⚠️ Preview truncated to the most recent %d lines. Use pagination to retrieve the full log.", maxLogPreviewLines))
392+
if result.NextCursor != "" {
393+
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))
394+
} else {
395+
mb.Paragraph("⚠️ Preview is truncated. No further logs are available at this time.")
396+
}
358397
}
359398
}
360399

361400
if result.Final {
362-
mb.Paragraph("✅ This job reported final logs. No additional pages are available.")
363-
} else if result.NextCursor != "" {
364-
mb.Paragraph(fmt.Sprintf("📄 **More available**. Use `cursor=\"%s\"`", result.NextCursor))
401+
mb.Paragraph("✅ This job is finished and it reported final logs.")
365402
} else {
366-
mb.Paragraph("ℹ️ Logs are still streaming. Retry shortly for additional output.")
403+
mb.Paragraph("ℹ️ Job is still running and logs are still streaming. Retry shortly without cursor to fetch most recent output.")
367404
}
368405

369406
mb.Line()

0 commit comments

Comments
 (0)