Skip to content

Fix tool handlers to return proper MCP content format - #21

Open
moonraker46 wants to merge 1 commit into
nloui:mainfrom
moonraker46:fix/mcp-tool-content-format
Open

Fix tool handlers to return proper MCP content format#21
moonraker46 wants to merge 1 commit into
nloui:mainfrom
moonraker46:fix/mcp-tool-content-format

Conversation

@moonraker46

@moonraker46 moonraker46 commented Feb 26, 2026

Copy link
Copy Markdown

...

Summary by CodeRabbit

Release Notes

  • Refactor
    • Standardized internal response formatting across multiple tool modules (correspondents, document types, documents, tags) to ensure consistent handling of API results throughout the system.

All tool handlers were returning raw API response objects directly,
causing Claude to receive results without the required MCP content
array. Claude.ai looks for the content field and shows nothing when
it is absent. Wrap all tool return values in the standard MCP format:
{ content: [{ type: "text", text: JSON.stringify(result) }] }
@coderabbitai

coderabbitai Bot commented Feb 26, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

All tool handlers across correspondents, document types, documents, and tags modules are refactored to wrap API results in a standardized content payload structure via a new toContent helper function, without modifying public function signatures.

Changes

Cohort / File(s) Summary
Tool Response Standardization
src/tools/correspondents.ts, src/tools/documentTypes.ts, src/tools/documents.ts, src/tools/tags.ts
Added internal toContent(result) helper function to each file that wraps API results into a standardized payload: { content: [{ type: "text", text: JSON.stringify(result) }] }. Updated all tool handlers (list, create, update, delete, bulk edit operations) to return wrapped results via toContent(await api...(...)) instead of raw values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Wrap, wrap, content so tight,
Standard payloads, everything right!
JSON strings in structured embrace,
Four tools dancing in uniform grace!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix tool handlers to return proper MCP content format' accurately describes the main change: wrapping API results in MCP content structure across all tool handlers.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/tools/documents.ts (1)

3-5: Consider extracting toContent into a shared utility.

The helper is duplicated across tool modules; centralizing it will prevent behavior drift and make future serialization fixes one-shot.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/documents.ts` around lines 3 - 5, Extract the toContent helper into
a single shared utility module (e.g., export function toContent(result: any))
and replace the local toContent implementation in src/tools/documents.ts with an
import from that shared module; update any other tool modules that currently
duplicate the function to import the single exported toContent so behavior stays
consistent, preserve the return shape ({ content: [{ type: "text" as const,
text: JSON.stringify(result) }] }) and TypeScript typing when moving, and
run/build tests to ensure no import/name collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tools/documentTypes.ts`:
- Around line 3-5: The toContent function can produce non-string or throw from
JSON.stringify(result); update toContent to ensure text is always a string by:
attempt JSON.stringify(result) inside a try/catch, fall back to String(result ??
"") (or "" for undefined/null) when stringify returns undefined or throws, and
guarantee the returned object shape remains { content: [{ type: "text" as const,
text: string }] }; apply the same change to every occurrence of toContent in
src/tools/documentTypes.ts, src/tools/tags.ts, src/tools/documents.ts, and
src/tools/correspondents.ts so all MCP payloads always have a string text.

---

Nitpick comments:
In `@src/tools/documents.ts`:
- Around line 3-5: Extract the toContent helper into a single shared utility
module (e.g., export function toContent(result: any)) and replace the local
toContent implementation in src/tools/documents.ts with an import from that
shared module; update any other tool modules that currently duplicate the
function to import the single exported toContent so behavior stays consistent,
preserve the return shape ({ content: [{ type: "text" as const, text:
JSON.stringify(result) }] }) and TypeScript typing when moving, and run/build
tests to ensure no import/name collisions.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ba7457 and 1cf165f.

📒 Files selected for processing (4)
  • src/tools/correspondents.ts
  • src/tools/documentTypes.ts
  • src/tools/documents.ts
  • src/tools/tags.ts

Comment on lines +3 to +5
function toContent(result: any) {
return { content: [{ type: "text" as const, text: JSON.stringify(result) }] };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the toContent function definition and its usage
cd /tmp && find . -name "documentTypes.ts" -type f 2>/dev/null | head -5

Repository: nloui/paperless-mcp

Length of output: 45


🏁 Script executed:

# Let's find the file and examine it
fd documentTypes.ts --type f

Repository: nloui/paperless-mcp

Length of output: 89


🏁 Script executed:

# Search for toContent definition and usages
rg -n "toContent" --type ts

Repository: nloui/paperless-mcp

Length of output: 1625


🏁 Script executed:

# Get the specific file content to see the toContent function
cat -n src/tools/documentTypes.ts | head -50

Repository: nloui/paperless-mcp

Length of output: 3990


🏁 Script executed:

# Find the API implementation to check method return types
fd api --type d

Repository: nloui/paperless-mcp

Length of output: 71


🏁 Script executed:

# Search for API method definitions, particularly for bulk operations and delete
rg -n "bulkEditObjects|bulkEditDocuments|deleteTag|createDocumentType|getDocumentTypes" --type ts -A 5 | head -100

Repository: nloui/paperless-mcp

Length of output: 3957


🏁 Script executed:

# Look for the api module/implementation
find . -name "*.ts" -type f | xargs rg -l "export.*bulkEditObjects|export.*deleteTag" | head -5

Repository: nloui/paperless-mcp

Length of output: 45


🏁 Script executed:

# Get the API implementation file
cat -n src/api/PaperlessAPI.ts | head -100

Repository: nloui/paperless-mcp

Length of output: 3383


🏁 Script executed:

# Look for the request method to understand what it returns
rg -n "async request|private request" src/api/PaperlessAPI.ts -A 10 | head -50

Repository: nloui/paperless-mcp

Length of output: 462


🏁 Script executed:

# Check specifically for DELETE operations and what they return
rg -n "DELETE" src/api/PaperlessAPI.ts -B 2 -A 5

Repository: nloui/paperless-mcp

Length of output: 257


🏁 Script executed:

# Get more context on the request method to see complete implementation
cat -n src/api/PaperlessAPI.ts | sed -n '10,40p'

Repository: nloui/paperless-mcp

Length of output: 1036


🏁 Script executed:

# Check if there are any API endpoints that might return empty/void responses
rg -n "204|No Content|DELETE.*response" src/api/

Repository: nloui/paperless-mcp

Length of output: 45


🏁 Script executed:

# Verify JSON.stringify behavior with undefined and edge cases using Node.js
node << 'EOF'
// Test JSON.stringify behavior
console.log("=== JSON.stringify behavior ===");
console.log("undefined:", JSON.stringify(undefined));
console.log("null:", JSON.stringify(null));
console.log("empty object:", JSON.stringify({}));

// Test what happens with circular references
try {
  const circular = { a: 1 };
  circular.self = circular;
  JSON.stringify(circular);
} catch (e) {
  console.log("Circular ref error:", e.message);
}

// Test with BigInt
try {
  JSON.stringify({ value: BigInt(123) });
} catch (e) {
  console.log("BigInt error:", e.message);
}

// Test edge case - what if result is a primitive?
console.log("String:", JSON.stringify("hello"));
console.log("Number:", JSON.stringify(42));
console.log("Boolean:", JSON.stringify(true));
EOF

Repository: nloui/paperless-mcp

Length of output: 389


🏁 Script executed:

# Check what the text field should actually be in MCP protocol
rg -n "type.*text.*const" src/tools/ -B 2 -A 2 | head -30

Repository: nloui/paperless-mcp

Length of output: 1067


Harden toContent so text is always a string.

JSON.stringify(result) can return undefined (for undefined/void values) or throw on non-serializable types (circular refs, BigInt), producing invalid MCP payloads where text must be a string. DELETE operations or 204 responses in particular may trigger this.

🔧 Suggested fix
-function toContent(result: any) {
-  return { content: [{ type: "text" as const, text: JSON.stringify(result) }] };
+function toContent(result: unknown) {
+  let text: string;
+  try {
+    const json = JSON.stringify(result);
+    text = json ?? "null";
+  } catch {
+    text = String(result);
+  }
+  return { content: [{ type: "text" as const, text }] };
 }

This function appears in src/tools/documentTypes.ts, src/tools/tags.ts, src/tools/documents.ts, and src/tools/correspondents.ts and should be hardened consistently across all files.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tools/documentTypes.ts` around lines 3 - 5, The toContent function can
produce non-string or throw from JSON.stringify(result); update toContent to
ensure text is always a string by: attempt JSON.stringify(result) inside a
try/catch, fall back to String(result ?? "") (or "" for undefined/null) when
stringify returns undefined or throws, and guarantee the returned object shape
remains { content: [{ type: "text" as const, text: string }] }; apply the same
change to every occurrence of toContent in src/tools/documentTypes.ts,
src/tools/tags.ts, src/tools/documents.ts, and src/tools/correspondents.ts so
all MCP payloads always have a string text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant