enhancement: client side 'task' functions for 'get', 'list', 'cancel' and 'result' operation #770
Merged
ezynda3 merged 3 commits intomark3labs:mainfrom Apr 4, 2026
Merged
enhancement: client side 'task' functions for 'get', 'list', 'cancel' and 'result' operation #770ezynda3 merged 3 commits intomark3labs:mainfrom
ezynda3 merged 3 commits intomark3labs:mainfrom
Conversation
…ations Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds four exported Client methods for task RPCs and four JSON parsing functions plus an internal helper to deserialize task-related MCP responses into typed result structures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
mcp/utils.go (2)
1055-1056: GoDoc comments should start with the identifier name and use proper grammar.Per coding guidelines, exported functions must have GoDoc comments starting with the identifier name. The current comments use lowercase "parse" instead of proper sentence structure.
✏️ Suggested improvements
-// ParseCancelTaskResult parse json message and convert to CancelTaskResult structure +// ParseCancelTaskResult parses a JSON message and converts it to a CancelTaskResult. func ParseCancelTaskResult(rawMessage *json.RawMessage) (*CancelTaskResult, error) { -// ParseListTasksResult parse json message and convert to ListTasksResult structure +// ParseListTasksResult parses a JSON message and converts it to a ListTasksResult. func ParseListTasksResult(rawMessage *json.RawMessage) (*ListTasksResult, error) { -// ParseTaskResultResult parse json message and convert to TaskResultResult structure +// ParseTaskResultResult parses a JSON message and converts it to a TaskResultResult. func ParseTaskResultResult(rawMessage *json.RawMessage) (*TaskResultResult, error) { -// ParseGetTaskResult parse json message and convert to GetTaskResult structure +// ParseGetTaskResult parses a JSON message and converts it to a GetTaskResult. func ParseGetTaskResult(rawMessage *json.RawMessage) (*GetTaskResult, error) {As per coding guidelines: "All exported types and functions must have GoDoc comments starting with the identifier name."
Also applies to: 1080-1081, 1123-1124, 1153-1154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp/utils.go` around lines 1055 - 1056, The GoDoc for the exported function ParseCancelTaskResult does not start with the identifier name and uses incorrect grammar; update its comment to start with "ParseCancelTaskResult" followed by a proper sentence describing its behavior (e.g., "ParseCancelTaskResult parses a JSON message and converts it to a CancelTaskResult.") and apply the same fix to the other exported parser functions referenced in the review (the exported functions at the other noted ranges) so each comment begins with the exact function name and a concise, grammatically correct description.
1001-1053: Consider validating required task fields.The
jsonToTaskfunction silently ignores missing required fields (taskId,status,createdAt,lastUpdatedAt). Callers may receive incompleteTaskobjects without any indication that the response was malformed. Consider returning an error if required fields are missing, consistent with how other parsers likeParseResourceContentshandle missing required data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcp/utils.go` around lines 1001 - 1053, The jsonToTask function currently silently ignores missing required fields and should be changed to validate and report errors: update the signature of jsonToTask to return error (e.g., func jsonToTask(jsonContent map[string]any, result *GetTaskResult) error), check that required keys ("taskId", "status", "createdAt", "lastUpdatedAt") exist and are of the expected types, and return a descriptive error if any are missing or of the wrong type; ensure you still populate optional fields like ttl and pollInterval when present and mirror the error handling style used by ParseResourceContents (construct and return errors rather than panicking or silently omitting fields).client/client.go (1)
735-746: Consider adding pagination support forListTasks.Other list methods in this file (
ListResources,ListPrompts,ListTools) provide both aListXByPagevariant and an auto-paginatingListXmethod that loops untilNextCursoris empty.ListTasksonly provides the single-page variant. If the MCP spec supports pagination for task listing (whichListTasksResultwithPaginatedResultsuggests), consider addingListTasksByPageand an auto-paginatingListTasksfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/client.go` around lines 735 - 746, Add pagination support for tasks by implementing a ListTasksByPage(ctx, request) that calls c.sendRequest with mcp.MethodTasksList and accepts/returns a single page (use mcp.ParseListTasksResult to parse the response and preserve the PaginatedResult/NextCursor), and then change/overload ListTasks to auto-paginate: loop calling ListTasksByPage while the returned PaginatedResult.NextCursor is non-empty, appending items into a cumulative mcp.ListTasksResult (or slice) and returning the aggregated result; reference symbols: Client.ListTasks, new Client.ListTasksByPage, c.sendRequest, mcp.ParseListTasksResult, mcp.ListTasksResult, and PaginatedResult.NextCursor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcp/utils.go`:
- Around line 1017-1022: The code is reading jsonContent["StatusMessage"]
(PascalCase) but the JSON uses "statusMessage" (camelCase), so update the key
lookup in the jsonContent map to "statusMessage" (or check both keys) so
result.StatusMessage is assigned correctly; locate the block that reads
jsonContent and assigns to result.StatusMessage and replace the key string
"StatusMessage" with "statusMessage" (or add a secondary lookup for
"StatusMessage" to maintain backward compatibility).
- Around line 1113-1118: The code is incorrectly asserting nextCursor from
jsonContent as type Cursor which will never succeed after JSON unmarshal; change
the logic in the block handling jsonContent["nextCursor"] (symbols: jsonContent,
nextCursor, Cursor, listTasksResult.PaginatedResult.NextCursor) to first assert
the value as the concrete JSON type (e.g., string or json.RawMessage), then
convert/unmarshal that into a Cursor (for example parse the string or call
json.Unmarshal into a Cursor) and only then assign it to
listTasksResult.PaginatedResult.NextCursor so the pagination value is correctly
extracted.
- Around line 1142-1148: The code currently attempts a type assertion
result.(TaskResultResult) which will always fail because json.Unmarshal into
map[string]any yields maps/primitives, not struct types; update the block that
reads jsonContent["result"] to assert result to map[string]any (e.g. resultMap),
then extract the "content" and "is_error" (or "IsError") fields from that map,
converting types safely (use string for content or reuse ParseCallToolResult
logic to parse nested content structures) and set resultResult.Content and
resultResult.IsError accordingly; ensure you handle absent keys and type
mismatches gracefully and mirror the parsing approach used in
ParseCallToolResult for nested content.
---
Nitpick comments:
In `@client/client.go`:
- Around line 735-746: Add pagination support for tasks by implementing a
ListTasksByPage(ctx, request) that calls c.sendRequest with mcp.MethodTasksList
and accepts/returns a single page (use mcp.ParseListTasksResult to parse the
response and preserve the PaginatedResult/NextCursor), and then change/overload
ListTasks to auto-paginate: loop calling ListTasksByPage while the returned
PaginatedResult.NextCursor is non-empty, appending items into a cumulative
mcp.ListTasksResult (or slice) and returning the aggregated result; reference
symbols: Client.ListTasks, new Client.ListTasksByPage, c.sendRequest,
mcp.ParseListTasksResult, mcp.ListTasksResult, and PaginatedResult.NextCursor.
In `@mcp/utils.go`:
- Around line 1055-1056: The GoDoc for the exported function
ParseCancelTaskResult does not start with the identifier name and uses incorrect
grammar; update its comment to start with "ParseCancelTaskResult" followed by a
proper sentence describing its behavior (e.g., "ParseCancelTaskResult parses a
JSON message and converts it to a CancelTaskResult.") and apply the same fix to
the other exported parser functions referenced in the review (the exported
functions at the other noted ranges) so each comment begins with the exact
function name and a concise, grammatically correct description.
- Around line 1001-1053: The jsonToTask function currently silently ignores
missing required fields and should be changed to validate and report errors:
update the signature of jsonToTask to return error (e.g., func
jsonToTask(jsonContent map[string]any, result *GetTaskResult) error), check that
required keys ("taskId", "status", "createdAt", "lastUpdatedAt") exist and are
of the expected types, and return a descriptive error if any are missing or of
the wrong type; ensure you still populate optional fields like ttl and
pollInterval when present and mirror the error handling style used by
ParseResourceContents (construct and return errors rather than panicking or
silently omitting fields).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 05053549-ffad-4ce3-bb08-a2df1c3da63d
📒 Files selected for processing (2)
client/client.gomcp/utils.go
Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com>
Signed-off-by: hai.yue <20416005+yuehaii@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The 'tasks' functions for mcp client:
1. The 'GetTask' function will retrieve the task current status.
2. The 'ListTasks' function will list all tasks
3. The 'TaskResult' function will retrieve the finished task information.
4. The 'CancelTask' function will cancel the running task.
Type of Change
Checklist
MCP Spec Compliance
Additional Information
Tested with below code and those tasks client APIs worked as expected:
Summary by CodeRabbit