GitHub/outline
1
0
Fork 0
mirror of https://github.com/outline/outline.git synced 2026-06-13 11:25:03 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: Allow empty string in optional MCP fields (#12310)

Browse Source 
* fix: Allow empty string in optional fields

* fix: Preserve empty strings for content fields in MCP tools

Address review feedback by reverting content/text fields (description, document
text, comment text) back to z.string().optional() so callers can intentionally
clear values via "". optionalString() is reserved for identifier and query
fields where "" is not a meaningful input.
This commit is contained in:
Tom Moor
2026-05-10 10:47:24 -04:00
committed by GitHub
parent ab42e4fda8
commit ff3b3ce552
7 changed files with 111 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/tools/collections.ts
+11 -18
View File
@@ -13,6 +13,7 @@ import {
error,
getActorFromContext,
buildAPIContext,
optionalString,
pathToUrl,
withTracing,
} from "./util";
@@ -37,12 +38,9 @@ export function collectionTools(server: McpServer, scopes: string[]) {
readOnlyHint: true,
},
inputSchema: {
query: z
.string()
.optional()
.describe(
"An optional search query to filter collections by name."
),
query: optionalString().describe(
"An optional search query to filter collections by name."
),
offset: z.coerce
.number()
.int()
@@ -157,14 +155,12 @@ export function collectionTools(server: McpServer, scopes: string[]) {
.string()
.optional()
.describe("A markdown description for the collection."),
icon: z
.string()
.optional()
.describe("An icon for the collection, e.g. an emoji."),
color: z
.string()
.optional()
.describe("The hex color for the collection icon, e.g. #FF0000."),
icon: optionalString().describe(
"An icon for the collection, e.g. an emoji."
),
color: optionalString().describe(
"The hex color for the collection icon, e.g. #FF0000."
),
},
},
withTracing("create_collection", async (input, context) => {
@@ -220,10 +216,7 @@ export function collectionTools(server: McpServer, scopes: string[]) {
id: z
.string()
.describe("The unique identifier of the collection to update."),
name: z
.string()
.optional()
.describe("The new name for the collection."),
name: optionalString().describe("The new name for the collection."),
description: z
.string()
.optional()
server/tools/comments.ts
+13 -18
View File
@@ -17,6 +17,7 @@ import {
success,
buildAPIContext,
getActorFromContext,
optionalString,
withTracing,
} from "./util";
@@ -63,18 +64,15 @@ export function commentTools(server: McpServer, scopes: string[]) {
readOnlyHint: true,
},
inputSchema: {
documentId: z
.string()
.optional()
.describe("The document ID to list comments for."),
collectionId: z
.string()
.optional()
.describe("The collection ID to list comments for."),
parentCommentId: z
.string()
.optional()
.describe("A parent comment ID to list only its replies."),
documentId: optionalString().describe(
"The document ID to list comments for."
),
collectionId: optionalString().describe(
"The collection ID to list comments for."
),
parentCommentId: optionalString().describe(
"A parent comment ID to list only the replies in that thread."
),
statusFilter: z
.array(z.enum(CommentStatusFilter))
.optional()
@@ -236,12 +234,9 @@ export function commentTools(server: McpServer, scopes: string[]) {
text: z
.string()
.describe("The markdown text content of the comment."),
parentCommentId: z
.string()
.optional()
.describe(
"The parent comment ID to reply to. Omit for a top-level comment."
),
parentCommentId: optionalString().describe(
"The parent comment ID to reply to. Omit for a top-level comment."
),
},
},
withTracing(
server/tools/documents.ts
+32 -54
View File
@@ -19,6 +19,7 @@ import {
getActorFromContext,
getBreadcrumbsForDocuments,
getDocumentBreadcrumb,
optionalString,
pathToUrl,
withTracing,
} from "./util";
@@ -45,16 +46,12 @@ export function documentTools(server: McpServer, scopes: string[]) {
readOnlyHint: true,
},
inputSchema: {
query: z
.string()
.optional()
.describe(
"A search query to find documents by content or title. When omitted, returns recent documents."
),
collectionId: z
.string()
.optional()
.describe("An optional collection ID to filter documents by."),
query: optionalString().describe(
"A search query to find documents by content or title. When omitted, returns recent documents."
),
collectionId: optionalString().describe(
"A collection ID to filter documents by."
),
offset: z.coerce
.number()
.int()
@@ -285,22 +282,18 @@ export function documentTools(server: McpServer, scopes: string[]) {
.string()
.optional()
.describe("The markdown content of the document."),
collectionId: z
.string()
.optional()
.describe("The collection to place the document in."),
parentDocumentId: z
.string()
.optional()
.describe("The parent document ID to nest this document under."),
icon: z
.string()
.optional()
.describe("An icon for the document, e.g. an emoji."),
color: z
.string()
.optional()
.describe("The hex color for the document icon, e.g. #FF0000."),
collectionId: optionalString().describe(
"The collection to place the document in."
),
parentDocumentId: optionalString().describe(
"The parent document ID to nest this document under."
),
icon: optionalString().describe(
"An icon for the document, e.g. an emoji."
),
color: optionalString().describe(
"The hex color for the document icon, e.g. #FF0000."
),
publish: z
.boolean()
.optional()
@@ -394,18 +387,12 @@ export function documentTools(server: McpServer, scopes: string[]) {
id: z
.string()
.describe("The unique identifier of the document to move."),
collectionId: z
.string()
.optional()
.describe(
"The destination collection ID. Required if parentDocumentId is not provided."
),
parentDocumentId: z
.string()
.optional()
.describe(
"The ID of the document to nest this document under. The document will be moved to the parent's collection."
),
collectionId: optionalString().describe(
"The destination collection ID. Required if parentDocumentId is not provided."
),
parentDocumentId: optionalString().describe(
"The ID of the document to nest this document under. The document will be moved to the parent's collection."
),
index: z
.number()
.int()
@@ -530,10 +517,7 @@ export function documentTools(server: McpServer, scopes: string[]) {
id: z
.string()
.describe("The unique identifier of the document to update."),
title: z
.string()
.optional()
.describe("The new title for the document."),
title: optionalString().describe("The new title for the document."),
text: z
.string()
.optional()
@@ -546,18 +530,12 @@ export function documentTools(server: McpServer, scopes: string[]) {
.describe(
'How to apply the text update. "replace" (default) replaces the entire document content. "append" adds text to the end. "prepend" adds text to the beginning. "patch" finds the exact markdown specified in findText and replaces only that portion, preserving the rest of the document including any rich formatting that cannot be represented in markdown.'
),
findText: z
.string()
.optional()
.describe(
'Required when editMode is "patch". The exact markdown substring to find in the document. This should be copied verbatim from the document\'s existing markdown content. The first occurrence will be replaced with the text parameter. Can span multiple blocks (paragraphs, headings, etc).'
),
collectionId: z
.string()
.optional()
.describe(
"The collection ID to publish a draft to, required when publishing a draft that has no collection."
),
findText: optionalString().describe(
'Required when editMode is "patch". The exact markdown substring to find in the document. This should be copied verbatim from the document\'s existing markdown content. The first occurrence will be replaced with the text parameter. Can span multiple blocks (paragraphs, headings, etc).'
),
collectionId: optionalString().describe(
"The collection ID to publish a draft to, required when publishing a draft that has no collection."
),
icon: z
.string()
.nullable()
server/tools/users.ts
+11 -8
View File
@@ -7,7 +7,13 @@ import { User, Team } from "@server/models";
import { authorize, can } from "@server/policies";
import { presentUser } from "@server/presenters";
import AuthenticationHelper from "@shared/helpers/AuthenticationHelper";
import { error, success, getActorFromContext, withTracing } from "./util";
import {
error,
success,
getActorFromContext,
optionalString,
withTracing,
} from "./util";
/**
* Registers user-related MCP tools on the given server, filtered by the
@@ -28,12 +34,9 @@ export function userTools(server: McpServer, scopes: string[]) {
readOnlyHint: true,
},
inputSchema: {
query: z
.string()
.optional()
.describe(
"An optional search query to filter users by name or email."
),
query: optionalString().describe(
"An optional search query to filter users by name or email."
),
role: z
.enum([
UserRole.Admin,
@@ -47,7 +50,7 @@ export function userTools(server: McpServer, scopes: string[]) {
.enum(["active", "suspended", "invited", "all"])
.optional()
.describe(
"Filter users by status. 'suspended' is only available to admins. Defaults to active, non-suspended users."
"Filter users by status. Defaults to active, non-suspended users. Note filtering by 'suspended' is only available to admins."
),
offset: z.coerce
.number()
server/tools/util.test.ts
+25 -1
View File
@@ -5,7 +5,11 @@ import {
buildTeam,
buildUser,
} from "@server/test/factories";
import { buildBreadcrumb, getBreadcrumbsForDocuments } from "./util";
import {
buildBreadcrumb,
getBreadcrumbsForDocuments,
optionalString,
} from "./util";
const node = (
id: string,
@@ -62,6 +66,26 @@ describe("buildBreadcrumb", () => {
});
});
describe("optionalString", () => {
const schema = optionalString();
it("returns undefined when input is omitted", () => {
expect(schema.parse(undefined)).toBeUndefined();
});
it("coerces an empty string to undefined", () => {
expect(schema.parse("")).toBeUndefined();
});
it("passes through a non-empty string", () => {
expect(schema.parse("hello")).toBe("hello");
});
it("preserves whitespace-only strings", () => {
expect(schema.parse(" ")).toBe(" ");
});
});
describe("getBreadcrumbsForDocuments", () => {
it("returns the collection name for a root-level document", async () => {
const team = await buildTeam();
server/tools/util.ts
+18
View File
@@ -1,5 +1,6 @@
import type { AuthInfo } from "@modelcontextprotocol/sdk/server/auth/types.js";
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
import { z } from "zod";
import { Collection, type Team, type User } from "@server/models";
import { addTags } from "@server/logging/tracer";
import { traceFunction } from "@server/logging/tracing";
@@ -46,6 +47,23 @@ export function buildAPIContext(context: McpContext) {
} as unknown as APIContext;
}
/**
* Builds a zod schema for an optional string MCP tool input that coerces
* empty strings to `undefined`. MCP clients sometimes send `""` for fields
* the caller intended to omit. Use this for identifier, query, and similar
* fields where `""` is not a meaningful value — keep `z.string().optional()`
* for content/text fields where an empty string is a valid input (e.g.
* clearing a description).
*
* @returns a zod schema accepting `string | undefined`, with `""` treated as `undefined`.
*/
export function optionalString() {
return z
.string()
.optional()
.transform((v) => (v === "" ? undefined : v));
}
/**
* Helper function to format successful MCP tool responses.
*
shared/i18n/locales/en_US/translation.json
+1
View File
@@ -169,6 +169,7 @@
"New workspace": "New workspace",
"Create a workspace": "Create a workspace",
"Login to workspace": "Login to workspace",
"template": "template",
"Template deleted": "Template deleted",
"Deleting": "Deleting",
"Are you sure about that? Deleting the <em>{{ templateName }}</em> template is permanent.": "Are you sure about that? Deleting the <em>{{ templateName }}</em> template is permanent.",