mirror of
https://github.com/outline/outline.git
synced 2026-06-13 11:25:03 +03:00
Ensure full urls are returned from MCP (#11482)
* Ensure full urls are returned from MCP towards #11474 * Fix pathToUrl using path.join for URLs and add test coverage path.join collapsed https:// to https:/ — use URL constructor instead. Added assertions verifying full URLs are returned for collections and documents across list, create, update, and resource endpoints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add scopes_supported --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -128,6 +128,7 @@ router.get("/.well-known/oauth-authorization-server", async (ctx) => {
|
||||
grant_types_supported: ["authorization_code", "refresh_token"],
|
||||
token_endpoint_auth_methods_supported: ["client_secret_post", "none"],
|
||||
code_challenge_methods_supported: ["S256"],
|
||||
scopes_supported: ["read", "write"],
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -116,11 +116,18 @@ describe("POST /mcp/", () => {
|
||||
});
|
||||
|
||||
const res = await callMcpTool(server, accessToken, "list_collections");
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "[]");
|
||||
const data = (res?.result?.content ?? []).map((c: { text: string }) =>
|
||||
JSON.parse(c.text)
|
||||
);
|
||||
|
||||
expect(data.length).toBeGreaterThanOrEqual(1);
|
||||
const ids = data.map((c: { id: string }) => c.id);
|
||||
expect(ids).toContain(collection.id);
|
||||
|
||||
const match = data.find(
|
||||
(c: { id: string }) => c.id === collection.id
|
||||
) as { url: string };
|
||||
expect(match.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
|
||||
it("list_collections does not return collections from another team", async () => {
|
||||
@@ -132,7 +139,9 @@ describe("POST /mcp/", () => {
|
||||
});
|
||||
|
||||
const res = await callMcpTool(server, accessToken, "list_collections");
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "[]");
|
||||
const data = (res?.result?.content ?? []).map((c: { text: string }) =>
|
||||
JSON.parse(c.text)
|
||||
);
|
||||
|
||||
const ids = data.map((c: { id: string }) => c.id);
|
||||
expect(ids).not.toContain(otherCollection.id);
|
||||
@@ -153,6 +162,7 @@ describe("POST /mcp/", () => {
|
||||
expect(data.icon).toEqual("rocket");
|
||||
expect(data.color).toEqual("#FF0000");
|
||||
expect(data.id).toBeDefined();
|
||||
expect(data.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
|
||||
it("update_collection updates fields on existing collection", async () => {
|
||||
@@ -170,6 +180,7 @@ describe("POST /mcp/", () => {
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "{}");
|
||||
|
||||
expect(data.name).toEqual("Updated Name");
|
||||
expect(data.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
|
||||
it("get_collection resource returns collection details", async () => {
|
||||
@@ -190,6 +201,7 @@ describe("POST /mcp/", () => {
|
||||
|
||||
const data = JSON.parse(res!.result!.contents![0].text ?? "{}");
|
||||
expect(data.id).toEqual(collection.id);
|
||||
expect(data.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -207,10 +219,17 @@ describe("POST /mcp/", () => {
|
||||
});
|
||||
|
||||
const res = await callMcpTool(server, accessToken, "list_documents");
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "[]");
|
||||
const data = (res?.result?.content ?? []).map((c: { text: string }) =>
|
||||
JSON.parse(c.text)
|
||||
);
|
||||
|
||||
const ids = data.map((d: { id: string }) => d.id);
|
||||
expect(ids).toContain(document.id);
|
||||
|
||||
const match = data.find((d: { id: string }) => d.id === document.id) as {
|
||||
url: string;
|
||||
};
|
||||
expect(match.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
|
||||
it("list_documents filters by collection", async () => {
|
||||
@@ -237,7 +256,9 @@ describe("POST /mcp/", () => {
|
||||
const res = await callMcpTool(server, accessToken, "list_documents", {
|
||||
collectionId: collection1.id,
|
||||
});
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "[]");
|
||||
const data = (res?.result?.content ?? []).map((c: { text: string }) =>
|
||||
JSON.parse(c.text)
|
||||
);
|
||||
|
||||
const ids = data.map((d: { id: string }) => d.id);
|
||||
expect(ids).toContain(doc1.id);
|
||||
@@ -265,6 +286,7 @@ describe("POST /mcp/", () => {
|
||||
expect(data.title).toEqual("New Document");
|
||||
expect(data.collectionId).toEqual(collection.id);
|
||||
expect(data.id).toBeDefined();
|
||||
expect(data.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
|
||||
it("create_document creates nested under parent document", async () => {
|
||||
@@ -310,6 +332,7 @@ describe("POST /mcp/", () => {
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "{}");
|
||||
|
||||
expect(data.title).toEqual("Updated Title");
|
||||
expect(data.url).toMatch(/^https?:\/\//);
|
||||
});
|
||||
|
||||
it("get_document resource returns metadata and markdown", async () => {
|
||||
@@ -338,6 +361,7 @@ describe("POST /mcp/", () => {
|
||||
const metadata = JSON.parse(res!.result!.contents![0].text ?? "{}");
|
||||
expect(metadata.id).toEqual(document.id);
|
||||
expect(metadata.title).toEqual(document.title);
|
||||
expect(metadata.url).toMatch(/^https?:\/\//);
|
||||
|
||||
// Second content is markdown text
|
||||
expect(res!.result!.contents![1].mimeType).toEqual("text/markdown");
|
||||
@@ -365,7 +389,9 @@ describe("POST /mcp/", () => {
|
||||
const res = await callMcpTool(server, accessToken, "list_comments", {
|
||||
documentId: document.id,
|
||||
});
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "[]");
|
||||
const data = (res?.result?.content ?? []).map((c: { text: string }) =>
|
||||
JSON.parse(c.text)
|
||||
);
|
||||
|
||||
const ids = data.map((c: { id: string }) => c.id);
|
||||
expect(ids).toContain(comment.id);
|
||||
@@ -517,7 +543,9 @@ describe("POST /mcp/", () => {
|
||||
|
||||
const res = await callMcpTool(server, accessToken, "list_collections");
|
||||
expect(res?.error).toBeUndefined();
|
||||
const data = JSON.parse(res?.result?.content?.[0]?.text ?? "[]");
|
||||
const data = (res?.result?.content ?? []).map((c: { text: string }) =>
|
||||
JSON.parse(c.text)
|
||||
);
|
||||
expect(data.length).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
|
||||
@@ -10,7 +10,13 @@ import { Collection, Team } from "@server/models";
|
||||
import { authorize } from "@server/policies";
|
||||
import { presentCollection } from "@server/presenters";
|
||||
import AuthenticationHelper from "@shared/helpers/AuthenticationHelper";
|
||||
import { success, error, getActorFromContext, buildAPIContext } from "./util";
|
||||
import {
|
||||
success,
|
||||
error,
|
||||
getActorFromContext,
|
||||
buildAPIContext,
|
||||
pathToUrl,
|
||||
} from "./util";
|
||||
|
||||
/**
|
||||
* Registers collection-related MCP tools and resources on the given server,
|
||||
@@ -93,8 +99,11 @@ export function collectionTools(server: McpServer, scopes: string[]) {
|
||||
});
|
||||
|
||||
const presented = await Promise.all(
|
||||
collections.map((collection) =>
|
||||
presentCollection(undefined, collection)
|
||||
collections.map(async (collection) =>
|
||||
pathToUrl(
|
||||
user.team,
|
||||
await presentCollection(undefined, collection)
|
||||
)
|
||||
)
|
||||
);
|
||||
return success(presented);
|
||||
@@ -132,7 +141,7 @@ export function collectionTools(server: McpServer, scopes: string[]) {
|
||||
{
|
||||
uri: uri.href,
|
||||
mimeType: "application/json",
|
||||
text: JSON.stringify(presented),
|
||||
text: JSON.stringify(pathToUrl(user.team, presented)),
|
||||
},
|
||||
{
|
||||
uri: uri.href,
|
||||
@@ -204,7 +213,10 @@ export function collectionTools(server: McpServer, scopes: string[]) {
|
||||
rejectOnEmpty: true,
|
||||
});
|
||||
|
||||
const presented = await presentCollection(undefined, reloaded);
|
||||
const presented = pathToUrl(
|
||||
user.team,
|
||||
await presentCollection(undefined, reloaded)
|
||||
);
|
||||
return success(presented);
|
||||
} catch (message) {
|
||||
return error(message);
|
||||
@@ -278,7 +290,10 @@ export function collectionTools(server: McpServer, scopes: string[]) {
|
||||
|
||||
await collection.saveWithCtx(ctx);
|
||||
|
||||
const presented = await presentCollection(undefined, collection);
|
||||
const presented = pathToUrl(
|
||||
user.team,
|
||||
await presentCollection(undefined, collection)
|
||||
);
|
||||
return success(presented);
|
||||
} catch (message) {
|
||||
return error(message);
|
||||
|
||||
+25
-13
@@ -16,7 +16,13 @@ import SearchHelper from "@server/models/helpers/SearchHelper";
|
||||
import { authorize } from "@server/policies";
|
||||
import { presentDocument } from "@server/presenters";
|
||||
import AuthenticationHelper from "@shared/helpers/AuthenticationHelper";
|
||||
import { error, success, buildAPIContext, getActorFromContext } from "./util";
|
||||
import {
|
||||
error,
|
||||
success,
|
||||
buildAPIContext,
|
||||
getActorFromContext,
|
||||
pathToUrl,
|
||||
} from "./util";
|
||||
import { TextEditMode } from "@shared/types";
|
||||
|
||||
/**
|
||||
@@ -61,7 +67,7 @@ export function documentTools(server: McpServer, scopes: string[]) {
|
||||
{
|
||||
uri: uri.href,
|
||||
mimeType: "application/json",
|
||||
text: JSON.stringify(attributes),
|
||||
text: JSON.stringify(pathToUrl(user.team, attributes)),
|
||||
},
|
||||
{
|
||||
uri: uri.href,
|
||||
@@ -142,10 +148,13 @@ export function documentTools(server: McpServer, scopes: string[]) {
|
||||
|
||||
const presented = await Promise.all(
|
||||
results.map(async (result) => {
|
||||
const doc = await presentDocument(undefined, result.document, {
|
||||
includeData: false,
|
||||
includeText: false,
|
||||
});
|
||||
const doc = pathToUrl(
|
||||
user.team,
|
||||
await presentDocument(undefined, result.document, {
|
||||
includeData: false,
|
||||
includeText: false,
|
||||
})
|
||||
);
|
||||
return { ...doc, context: result.context };
|
||||
})
|
||||
);
|
||||
@@ -169,11 +178,14 @@ export function documentTools(server: McpServer, scopes: string[]) {
|
||||
});
|
||||
|
||||
const presented = await Promise.all(
|
||||
documents.map((document) =>
|
||||
presentDocument(undefined, document, {
|
||||
includeData: false,
|
||||
includeText: false,
|
||||
})
|
||||
documents.map(async (document) =>
|
||||
pathToUrl(
|
||||
user.team,
|
||||
await presentDocument(undefined, document, {
|
||||
includeData: false,
|
||||
includeText: false,
|
||||
})
|
||||
)
|
||||
)
|
||||
);
|
||||
return success(presented);
|
||||
@@ -272,7 +284,7 @@ export function documentTools(server: McpServer, scopes: string[]) {
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: JSON.stringify(attributes),
|
||||
text: JSON.stringify(pathToUrl(user.team, attributes)),
|
||||
},
|
||||
{
|
||||
type: "text" as const,
|
||||
@@ -367,7 +379,7 @@ export function documentTools(server: McpServer, scopes: string[]) {
|
||||
content: [
|
||||
{
|
||||
type: "text" as const,
|
||||
text: JSON.stringify(attributes),
|
||||
text: JSON.stringify(pathToUrl(user.team, attributes)),
|
||||
},
|
||||
{
|
||||
type: "text" as const,
|
||||
|
||||
+32
-3
@@ -1,6 +1,6 @@
|
||||
import type { AuthInfo } from "@modelcontextprotocol/sdk/server/auth/types.js";
|
||||
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
|
||||
import type { User } from "@server/models";
|
||||
import type { Team, User } from "@server/models";
|
||||
import { type APIContext, AuthenticationType } from "@server/types";
|
||||
|
||||
interface McpContext {
|
||||
@@ -46,9 +46,14 @@ export function buildAPIContext(context: McpContext) {
|
||||
* @param data - the data to include in the response.
|
||||
* @returns a formatted response object for MCP tools.
|
||||
*/
|
||||
export function success<T>(data: T): CallToolResult {
|
||||
export function success<T>(data: T | T[]): CallToolResult {
|
||||
const payload = Array.isArray(data) ? data : [data];
|
||||
|
||||
return {
|
||||
content: [{ type: "text" as const, text: JSON.stringify(data) }],
|
||||
content: payload.map((item) => ({
|
||||
type: "text" as const,
|
||||
text: JSON.stringify(item),
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -66,3 +71,27 @@ export function error(err: unknown): CallToolResult {
|
||||
isError: true,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Utility function to construct a URL by joining a team URL with a path segment.
|
||||
*
|
||||
* @param team - the team object containing the base URL.
|
||||
* @param input - an object with attributes keys to be joined with the team URL.
|
||||
* @returns the combined URL string.
|
||||
*/
|
||||
export function pathToUrl(team: Team, input: Record<string, unknown>) {
|
||||
const baseUrl = team.url;
|
||||
|
||||
for (const [key, value] of Object.entries(input)) {
|
||||
if (["url", "path"].includes(key) && typeof value === "string") {
|
||||
// check for existing protocol to avoid double joining
|
||||
if (/^https?:\/\//.test(value)) {
|
||||
input[key] = value;
|
||||
} else {
|
||||
input[key] = new URL(value, baseUrl).href;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return input;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user